Add MaybeError to d3d12::Device::ExecuteCommandList

Closing a command list can fail. We need to handle the error
gracefully instead of ignoring it.

As a fallout from adding MaybeError to ExecuteCommandList, need to
also add MaybeError to TickImpl, and OnBeforePresent.

Bug:dawn:19
Change-Id: I13685f3dd731f4ab49cbff4ce4edfa960d630464
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/11841
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Rafael Cintron <rafael.cintron@microsoft.com>

src/dawn_native/Device.cpp

@@ -536,7 +536,9 @@ namespace dawn_native {
     // Other Device API methods
 
     void DeviceBase::Tick() {
-        TickImpl();
+        if (ConsumedError(TickImpl()))
+            return;
+
         {
             auto deferredResults = std::move(mDeferredCreateBufferMappedAsyncResults);
             for (const auto& deferred : deferredResults) {

src/dawn_native/Device.h

@@ -81,7 +81,7 @@ namespace dawn_native {
         virtual Serial GetCompletedCommandSerial() const = 0;
         virtual Serial GetLastSubmittedCommandSerial() const = 0;
         virtual Serial GetPendingCommandSerial() const = 0;
-        virtual void TickImpl() = 0;
+        virtual MaybeError TickImpl() = 0;
 
         // Many Dawn objects are completely immutable once created which means that if two
         // creations are given the same arguments, they can return the same object. Reusing

src/dawn_native/SwapChain.cpp

@@ -32,7 +32,7 @@ namespace dawn_native {
                 UNREACHABLE();
             }
 
-            void OnBeforePresent(TextureBase* texture) override {
+            MaybeError OnBeforePresent(TextureBase* texture) override {
                 UNREACHABLE();
             }
         };
@@ -127,7 +127,8 @@ namespace dawn_native {
         }
         ASSERT(!IsError());
 
-        OnBeforePresent(texture);
+        if (GetDevice()->ConsumedError(OnBeforePresent(texture)))
+            return;
 
         mImplementation.Present(mImplementation.userData);
     }

src/dawn_native/SwapChain.h

@@ -47,7 +47,7 @@ namespace dawn_native {
 
         const DawnSwapChainImplementation& GetImplementation();
         virtual TextureBase* GetNextTextureImpl(const TextureDescriptor*) = 0;
-        virtual void OnBeforePresent(TextureBase* texture) = 0;
+        virtual MaybeError OnBeforePresent(TextureBase* texture) = 0;
 
       private:
         MaybeError ValidateConfigure(dawn::TextureFormat format,

src/dawn_native/d3d12/DeviceD3D12.cpp

@@ -109,7 +109,10 @@ namespace dawn_native { namespace d3d12 {
         }
         NextSerial();
         WaitForSerial(mLastSubmittedSerial);  // Wait for all in-flight commands to finish executing
-        TickImpl();                           // Call tick one last time so resources are cleaned up
+
+        // Call tick one last time so resources are cleaned up. Ignore the return value so we can
+        // continue shutting down in an orderly fashion.
+        ConsumedError(TickImpl());
 
         // Free services explicitly so that they can free D3D12 resources before destruction of the
         // device.
@@ -209,7 +212,7 @@ namespace dawn_native { namespace d3d12 {
         return mLastSubmittedSerial + 1;
     }
 
-    void Device::TickImpl() {
+    MaybeError Device::TickImpl() {
         // Perform cleanup operations to free unused objects
         mCompletedSerial = mFence->GetCompletedValue();
 
@@ -222,8 +225,10 @@ namespace dawn_native { namespace d3d12 {
         mDescriptorHeapAllocator->Deallocate(mCompletedSerial);
         mMapRequestTracker->Tick(mCompletedSerial);
         mUsedComObjectRefs.ClearUpTo(mCompletedSerial);
-        ExecuteCommandList(nullptr);
+        DAWN_TRY(ExecuteCommandList(nullptr));
         NextSerial();
+
+        return {};
     }
 
     void Device::NextSerial() {
@@ -243,13 +248,18 @@ namespace dawn_native { namespace d3d12 {
         mUsedComObjectRefs.Enqueue(object, GetPendingCommandSerial());
     }
 
-    void Device::ExecuteCommandList(ID3D12CommandList* d3d12CommandList) {
+    MaybeError Device::ExecuteCommandList(ID3D12CommandList* d3d12CommandList) {
         UINT numLists = 0;
         std::array<ID3D12CommandList*, 2> d3d12CommandLists;
 
         // If there are pending commands, prepend them to ExecuteCommandLists
         if (mPendingCommands.open) {
-            mPendingCommands.commandList->Close();
+            const HRESULT hr = mPendingCommands.commandList->Close();
+            if (FAILED(hr)) {
+                mPendingCommands.open = false;
+                mPendingCommands.commandList.Reset();
+                return DAWN_DEVICE_LOST_ERROR("Error closing pending command list.");
+            }
             mPendingCommands.open = false;
             d3d12CommandLists[numLists++] = mPendingCommands.commandList.Get();
         }
@@ -260,6 +270,8 @@ namespace dawn_native { namespace d3d12 {
             mCommandQueue->ExecuteCommandLists(numLists, d3d12CommandLists.data());
             mPendingCommands.commandList.Reset();
         }
+
+        return {};
     }
 
     ResultOrError<BindGroupBase*> Device::CreateBindGroupImpl(

src/dawn_native/d3d12/DeviceD3D12.h

@@ -53,7 +53,7 @@ namespace dawn_native { namespace d3d12 {
 
         Serial GetCompletedCommandSerial() const final override;
         Serial GetLastSubmittedCommandSerial() const final override;
-        void TickImpl() override;
+        MaybeError TickImpl() override;
 
         ComPtr<ID3D12Device> GetD3D12Device() const;
         ComPtr<ID3D12CommandQueue> GetCommandQueue() const;
@@ -78,7 +78,7 @@ namespace dawn_native { namespace d3d12 {
 
         void ReferenceUntilUnused(ComPtr<IUnknown> object);
 
-        void ExecuteCommandList(ID3D12CommandList* d3d12CommandList);
+        MaybeError ExecuteCommandList(ID3D12CommandList* d3d12CommandList);
 
         ResultOrError<std::unique_ptr<StagingBufferBase>> CreateStagingBuffer(size_t size) override;
         MaybeError CopyFromStagingToBuffer(StagingBufferBase* source,

src/dawn_native/d3d12/QueueD3D12.cpp

@@ -33,7 +33,7 @@ namespace dawn_native { namespace d3d12 {
         }
         ASSERT_SUCCESS(mCommandList->Close());
 
-        device->ExecuteCommandList(mCommandList.Get());
+        DAWN_TRY(device->ExecuteCommandList(mCommandList.Get()));
 
         device->NextSerial();
         return {};

src/dawn_native/d3d12/SwapChainD3D12.cpp

@@ -48,13 +48,15 @@ namespace dawn_native { namespace d3d12 {
         return new Texture(ToBackend(GetDevice()), descriptor, nativeTexture);
     }
 
-    void SwapChain::OnBeforePresent(TextureBase* texture) {
+    MaybeError SwapChain::OnBeforePresent(TextureBase* texture) {
         Device* device = ToBackend(GetDevice());
 
         // Perform the necessary transition for the texture to be presented.
         ToBackend(texture)->TransitionUsageNow(device->GetPendingCommandList(), mTextureUsage);
 
-        device->ExecuteCommandList(nullptr);
+        DAWN_TRY(device->ExecuteCommandList(nullptr));
+
+        return {};
     }
 
 }}  // namespace dawn_native::d3d12

src/dawn_native/d3d12/SwapChainD3D12.h

@@ -28,7 +28,7 @@ namespace dawn_native { namespace d3d12 {
 
       protected:
         TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
-        void OnBeforePresent(TextureBase* texture) override;
+        MaybeError OnBeforePresent(TextureBase* texture) override;
 
         dawn::TextureUsage mTextureUsage;
     };

src/dawn_native/metal/DeviceMTL.h

@@ -43,7 +43,7 @@ namespace dawn_native { namespace metal {
 
         Serial GetCompletedCommandSerial() const final override;
         Serial GetLastSubmittedCommandSerial() const final override;
-        void TickImpl() override;
+        MaybeError TickImpl() override;
 
         id<MTLDevice> GetMTLDevice();
         id<MTLCommandQueue> GetMTLQueue();

src/dawn_native/metal/DeviceMTL.mm

@@ -151,7 +151,7 @@ namespace dawn_native { namespace metal {
         return mLastSubmittedSerial + 1;
     }
 
-    void Device::TickImpl() {
+    MaybeError Device::TickImpl() {
         Serial completedSerial = GetCompletedCommandSerial();
 
         mDynamicUploader->Deallocate(completedSerial);
@@ -165,6 +165,8 @@ namespace dawn_native { namespace metal {
             mCompletedSerial++;
             mLastSubmittedSerial++;
         }
+
+        return {};
     }
 
     id<MTLDevice> Device::GetMTLDevice() {

src/dawn_native/metal/SwapChainMTL.h

@@ -28,7 +28,7 @@ namespace dawn_native { namespace metal {
 
       protected:
         TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
-        void OnBeforePresent(TextureBase* texture) override;
+        MaybeError OnBeforePresent(TextureBase* texture) override;
     };
 
 }}  // namespace dawn_native::metal

src/dawn_native/metal/SwapChainMTL.mm

@@ -46,7 +46,8 @@ namespace dawn_native { namespace metal {
         return new Texture(ToBackend(GetDevice()), descriptor, nativeTexture);
     }
 
-    void SwapChain::OnBeforePresent(TextureBase*) {
+    MaybeError SwapChain::OnBeforePresent(TextureBase*) {
+        return {};
     }
 
 }}  // namespace dawn_native::metal

src/dawn_native/null/DeviceNull.cpp

@@ -197,8 +197,9 @@ namespace dawn_native { namespace null {
         return mLastSubmittedSerial + 1;
     }
 
-    void Device::TickImpl() {
+    MaybeError Device::TickImpl() {
         SubmitPendingOperations();
+        return {};
     }
 
     void Device::AddPendingOperation(std::unique_ptr<PendingOperation> operation) {
@@ -338,7 +339,8 @@ namespace dawn_native { namespace null {
         return GetDevice()->CreateTexture(descriptor);
     }
 
-    void SwapChain::OnBeforePresent(TextureBase*) {
+    MaybeError SwapChain::OnBeforePresent(TextureBase*) {
+        return {};
     }
 
     // NativeSwapChainImpl

src/dawn_native/null/DeviceNull.h

@@ -92,7 +92,7 @@ namespace dawn_native { namespace null {
         Serial GetCompletedCommandSerial() const final override;
         Serial GetLastSubmittedCommandSerial() const final override;
         Serial GetPendingCommandSerial() const override;
-        void TickImpl() override;
+        MaybeError TickImpl() override;
 
         void AddPendingOperation(std::unique_ptr<PendingOperation> operation);
         void SubmitPendingOperations();
@@ -201,7 +201,7 @@ namespace dawn_native { namespace null {
 
       protected:
         TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
-        void OnBeforePresent(TextureBase*) override;
+        MaybeError OnBeforePresent(TextureBase*) override;
     };
 
     class NativeSwapChainImpl {

src/dawn_native/opengl/DeviceGL.cpp

@@ -132,8 +132,9 @@ namespace dawn_native { namespace opengl {
         return mLastSubmittedSerial + 1;
     }
 
-    void Device::TickImpl() {
+    MaybeError Device::TickImpl() {
         CheckPassedFences();
+        return {};
     }
 
     void Device::CheckPassedFences() {

src/dawn_native/opengl/DeviceGL.h

@@ -53,7 +53,7 @@ namespace dawn_native { namespace opengl {
         Serial GetCompletedCommandSerial() const final override;
         Serial GetLastSubmittedCommandSerial() const final override;
         Serial GetPendingCommandSerial() const override;
-        void TickImpl() override;
+        MaybeError TickImpl() override;
 
         ResultOrError<std::unique_ptr<StagingBufferBase>> CreateStagingBuffer(size_t size) override;
         MaybeError CopyFromStagingToBuffer(StagingBufferBase* source,

src/dawn_native/opengl/SwapChainGL.cpp

@@ -44,7 +44,8 @@ namespace dawn_native { namespace opengl {
                            TextureBase::TextureState::OwnedExternal);
     }
 
-    void SwapChain::OnBeforePresent(TextureBase*) {
+    MaybeError SwapChain::OnBeforePresent(TextureBase*) {
+        return {};
     }
 
 }}  // namespace dawn_native::opengl

src/dawn_native/opengl/SwapChainGL.h

@@ -30,7 +30,7 @@ namespace dawn_native { namespace opengl {
 
       protected:
         TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
-        void OnBeforePresent(TextureBase* texture) override;
+        MaybeError OnBeforePresent(TextureBase* texture) override;
     };
 
 }}  // namespace dawn_native::opengl

src/dawn_native/vulkan/DeviceVk.cpp

@@ -209,7 +209,7 @@ namespace dawn_native { namespace vulkan {
         return mLastSubmittedSerial + 1;
     }
 
-    void Device::TickImpl() {
+    MaybeError Device::TickImpl() {
         CheckPassedFences();
         RecycleCompletedCommands();
 
@@ -231,6 +231,8 @@ namespace dawn_native { namespace vulkan {
             mCompletedSerial++;
             mLastSubmittedSerial++;
         }
+
+        return {};
     }
 
     VkInstance Device::GetVkInstance() const {

src/dawn_native/vulkan/DeviceVk.h

@@ -83,7 +83,7 @@ namespace dawn_native { namespace vulkan {
 
         Serial GetCompletedCommandSerial() const final override;
         Serial GetLastSubmittedCommandSerial() const final override;
-        void TickImpl() override;
+        MaybeError TickImpl() override;
 
         ResultOrError<std::unique_ptr<StagingBufferBase>> CreateStagingBuffer(size_t size) override;
         MaybeError CopyFromStagingToBuffer(StagingBufferBase* source,

src/dawn_native/vulkan/SwapChainVk.cpp

@@ -46,7 +46,7 @@ namespace dawn_native { namespace vulkan {
         return new Texture(ToBackend(GetDevice()), descriptor, nativeTexture);
     }
 
-    void SwapChain::OnBeforePresent(TextureBase* texture) {
+    MaybeError SwapChain::OnBeforePresent(TextureBase* texture) {
         Device* device = ToBackend(GetDevice());
 
         // Perform the necessary pipeline barriers for the texture to be used with the usage
@@ -55,6 +55,8 @@ namespace dawn_native { namespace vulkan {
         ToBackend(texture)->TransitionUsageNow(recordingContext, mTextureUsage);
 
         device->SubmitPendingCommands();
+
+        return {};
     }
 
 }}  // namespace dawn_native::vulkan

src/dawn_native/vulkan/SwapChainVk.h

@@ -30,7 +30,7 @@ namespace dawn_native { namespace vulkan {
 
       protected:
         TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
-        void OnBeforePresent(TextureBase* texture) override;
+        MaybeError OnBeforePresent(TextureBase* texture) override;
 
       private:
         dawn::TextureUsage mTextureUsage;