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>
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 2a6b915..0f9bd64 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -536,7 +536,9 @@
// Other Device API methods
void DeviceBase::Tick() {
- TickImpl();
+ if (ConsumedError(TickImpl()))
+ return;
+
{
auto deferredResults = std::move(mDeferredCreateBufferMappedAsyncResults);
for (const auto& deferred : deferredResults) {
diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h
index a17d65a..4ed2f629 100644
--- a/src/dawn_native/Device.h
+++ b/src/dawn_native/Device.h
@@ -81,7 +81,7 @@
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
diff --git a/src/dawn_native/SwapChain.cpp b/src/dawn_native/SwapChain.cpp
index d5f685a..8194fe1 100644
--- a/src/dawn_native/SwapChain.cpp
+++ b/src/dawn_native/SwapChain.cpp
@@ -32,7 +32,7 @@
UNREACHABLE();
}
- void OnBeforePresent(TextureBase* texture) override {
+ MaybeError OnBeforePresent(TextureBase* texture) override {
UNREACHABLE();
}
};
@@ -127,7 +127,8 @@
}
ASSERT(!IsError());
- OnBeforePresent(texture);
+ if (GetDevice()->ConsumedError(OnBeforePresent(texture)))
+ return;
mImplementation.Present(mImplementation.userData);
}
diff --git a/src/dawn_native/SwapChain.h b/src/dawn_native/SwapChain.h
index 8b0e9fd..c9b6502 100644
--- a/src/dawn_native/SwapChain.h
+++ b/src/dawn_native/SwapChain.h
@@ -47,7 +47,7 @@
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,
diff --git a/src/dawn_native/d3d12/DeviceD3D12.cpp b/src/dawn_native/d3d12/DeviceD3D12.cpp
index c45f8fe..651b366 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.cpp
+++ b/src/dawn_native/d3d12/DeviceD3D12.cpp
@@ -109,7 +109,10 @@
}
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 @@
return mLastSubmittedSerial + 1;
}
- void Device::TickImpl() {
+ MaybeError Device::TickImpl() {
// Perform cleanup operations to free unused objects
mCompletedSerial = mFence->GetCompletedValue();
@@ -222,8 +225,10 @@
mDescriptorHeapAllocator->Deallocate(mCompletedSerial);
mMapRequestTracker->Tick(mCompletedSerial);
mUsedComObjectRefs.ClearUpTo(mCompletedSerial);
- ExecuteCommandList(nullptr);
+ DAWN_TRY(ExecuteCommandList(nullptr));
NextSerial();
+
+ return {};
}
void Device::NextSerial() {
@@ -243,13 +248,18 @@
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 @@
mCommandQueue->ExecuteCommandLists(numLists, d3d12CommandLists.data());
mPendingCommands.commandList.Reset();
}
+
+ return {};
}
ResultOrError<BindGroupBase*> Device::CreateBindGroupImpl(
diff --git a/src/dawn_native/d3d12/DeviceD3D12.h b/src/dawn_native/d3d12/DeviceD3D12.h
index a250fad..e1a2fe2 100644
--- a/src/dawn_native/d3d12/DeviceD3D12.h
+++ b/src/dawn_native/d3d12/DeviceD3D12.h
@@ -53,7 +53,7 @@
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 @@
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,
diff --git a/src/dawn_native/d3d12/QueueD3D12.cpp b/src/dawn_native/d3d12/QueueD3D12.cpp
index c9d24b15..acd24b8 100644
--- a/src/dawn_native/d3d12/QueueD3D12.cpp
+++ b/src/dawn_native/d3d12/QueueD3D12.cpp
@@ -33,7 +33,7 @@
}
ASSERT_SUCCESS(mCommandList->Close());
- device->ExecuteCommandList(mCommandList.Get());
+ DAWN_TRY(device->ExecuteCommandList(mCommandList.Get()));
device->NextSerial();
return {};
diff --git a/src/dawn_native/d3d12/SwapChainD3D12.cpp b/src/dawn_native/d3d12/SwapChainD3D12.cpp
index 8d67231..49cd6d5 100644
--- a/src/dawn_native/d3d12/SwapChainD3D12.cpp
+++ b/src/dawn_native/d3d12/SwapChainD3D12.cpp
@@ -48,13 +48,15 @@
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
diff --git a/src/dawn_native/d3d12/SwapChainD3D12.h b/src/dawn_native/d3d12/SwapChainD3D12.h
index 833ba48..151994c 100644
--- a/src/dawn_native/d3d12/SwapChainD3D12.h
+++ b/src/dawn_native/d3d12/SwapChainD3D12.h
@@ -28,7 +28,7 @@
protected:
TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
- void OnBeforePresent(TextureBase* texture) override;
+ MaybeError OnBeforePresent(TextureBase* texture) override;
dawn::TextureUsage mTextureUsage;
};
diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h
index 4eff03f..5d8c671 100644
--- a/src/dawn_native/metal/DeviceMTL.h
+++ b/src/dawn_native/metal/DeviceMTL.h
@@ -43,7 +43,7 @@
Serial GetCompletedCommandSerial() const final override;
Serial GetLastSubmittedCommandSerial() const final override;
- void TickImpl() override;
+ MaybeError TickImpl() override;
id<MTLDevice> GetMTLDevice();
id<MTLCommandQueue> GetMTLQueue();
diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm
index a62aad8..04e9d5f 100644
--- a/src/dawn_native/metal/DeviceMTL.mm
+++ b/src/dawn_native/metal/DeviceMTL.mm
@@ -151,7 +151,7 @@
return mLastSubmittedSerial + 1;
}
- void Device::TickImpl() {
+ MaybeError Device::TickImpl() {
Serial completedSerial = GetCompletedCommandSerial();
mDynamicUploader->Deallocate(completedSerial);
@@ -165,6 +165,8 @@
mCompletedSerial++;
mLastSubmittedSerial++;
}
+
+ return {};
}
id<MTLDevice> Device::GetMTLDevice() {
diff --git a/src/dawn_native/metal/SwapChainMTL.h b/src/dawn_native/metal/SwapChainMTL.h
index 063add6..5141ea7 100644
--- a/src/dawn_native/metal/SwapChainMTL.h
+++ b/src/dawn_native/metal/SwapChainMTL.h
@@ -28,7 +28,7 @@
protected:
TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
- void OnBeforePresent(TextureBase* texture) override;
+ MaybeError OnBeforePresent(TextureBase* texture) override;
};
}} // namespace dawn_native::metal
diff --git a/src/dawn_native/metal/SwapChainMTL.mm b/src/dawn_native/metal/SwapChainMTL.mm
index c955212..92458a2 100644
--- a/src/dawn_native/metal/SwapChainMTL.mm
+++ b/src/dawn_native/metal/SwapChainMTL.mm
@@ -46,7 +46,8 @@
return new Texture(ToBackend(GetDevice()), descriptor, nativeTexture);
}
- void SwapChain::OnBeforePresent(TextureBase*) {
+ MaybeError SwapChain::OnBeforePresent(TextureBase*) {
+ return {};
}
}} // namespace dawn_native::metal
diff --git a/src/dawn_native/null/DeviceNull.cpp b/src/dawn_native/null/DeviceNull.cpp
index 07c296f..08e39b2 100644
--- a/src/dawn_native/null/DeviceNull.cpp
+++ b/src/dawn_native/null/DeviceNull.cpp
@@ -197,8 +197,9 @@
return mLastSubmittedSerial + 1;
}
- void Device::TickImpl() {
+ MaybeError Device::TickImpl() {
SubmitPendingOperations();
+ return {};
}
void Device::AddPendingOperation(std::unique_ptr<PendingOperation> operation) {
@@ -338,7 +339,8 @@
return GetDevice()->CreateTexture(descriptor);
}
- void SwapChain::OnBeforePresent(TextureBase*) {
+ MaybeError SwapChain::OnBeforePresent(TextureBase*) {
+ return {};
}
// NativeSwapChainImpl
diff --git a/src/dawn_native/null/DeviceNull.h b/src/dawn_native/null/DeviceNull.h
index f0664d6..ef98719 100644
--- a/src/dawn_native/null/DeviceNull.h
+++ b/src/dawn_native/null/DeviceNull.h
@@ -92,7 +92,7 @@
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 @@
protected:
TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
- void OnBeforePresent(TextureBase*) override;
+ MaybeError OnBeforePresent(TextureBase*) override;
};
class NativeSwapChainImpl {
diff --git a/src/dawn_native/opengl/DeviceGL.cpp b/src/dawn_native/opengl/DeviceGL.cpp
index 0283722..a306e74 100644
--- a/src/dawn_native/opengl/DeviceGL.cpp
+++ b/src/dawn_native/opengl/DeviceGL.cpp
@@ -132,8 +132,9 @@
return mLastSubmittedSerial + 1;
}
- void Device::TickImpl() {
+ MaybeError Device::TickImpl() {
CheckPassedFences();
+ return {};
}
void Device::CheckPassedFences() {
diff --git a/src/dawn_native/opengl/DeviceGL.h b/src/dawn_native/opengl/DeviceGL.h
index bfbdc27..5bafedd 100644
--- a/src/dawn_native/opengl/DeviceGL.h
+++ b/src/dawn_native/opengl/DeviceGL.h
@@ -53,7 +53,7 @@
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,
diff --git a/src/dawn_native/opengl/SwapChainGL.cpp b/src/dawn_native/opengl/SwapChainGL.cpp
index e988bc4..bbd7074 100644
--- a/src/dawn_native/opengl/SwapChainGL.cpp
+++ b/src/dawn_native/opengl/SwapChainGL.cpp
@@ -44,7 +44,8 @@
TextureBase::TextureState::OwnedExternal);
}
- void SwapChain::OnBeforePresent(TextureBase*) {
+ MaybeError SwapChain::OnBeforePresent(TextureBase*) {
+ return {};
}
}} // namespace dawn_native::opengl
diff --git a/src/dawn_native/opengl/SwapChainGL.h b/src/dawn_native/opengl/SwapChainGL.h
index 9b76514..d4df7d3 100644
--- a/src/dawn_native/opengl/SwapChainGL.h
+++ b/src/dawn_native/opengl/SwapChainGL.h
@@ -30,7 +30,7 @@
protected:
TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
- void OnBeforePresent(TextureBase* texture) override;
+ MaybeError OnBeforePresent(TextureBase* texture) override;
};
}} // namespace dawn_native::opengl
diff --git a/src/dawn_native/vulkan/DeviceVk.cpp b/src/dawn_native/vulkan/DeviceVk.cpp
index e05cef4..e8581aa 100644
--- a/src/dawn_native/vulkan/DeviceVk.cpp
+++ b/src/dawn_native/vulkan/DeviceVk.cpp
@@ -209,7 +209,7 @@
return mLastSubmittedSerial + 1;
}
- void Device::TickImpl() {
+ MaybeError Device::TickImpl() {
CheckPassedFences();
RecycleCompletedCommands();
@@ -231,6 +231,8 @@
mCompletedSerial++;
mLastSubmittedSerial++;
}
+
+ return {};
}
VkInstance Device::GetVkInstance() const {
diff --git a/src/dawn_native/vulkan/DeviceVk.h b/src/dawn_native/vulkan/DeviceVk.h
index 529d75a..f4e7288 100644
--- a/src/dawn_native/vulkan/DeviceVk.h
+++ b/src/dawn_native/vulkan/DeviceVk.h
@@ -83,7 +83,7 @@
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,
diff --git a/src/dawn_native/vulkan/SwapChainVk.cpp b/src/dawn_native/vulkan/SwapChainVk.cpp
index 570760d..e9bcaf7 100644
--- a/src/dawn_native/vulkan/SwapChainVk.cpp
+++ b/src/dawn_native/vulkan/SwapChainVk.cpp
@@ -46,7 +46,7 @@
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 @@
ToBackend(texture)->TransitionUsageNow(recordingContext, mTextureUsage);
device->SubmitPendingCommands();
+
+ return {};
}
}} // namespace dawn_native::vulkan
diff --git a/src/dawn_native/vulkan/SwapChainVk.h b/src/dawn_native/vulkan/SwapChainVk.h
index 190346c..2e9db00 100644
--- a/src/dawn_native/vulkan/SwapChainVk.h
+++ b/src/dawn_native/vulkan/SwapChainVk.h
@@ -30,7 +30,7 @@
protected:
TextureBase* GetNextTextureImpl(const TextureDescriptor* descriptor) override;
- void OnBeforePresent(TextureBase* texture) override;
+ MaybeError OnBeforePresent(TextureBase* texture) override;
private:
dawn::TextureUsage mTextureUsage;
