diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp index c7124e5627..338d515e75 100644 --- a/src/dawn/native/Buffer.cpp +++ b/src/dawn/native/Buffer.cpp @@ -506,9 +506,7 @@ void BufferBase::APIUnmap() { if (GetDevice()->ConsumedError(ValidateUnmap(), "calling %s.Unmap().", this)) { return; } - if (GetDevice()->ConsumedError(Unmap(), "calling %s.Unmap().", this)) { - return; - } + DAWN_UNUSED(GetDevice()->ConsumedError(Unmap(), "calling %s.Unmap().", this)); } MaybeError BufferBase::Unmap() { diff --git a/src/dawn/native/ComputePassEncoder.cpp b/src/dawn/native/ComputePassEncoder.cpp index d3c6a5cceb..63505c7a44 100644 --- a/src/dawn/native/ComputePassEncoder.cpp +++ b/src/dawn/native/ComputePassEncoder.cpp @@ -151,7 +151,7 @@ ObjectType ComputePassEncoder::GetType() const { void ComputePassEncoder::APIEnd() { if (mEnded && IsValidationEnabled()) { - GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("%s was already ended.", this)); + GetDevice()->HandleError(DAWN_VALIDATION_ERROR("%s was already ended.", this)); return; } diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index 28956d0b12..53356e1da4 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -469,6 +469,7 @@ void DeviceBase::APIDestroy() { void DeviceBase::HandleError(std::unique_ptr error, InternalErrorType additionalAllowedErrors, WGPUDeviceLostReason lost_reason) { + AppendDebugLayerMessages(error.get()); InternalErrorType allowedErrors = InternalErrorType::Validation | InternalErrorType::DeviceLost | additionalAllowedErrors; InternalErrorType type = error->GetType(); @@ -545,7 +546,6 @@ void DeviceBase::HandleError(std::unique_ptr error, void DeviceBase::ConsumeError(std::unique_ptr error, InternalErrorType additionalAllowedErrors) { ASSERT(error != nullptr); - AppendDebugLayerMessages(error.get()); HandleError(std::move(error), additionalAllowedErrors); } @@ -1203,21 +1203,24 @@ BufferBase* DeviceBase::APICreateErrorBuffer(const BufferDescriptor* desc) { // The validation errors on BufferDescriptor should be prior to any OOM errors when // MapppedAtCreation == false. MaybeError maybeError = ValidateBufferDescriptor(this, &fakeDescriptor); - if (maybeError.IsError()) { - ConsumedError(maybeError.AcquireError(), InternalErrorType::OutOfMemory, - "calling %s.CreateBuffer(%s).", this, desc); - } else { - const DawnBufferDescriptorErrorInfoFromWireClient* clientErrorInfo = nullptr; - FindInChain(desc->nextInChain, &clientErrorInfo); - if (clientErrorInfo != nullptr && clientErrorInfo->outOfMemory) { - ConsumedError(DAWN_OUT_OF_MEMORY_ERROR("Failed to allocate memory for buffer mapping"), - InternalErrorType::OutOfMemory); - } - } // Set the size of the error buffer to 0 as this function is called only when an OOM happens at // the client side. fakeDescriptor.size = 0; + + if (maybeError.IsError()) { + std::unique_ptr error = maybeError.AcquireError(); + error->AppendContext("calling %s.CreateBuffer(%s).", this, desc); + HandleError(std::move(error), InternalErrorType::OutOfMemory); + } else { + const DawnBufferDescriptorErrorInfoFromWireClient* clientErrorInfo = nullptr; + FindInChain(desc->nextInChain, &clientErrorInfo); + if (clientErrorInfo != nullptr && clientErrorInfo->outOfMemory) { + HandleError(DAWN_OUT_OF_MEMORY_ERROR("Failed to allocate memory for buffer mapping"), + InternalErrorType::OutOfMemory); + } + } + return BufferBase::MakeError(this, &fakeDescriptor); } @@ -1401,7 +1404,7 @@ void DeviceBase::APIInjectError(wgpu::ErrorType type, const char* message) { } void DeviceBase::APIValidateTextureDescriptor(const TextureDescriptor* desc) { - ConsumedError(ValidateTextureDescriptor(this, desc)); + DAWN_UNUSED(ConsumedError(ValidateTextureDescriptor(this, desc))); } QueueBase* DeviceBase::GetQueue() const { diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h index ba3c7c754d..0507bc2d18 100644 --- a/src/dawn/native/Device.h +++ b/src/dawn/native/Device.h @@ -76,8 +76,12 @@ class DeviceBase : public RefCountedWithExternalCount { InternalErrorType additionalAllowedErrors = InternalErrorType::None, WGPUDeviceLostReason lost_reason = WGPUDeviceLostReason_Undefined); - bool ConsumedError(MaybeError maybeError, - InternalErrorType additionalAllowedErrors = InternalErrorType::None) { + // Variants of ConsumedError must use the returned boolean to handle failure cases since an + // error may cause a device loss and further execution may be undefined. This is especially + // true for the ResultOrError variants. + [[nodiscard]] bool ConsumedError( + MaybeError maybeError, + InternalErrorType additionalAllowedErrors = InternalErrorType::None) { if (DAWN_UNLIKELY(maybeError.IsError())) { ConsumeError(maybeError.AcquireError(), additionalAllowedErrors); return true; @@ -86,9 +90,10 @@ class DeviceBase : public RefCountedWithExternalCount { } template - bool ConsumedError(ResultOrError resultOrError, - T* result, - InternalErrorType additionalAllowedErrors = InternalErrorType::None) { + [[nodiscard]] bool ConsumedError( + ResultOrError resultOrError, + T* result, + InternalErrorType additionalAllowedErrors = InternalErrorType::None) { if (DAWN_UNLIKELY(resultOrError.IsError())) { ConsumeError(resultOrError.AcquireError(), additionalAllowedErrors); return true; @@ -98,10 +103,10 @@ class DeviceBase : public RefCountedWithExternalCount { } template - bool ConsumedError(MaybeError maybeError, - InternalErrorType additionalAllowedErrors, - const char* formatStr, - const Args&... args) { + [[nodiscard]] bool ConsumedError(MaybeError maybeError, + InternalErrorType additionalAllowedErrors, + const char* formatStr, + const Args&... args) { if (DAWN_UNLIKELY(maybeError.IsError())) { std::unique_ptr error = maybeError.AcquireError(); if (error->GetType() == InternalErrorType::Validation) { @@ -114,17 +119,18 @@ class DeviceBase : public RefCountedWithExternalCount { } template - bool ConsumedError(MaybeError maybeError, const char* formatStr, Args&&... args) { - return ConsumedError(std::move(maybeError), InternalErrorType::None, formatStr, - std::forward(args)...); + [[nodiscard]] bool ConsumedError(MaybeError maybeError, + const char* formatStr, + const Args&... args) { + return ConsumedError(std::move(maybeError), InternalErrorType::None, formatStr, args...); } template - bool ConsumedError(ResultOrError resultOrError, - T* result, - InternalErrorType additionalAllowedErrors, - const char* formatStr, - const Args&... args) { + [[nodiscard]] bool ConsumedError(ResultOrError resultOrError, + T* result, + InternalErrorType additionalAllowedErrors, + const char* formatStr, + const Args&... args) { if (DAWN_UNLIKELY(resultOrError.IsError())) { std::unique_ptr error = resultOrError.AcquireError(); if (error->GetType() == InternalErrorType::Validation) { @@ -138,12 +144,12 @@ class DeviceBase : public RefCountedWithExternalCount { } template - bool ConsumedError(ResultOrError resultOrError, - T* result, - const char* formatStr, - Args&&... args) { + [[nodiscard]] bool ConsumedError(ResultOrError resultOrError, + T* result, + const char* formatStr, + const Args&... args) { return ConsumedError(std::move(resultOrError), result, InternalErrorType::None, formatStr, - std::forward(args)...); + args...); } MaybeError ValidateObject(const ApiObjectBase* object) const; diff --git a/src/dawn/native/Queue.cpp b/src/dawn/native/Queue.cpp index a8d92e304b..926e13a83b 100644 --- a/src/dawn/native/Queue.cpp +++ b/src/dawn/native/Queue.cpp @@ -281,7 +281,7 @@ void QueueBase::APIWriteBuffer(BufferBase* buffer, uint64_t bufferOffset, const void* data, size_t size) { - GetDevice()->ConsumedError(WriteBuffer(buffer, bufferOffset, data, size)); + DAWN_UNUSED(GetDevice()->ConsumedError(WriteBuffer(buffer, bufferOffset, data, size))); } MaybeError QueueBase::WriteBuffer(BufferBase* buffer, @@ -322,8 +322,8 @@ void QueueBase::APIWriteTexture(const ImageCopyTexture* destination, size_t dataSize, const TextureDataLayout* dataLayout, const Extent3D* writeSize) { - GetDevice()->ConsumedError( - WriteTextureInternal(destination, data, dataSize, *dataLayout, writeSize)); + DAWN_UNUSED(GetDevice()->ConsumedError( + WriteTextureInternal(destination, data, dataSize, *dataLayout, writeSize))); } MaybeError QueueBase::WriteTextureInternal(const ImageCopyTexture* destination, @@ -389,16 +389,16 @@ void QueueBase::APICopyTextureForBrowser(const ImageCopyTexture* source, const ImageCopyTexture* destination, const Extent3D* copySize, const CopyTextureForBrowserOptions* options) { - GetDevice()->ConsumedError( - CopyTextureForBrowserInternal(source, destination, copySize, options)); + DAWN_UNUSED(GetDevice()->ConsumedError( + CopyTextureForBrowserInternal(source, destination, copySize, options))); } void QueueBase::APICopyExternalTextureForBrowser(const ImageCopyExternalTexture* source, const ImageCopyTexture* destination, const Extent3D* copySize, const CopyTextureForBrowserOptions* options) { - GetDevice()->ConsumedError( - CopyExternalTextureForBrowserInternal(source, destination, copySize, options)); + DAWN_UNUSED(GetDevice()->ConsumedError( + CopyExternalTextureForBrowserInternal(source, destination, copySize, options))); } MaybeError QueueBase::CopyTextureForBrowserInternal(const ImageCopyTexture* source, diff --git a/src/dawn/native/RenderPassEncoder.cpp b/src/dawn/native/RenderPassEncoder.cpp index 3b5a0624d8..5b3e409658 100644 --- a/src/dawn/native/RenderPassEncoder.cpp +++ b/src/dawn/native/RenderPassEncoder.cpp @@ -137,7 +137,7 @@ void RenderPassEncoder::TrackQueryAvailability(QuerySetBase* querySet, uint32_t void RenderPassEncoder::APIEnd() { if (mEnded && IsValidationEnabled()) { - GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("%s was already ended.", this)); + GetDevice()->HandleError(DAWN_VALIDATION_ERROR("%s was already ended.", this)); return; } diff --git a/src/dawn/native/SwapChain.cpp b/src/dawn/native/SwapChain.cpp index 3e1d3de721..a43067fb26 100644 --- a/src/dawn/native/SwapChain.cpp +++ b/src/dawn/native/SwapChain.cpp @@ -35,16 +35,16 @@ class ErrorSwapChain final : public SwapChainBase { wgpu::TextureUsage allowedUsage, uint32_t width, uint32_t height) override { - GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("%s is an error swapchain.", this)); + GetDevice()->HandleError(DAWN_VALIDATION_ERROR("%s is an error swapchain.", this)); } TextureViewBase* APIGetCurrentTextureView() override { - GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("%s is an error swapchain.", this)); + GetDevice()->HandleError(DAWN_VALIDATION_ERROR("%s is an error swapchain.", this)); return TextureViewBase::MakeError(GetDevice()); } void APIPresent() override { - GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("%s is an error swapchain.", this)); + GetDevice()->HandleError(DAWN_VALIDATION_ERROR("%s is an error swapchain.", this)); } }; @@ -300,7 +300,7 @@ void NewSwapChainBase::APIConfigure(wgpu::TextureFormat format, wgpu::TextureUsage allowedUsage, uint32_t width, uint32_t height) { - GetDevice()->ConsumedError( + GetDevice()->HandleError( DAWN_VALIDATION_ERROR("Configure is invalid for surface-based swapchains.")); } diff --git a/src/dawn/tests/unittests/validation/DeviceValidationTests.cpp b/src/dawn/tests/unittests/validation/DeviceValidationTests.cpp index e1c9e0b781..041ca96d85 100644 --- a/src/dawn/tests/unittests/validation/DeviceValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/DeviceValidationTests.cpp @@ -226,6 +226,6 @@ TEST_F(DeviceTickValidationTest, DestroyDeviceBeforeInternalTick) { ExpectDeviceDestruction(); device.Destroy(); dawn::native::DeviceBase* nativeDevice = dawn::native::FromAPI(device.Get()); - ASSERT_DEVICE_ERROR(nativeDevice->ConsumedError(nativeDevice->Tick()), + ASSERT_DEVICE_ERROR(EXPECT_TRUE(nativeDevice->ConsumedError(nativeDevice->Tick())), HasSubstr("[Device] is lost.")); }