From 6fee61ca9c96528b34e1df26c63e9e61db3c8ac3 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 10 Sep 2018 16:17:24 +0200 Subject: [PATCH] Make Dawn error macro more explicit and have an "error type" The error type will help distinguish between validation errors, context losts and others which should be handled differently. Take advantage of advantage of this to change DAWN_RETURN_ERROR to "return DAWN_FOO_ERROR" to have the return be more explicit. Also removes usage of DAWN_TRY_ASSERT for more explicit checks. Change-Id: Icbce16b0c8d8eb084b0af2fc132acee776909a36 --- .../templates/dawn_native/ValidationUtils.cpp | 4 +- src/dawn_native/BindGroupLayout.cpp | 14 ++++-- src/dawn_native/Buffer.cpp | 21 ++++---- src/dawn_native/CommandBuffer.cpp | 50 ++++++++++--------- src/dawn_native/CommandBufferStateTracker.cpp | 8 +-- src/dawn_native/ComputePipeline.cpp | 10 ++-- src/dawn_native/Error.cpp | 8 ++- src/dawn_native/Error.h | 30 ++++++----- src/dawn_native/ErrorData.cpp | 7 ++- src/dawn_native/ErrorData.h | 7 ++- src/dawn_native/PipelineLayout.cpp | 16 ++++-- src/dawn_native/Sampler.cpp | 4 +- src/dawn_native/ShaderModule.cpp | 6 ++- src/dawn_native/Texture.cpp | 7 ++- src/dawn_native/d3d12/PlatformFunctions.cpp | 6 +-- src/tests/unittests/ErrorTests.cpp | 20 ++++---- 16 files changed, 132 insertions(+), 86 deletions(-) diff --git a/generator/templates/dawn_native/ValidationUtils.cpp b/generator/templates/dawn_native/ValidationUtils.cpp index 9c5ef70a6c..ac98da478e 100644 --- a/generator/templates/dawn_native/ValidationUtils.cpp +++ b/generator/templates/dawn_native/ValidationUtils.cpp @@ -24,7 +24,7 @@ namespace dawn_native { return {}; {% endfor %} default: - DAWN_RETURN_ERROR("Invalid value for {{as_cType(type.name)}}"); + return DAWN_VALIDATION_ERROR("Invalid value for {{as_cType(type.name)}}"); } } @@ -35,7 +35,7 @@ namespace dawn_native { if ((value & static_cast(~{{type.full_mask}})) == 0) { return {}; } - DAWN_RETURN_ERROR("Invalid value for {{as_cType(type.name)}}"); + return DAWN_VALIDATION_ERROR("Invalid value for {{as_cType(type.name)}}"); } {% endfor %} diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index c02b300e77..af89f56f83 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -25,18 +25,22 @@ namespace dawn_native { MaybeError ValidateBindGroupLayoutDescriptor(DeviceBase*, const BindGroupLayoutDescriptor* descriptor) { - DAWN_TRY_ASSERT(descriptor->nextInChain == nullptr, "nextInChain must be nullptr"); + if (descriptor->nextInChain != nullptr) { + return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); + } std::bitset bindingsSet; for (uint32_t i = 0; i < descriptor->numBindings; ++i) { auto& binding = descriptor->bindings[i]; - DAWN_TRY_ASSERT(binding.binding <= kMaxBindingsPerGroup, - "some binding index exceeds the maximum value"); DAWN_TRY(ValidateShaderStageBit(binding.visibility)); DAWN_TRY(ValidateBindingType(binding.type)); - DAWN_TRY_ASSERT(!bindingsSet[binding.binding], - "some binding index was specified more than once"); + if (binding.binding > kMaxBindingsPerGroup) { + return DAWN_VALIDATION_ERROR("some binding index exceeds the maximum value"); + } + if (bindingsSet[binding.binding]) { + return DAWN_VALIDATION_ERROR("some binding index was specified more than once"); + } bindingsSet.set(binding.binding); } return {}; diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index d22b0e9908..db113d628e 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp @@ -24,7 +24,10 @@ namespace dawn_native { MaybeError ValidateBufferDescriptor(DeviceBase*, const BufferDescriptor* descriptor) { - DAWN_TRY_ASSERT(descriptor->nextInChain == nullptr, "nextInChain must be nullptr"); + if (descriptor->nextInChain != nullptr) { + return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); + } + DAWN_TRY(ValidateBufferUsageBit(descriptor->usage)); dawn::BufferUsageBit usage = descriptor->usage; @@ -32,13 +35,13 @@ namespace dawn_native { const dawn::BufferUsageBit kMapWriteAllowedUsages = dawn::BufferUsageBit::MapWrite | dawn::BufferUsageBit::TransferSrc; if (usage & dawn::BufferUsageBit::MapWrite && (usage & kMapWriteAllowedUsages) != usage) { - DAWN_RETURN_ERROR("Only TransferSrc is allowed with MapWrite"); + return DAWN_VALIDATION_ERROR("Only TransferSrc is allowed with MapWrite"); } const dawn::BufferUsageBit kMapReadAllowedUsages = dawn::BufferUsageBit::MapRead | dawn::BufferUsageBit::TransferDst; if (usage & dawn::BufferUsageBit::MapRead && (usage & kMapReadAllowedUsages) != usage) { - DAWN_RETURN_ERROR("Only TransferDst is allowed with MapRead"); + return DAWN_VALIDATION_ERROR("Only TransferDst is allowed with MapRead"); } return {}; @@ -166,11 +169,11 @@ namespace dawn_native { MaybeError BufferBase::ValidateSetSubData(uint32_t start, uint32_t count) const { // TODO(cwallez@chromium.org): check for overflows. if (start + count > GetSize()) { - DAWN_RETURN_ERROR("Buffer subdata out of range"); + return DAWN_VALIDATION_ERROR("Buffer subdata out of range"); } if (!(mUsage & dawn::BufferUsageBit::TransferDst)) { - DAWN_RETURN_ERROR("Buffer needs the transfer dst usage bit"); + return DAWN_VALIDATION_ERROR("Buffer needs the transfer dst usage bit"); } return {}; @@ -181,15 +184,15 @@ namespace dawn_native { dawn::BufferUsageBit requiredUsage) const { // TODO(cwallez@chromium.org): check for overflows. if (start + size > GetSize()) { - DAWN_RETURN_ERROR("Buffer map read out of range"); + return DAWN_VALIDATION_ERROR("Buffer map read out of range"); } if (mIsMapped) { - DAWN_RETURN_ERROR("Buffer already mapped"); + return DAWN_VALIDATION_ERROR("Buffer already mapped"); } if (!(mUsage & requiredUsage)) { - DAWN_RETURN_ERROR("Buffer needs the correct map usage bit"); + return DAWN_VALIDATION_ERROR("Buffer needs the correct map usage bit"); } return {}; @@ -197,7 +200,7 @@ namespace dawn_native { MaybeError BufferBase::ValidateUnmap() const { if (!mIsMapped) { - DAWN_RETURN_ERROR("Buffer wasn't mapped"); + return DAWN_VALIDATION_ERROR("Buffer wasn't mapped"); } return {}; diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp index dafbac29b5..a2f5b2c224 100644 --- a/src/dawn_native/CommandBuffer.cpp +++ b/src/dawn_native/CommandBuffer.cpp @@ -35,11 +35,11 @@ namespace dawn_native { MaybeError ValidateCopyLocationFitsInTexture(const TextureCopyLocation& location) { const TextureBase* texture = location.texture.Get(); if (location.level >= texture->GetNumMipLevels()) { - DAWN_RETURN_ERROR("Copy mip-level out of range"); + return DAWN_VALIDATION_ERROR("Copy mip-level out of range"); } if (location.slice >= texture->GetArrayLayers()) { - DAWN_RETURN_ERROR("Copy array-layer out of range"); + return DAWN_VALIDATION_ERROR("Copy array-layer out of range"); } // All texture dimensions are in uint32_t so by doing checks in uint64_t we avoid @@ -49,13 +49,13 @@ namespace dawn_native { (static_cast(texture->GetWidth()) >> level) || uint64_t(location.y) + uint64_t(location.height) > (static_cast(texture->GetHeight()) >> level)) { - DAWN_RETURN_ERROR("Copy would touch outside of the texture"); + return DAWN_VALIDATION_ERROR("Copy would touch outside of the texture"); } // TODO(cwallez@chromium.org): Check the depth bound differently for 2D arrays and 3D // textures if (location.z != 0 || location.depth != 1) { - DAWN_RETURN_ERROR("No support for z != 0 and depth != 1 for now"); + return DAWN_VALIDATION_ERROR("No support for z != 0 and depth != 1 for now"); } return {}; @@ -69,7 +69,7 @@ namespace dawn_native { MaybeError ValidateCopySizeFitsInBuffer(const BufferCopyLocation& location, uint32_t dataSize) { if (!FitsInBuffer(location.buffer.Get(), location.offset, dataSize)) { - DAWN_RETURN_ERROR("Copy would overflow the buffer"); + return DAWN_VALIDATION_ERROR("Copy would overflow the buffer"); } return {}; @@ -80,7 +80,7 @@ namespace dawn_native { uint32_t texelSize = static_cast(TextureFormatPixelSize(texture->GetFormat())); if (location.offset % texelSize != 0) { - DAWN_RETURN_ERROR("Buffer offset must be a multiple of the texel size"); + return DAWN_VALIDATION_ERROR("Buffer offset must be a multiple of the texel size"); } return {}; @@ -102,12 +102,13 @@ namespace dawn_native { MaybeError ValidateRowPitch(const TextureCopyLocation& location, uint32_t rowPitch) { if (rowPitch % kTextureRowPitchAlignment != 0) { - DAWN_RETURN_ERROR("Row pitch must be a multiple of 256"); + return DAWN_VALIDATION_ERROR("Row pitch must be a multiple of 256"); } uint32_t texelSize = TextureFormatPixelSize(location.texture.Get()->GetFormat()); if (rowPitch < location.width * texelSize) { - DAWN_RETURN_ERROR("Row pitch must not be less than the number of bytes per row"); + return DAWN_VALIDATION_ERROR( + "Row pitch must not be less than the number of bytes per row"); } return {}; @@ -116,7 +117,7 @@ namespace dawn_native { MaybeError ValidateCanUseAs(BufferBase* buffer, dawn::BufferUsageBit usage) { ASSERT(HasZeroOrOneBits(usage)); if (!(buffer->GetUsage() & usage)) { - DAWN_RETURN_ERROR("buffer doesn't have the required usage."); + return DAWN_VALIDATION_ERROR("buffer doesn't have the required usage."); } return {}; @@ -125,7 +126,7 @@ namespace dawn_native { MaybeError ValidateCanUseAs(TextureBase* texture, dawn::TextureUsageBit usage) { ASSERT(HasZeroOrOneBits(usage)); if (!(texture->GetUsage() & usage)) { - DAWN_RETURN_ERROR("texture doesn't have the required usage."); + return DAWN_VALIDATION_ERROR("texture doesn't have the required usage."); } return {}; @@ -172,7 +173,8 @@ namespace dawn_native { MaybeError ValidateUsages(PassType pass) const { // Storage resources cannot be used twice in the same compute pass if (pass == PassType::Compute && mStorageUsedMultipleTimes) { - DAWN_RETURN_ERROR("Storage resource used multiple times in compute pass"); + return DAWN_VALIDATION_ERROR( + "Storage resource used multiple times in compute pass"); } // Buffers can only be used as single-write or multiple read. @@ -181,14 +183,14 @@ namespace dawn_native { dawn::BufferUsageBit usage = it.second; if (usage & ~buffer->GetUsage()) { - DAWN_RETURN_ERROR("Buffer missing usage for the pass"); + return DAWN_VALIDATION_ERROR("Buffer missing usage for the pass"); } bool readOnly = (usage & kReadOnlyBufferUsages) == usage; bool singleUse = dawn::HasZeroOrOneBits(usage); if (!readOnly && !singleUse) { - DAWN_RETURN_ERROR( + return DAWN_VALIDATION_ERROR( "Buffer used as writeable usage and another usage in pass"); } } @@ -200,13 +202,14 @@ namespace dawn_native { dawn::TextureUsageBit usage = it.second; if (usage & ~texture->GetUsage()) { - DAWN_RETURN_ERROR("Texture missing usage for the pass"); + return DAWN_VALIDATION_ERROR("Texture missing usage for the pass"); } // For textures the only read-only usage in a pass is Sampled, so checking the // usage constraint simplifies to checking a single usage bit is set. if (!dawn::HasZeroOrOneBits(it.second)) { - DAWN_RETURN_ERROR("Texture used with more than one usage in pass"); + return DAWN_VALIDATION_ERROR( + "Texture used with more than one usage in pass"); } } @@ -386,7 +389,7 @@ namespace dawn_native { } break; default: - DAWN_RETURN_ERROR("Command disallowed outside of a pass"); + return DAWN_VALIDATION_ERROR("Command disallowed outside of a pass"); } } @@ -426,7 +429,7 @@ namespace dawn_native { // recorded because it impacts the size of an allocation in the // CommandAllocator. if (cmd->stages & ~dawn::ShaderStageBit::Compute) { - DAWN_RETURN_ERROR( + return DAWN_VALIDATION_ERROR( "SetPushConstants stage must be compute or 0 in compute passes"); } } break; @@ -439,11 +442,11 @@ namespace dawn_native { } break; default: - DAWN_RETURN_ERROR("Command disallowed inside a compute pass"); + return DAWN_VALIDATION_ERROR("Command disallowed inside a compute pass"); } } - DAWN_RETURN_ERROR("Unfinished compute pass"); + return DAWN_VALIDATION_ERROR("Unfinished compute pass"); } MaybeError CommandBufferBuilder::ValidateRenderPass(RenderPassDescriptorBase* renderPass) { @@ -487,7 +490,8 @@ namespace dawn_native { RenderPipelineBase* pipeline = cmd->pipeline.Get(); if (!pipeline->IsCompatibleWith(renderPass)) { - DAWN_RETURN_ERROR("Pipeline is incompatible with this render pass"); + return DAWN_VALIDATION_ERROR( + "Pipeline is incompatible with this render pass"); } persistentState.SetRenderPipeline(pipeline); @@ -501,7 +505,7 @@ namespace dawn_native { // CommandAllocator. if (cmd->stages & ~(dawn::ShaderStageBit::Vertex | dawn::ShaderStageBit::Fragment)) { - DAWN_RETURN_ERROR( + return DAWN_VALIDATION_ERROR( "SetPushConstants stage must be a subset of (vertex|fragment) in " "render passes"); } @@ -545,11 +549,11 @@ namespace dawn_native { } break; default: - DAWN_RETURN_ERROR("Command disallowed inside a render pass"); + return DAWN_VALIDATION_ERROR("Command disallowed inside a render pass"); } } - DAWN_RETURN_ERROR("Unfinished render pass"); + return DAWN_VALIDATION_ERROR("Unfinished render pass"); } // Implementation of the API's command recording methods diff --git a/src/dawn_native/CommandBufferStateTracker.cpp b/src/dawn_native/CommandBufferStateTracker.cpp index 25ea3f3176..44799ac109 100644 --- a/src/dawn_native/CommandBufferStateTracker.cpp +++ b/src/dawn_native/CommandBufferStateTracker.cpp @@ -117,19 +117,19 @@ namespace dawn_native { ASSERT(aspects.any()); if (aspects[VALIDATION_ASPECT_INDEX_BUFFER]) { - DAWN_RETURN_ERROR("Missing index buffer"); + return DAWN_VALIDATION_ERROR("Missing index buffer"); } if (aspects[VALIDATION_ASPECT_VERTEX_BUFFERS]) { - DAWN_RETURN_ERROR("Missing vertex buffer"); + return DAWN_VALIDATION_ERROR("Missing vertex buffer"); } if (aspects[VALIDATION_ASPECT_BIND_GROUPS]) { - DAWN_RETURN_ERROR("Missing bind group"); + return DAWN_VALIDATION_ERROR("Missing bind group"); } if (aspects[VALIDATION_ASPECT_PIPELINE]) { - DAWN_RETURN_ERROR("Missing pipeline"); + return DAWN_VALIDATION_ERROR("Missing pipeline"); } UNREACHABLE(); diff --git a/src/dawn_native/ComputePipeline.cpp b/src/dawn_native/ComputePipeline.cpp index 8829996460..6e961f19f0 100644 --- a/src/dawn_native/ComputePipeline.cpp +++ b/src/dawn_native/ComputePipeline.cpp @@ -20,18 +20,20 @@ namespace dawn_native { MaybeError ValidateComputePipelineDescriptor(DeviceBase*, const ComputePipelineDescriptor* descriptor) { - DAWN_TRY_ASSERT(descriptor->nextInChain == nullptr, "nextInChain must be nullptr"); + if (descriptor->nextInChain != nullptr) { + return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); + } if (descriptor->entryPoint != std::string("main")) { - DAWN_RETURN_ERROR("Currently the entry point has to be main()"); + return DAWN_VALIDATION_ERROR("Currently the entry point has to be main()"); } if (descriptor->module->GetExecutionModel() != dawn::ShaderStage::Compute) { - DAWN_RETURN_ERROR("Setting module with wrong execution model"); + return DAWN_VALIDATION_ERROR("Setting module with wrong execution model"); } if (!descriptor->module->IsCompatibleWithPipelineLayout(descriptor->layout)) { - DAWN_RETURN_ERROR("Stage not compatible with layout"); + return DAWN_VALIDATION_ERROR("Stage not compatible with layout"); } return {}; diff --git a/src/dawn_native/Error.cpp b/src/dawn_native/Error.cpp index 06e029063c..9467c3a039 100644 --- a/src/dawn_native/Error.cpp +++ b/src/dawn_native/Error.cpp @@ -18,8 +18,12 @@ namespace dawn_native { - ErrorData* MakeError(const char* message, const char* file, const char* function, int line) { - ErrorData* error = new ErrorData(message); + ErrorData* MakeError(ErrorType type, + const char* message, + const char* file, + const char* function, + int line) { + ErrorData* error = new ErrorData(type, message); error->AppendBacktrace(file, function, line); return error; } diff --git a/src/dawn_native/Error.h b/src/dawn_native/Error.h index 3c2ea46073..f1736319da 100644 --- a/src/dawn_native/Error.h +++ b/src/dawn_native/Error.h @@ -23,6 +23,11 @@ namespace dawn_native { // file to avoid having all files including headers like and class ErrorData; + enum class ErrorType : uint32_t { + Validation, + ContextLost, + }; + // MaybeError and ResultOrError are meant to be used as return value for function that are not // expected to, but might fail. The handling of error is potentially much slower than successes. using MaybeError = Result; @@ -35,16 +40,13 @@ namespace dawn_native { // return SomethingOfTypeT; // for ResultOrError // // Returning an error is done via: - // DAWN_RETURN_ERROR("My error message"); -#define DAWN_RETURN_ERROR(MESSAGE) return MakeError(MESSAGE, __FILE__, __func__, __LINE__) -#define DAWN_TRY_ASSERT(EXPR, MESSAGE) \ - { \ - if (!(EXPR)) { \ - DAWN_RETURN_ERROR(MESSAGE); \ - } \ - } \ - for (;;) \ - break + // return DAWN_MAKE_ERROR(errorType, "My error message"); + // + // but shorthand version for specific error types are preferred: + // return DAWN_VALIDATION_ERROR("My error message"); +#define DAWN_MAKE_ERROR(TYPE, MESSAGE) MakeError(TYPE, MESSAGE, __FILE__, __func__, __LINE__) +#define DAWN_VALIDATION_ERROR(MESSAGE) DAWN_MAKE_ERROR(ErrorType::Validation, MESSAGE) +#define DAWN_CONTEXT_LOST_ERROR(MESSAGE) DAWN_MAKE_ERROR(ErrorType::ContextLost, MESSAGE) #define DAWN_CONCAT1(x, y) x##y #define DAWN_CONCAT2(x, y) DAWN_CONCAT1(x, y) @@ -83,8 +85,12 @@ namespace dawn_native { // Implementation detail of DAWN_TRY and DAWN_TRY_ASSIGN's adding to the Error's backtrace. void AppendBacktrace(ErrorData* error, const char* file, const char* function, int line); - // Implementation detail of DAWN_RETURN_ERROR - ErrorData* MakeError(const char* message, const char* file, const char* function, int line); + // Implementation detail of DAWN_MAKE_ERROR + ErrorData* MakeError(ErrorType type, + const char* message, + const char* file, + const char* function, + int line); } // namespace dawn_native diff --git a/src/dawn_native/ErrorData.cpp b/src/dawn_native/ErrorData.cpp index cd8c3be077..77a0e3f9dc 100644 --- a/src/dawn_native/ErrorData.cpp +++ b/src/dawn_native/ErrorData.cpp @@ -18,7 +18,8 @@ namespace dawn_native { ErrorData::ErrorData() = default; - ErrorData::ErrorData(std::string message) : mMessage(std::move(message)) { + ErrorData::ErrorData(ErrorType type, std::string message) + : mType(type), mMessage(std::move(message)) { } void ErrorData::AppendBacktrace(const char* file, const char* function, int line) { @@ -30,6 +31,10 @@ namespace dawn_native { mBacktrace.push_back(std::move(record)); } + ErrorType ErrorData::GetType() const { + return mType; + } + const std::string& ErrorData::GetMessage() const { return mMessage; } diff --git a/src/dawn_native/ErrorData.h b/src/dawn_native/ErrorData.h index 534ce5cd2a..0152003483 100644 --- a/src/dawn_native/ErrorData.h +++ b/src/dawn_native/ErrorData.h @@ -15,15 +15,18 @@ #ifndef DAWNNATIVE_ERRORDATA_H_ #define DAWNNATIVE_ERRORDATA_H_ +#include #include #include namespace dawn_native { + enum class ErrorType : uint32_t; + class ErrorData { public: ErrorData(); - ErrorData(std::string message); + ErrorData(ErrorType type, std::string message); struct BacktraceRecord { const char* file; @@ -32,10 +35,12 @@ namespace dawn_native { }; void AppendBacktrace(const char* file, const char* function, int line); + ErrorType GetType() const; const std::string& GetMessage() const; const std::vector& GetBacktrace() const; private: + ErrorType mType; std::string mMessage; std::vector mBacktrace; }; diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp index fd04bf1d40..3ade2fcbf9 100644 --- a/src/dawn_native/PipelineLayout.cpp +++ b/src/dawn_native/PipelineLayout.cpp @@ -22,12 +22,18 @@ namespace dawn_native { MaybeError ValidatePipelineLayoutDescriptor(DeviceBase*, const PipelineLayoutDescriptor* descriptor) { - DAWN_TRY_ASSERT(descriptor->nextInChain == nullptr, "nextInChain must be nullptr"); - DAWN_TRY_ASSERT(descriptor->numBindGroupLayouts <= kMaxBindGroups, - "too many bind group layouts"); + if (descriptor->nextInChain != nullptr) { + return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); + } + + if (descriptor->numBindGroupLayouts > kMaxBindGroups) { + return DAWN_VALIDATION_ERROR("too many bind group layouts"); + } + for (uint32_t i = 0; i < descriptor->numBindGroupLayouts; ++i) { - DAWN_TRY_ASSERT(descriptor->bindGroupLayouts[i] != nullptr, - "bind group layouts may not be null"); + if (descriptor->bindGroupLayouts[i] == nullptr) { + return DAWN_VALIDATION_ERROR("bind group layouts may not be null"); + } } return {}; } diff --git a/src/dawn_native/Sampler.cpp b/src/dawn_native/Sampler.cpp index 853104319c..1c3c76b582 100644 --- a/src/dawn_native/Sampler.cpp +++ b/src/dawn_native/Sampler.cpp @@ -20,7 +20,9 @@ namespace dawn_native { MaybeError ValidateSamplerDescriptor(DeviceBase*, const SamplerDescriptor* descriptor) { - DAWN_TRY_ASSERT(descriptor->nextInChain == nullptr, "nextInChain must be nullptr"); + if (descriptor->nextInChain != nullptr) { + return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); + } DAWN_TRY(ValidateFilterMode(descriptor->minFilter)); DAWN_TRY(ValidateFilterMode(descriptor->magFilter)); DAWN_TRY(ValidateFilterMode(descriptor->mipmapFilter)); diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index faf9551ba9..21ce155fc2 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -26,7 +26,9 @@ namespace dawn_native { MaybeError ValidateShaderModuleDescriptor(DeviceBase*, const ShaderModuleDescriptor* descriptor) { - DAWN_TRY_ASSERT(descriptor->nextInChain == nullptr, "nextInChain must be nullptr"); + if (descriptor->nextInChain != nullptr) { + return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); + } spvtools::SpirvTools spirvTools(SPV_ENV_WEBGPU_0); @@ -55,7 +57,7 @@ namespace dawn_native { }); if (!spirvTools.Validate(descriptor->code, descriptor->codeSize)) { - DAWN_RETURN_ERROR(errorStream.str().c_str()); + return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } return {}; diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp index e0ffa79b32..a438e69e6c 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -20,7 +20,10 @@ namespace dawn_native { MaybeError ValidateTextureDescriptor(DeviceBase*, const TextureDescriptor* descriptor) { - DAWN_TRY_ASSERT(descriptor->nextInChain == nullptr, "nextInChain must be nullptr"); + if (descriptor->nextInChain != nullptr) { + return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); + } + DAWN_TRY(ValidateTextureUsageBit(descriptor->usage)); DAWN_TRY(ValidateTextureDimension(descriptor->dimension)); DAWN_TRY(ValidateTextureFormat(descriptor->format)); @@ -28,7 +31,7 @@ namespace dawn_native { // TODO(jiawei.shao@intel.com): check stuff based on the dimension if (descriptor->width == 0 || descriptor->height == 0 || descriptor->depth == 0 || descriptor->arrayLayer == 0 || descriptor->mipLevel == 0) { - DAWN_RETURN_ERROR("Cannot create an empty texture"); + return DAWN_VALIDATION_ERROR("Cannot create an empty texture"); } return {}; diff --git a/src/dawn_native/d3d12/PlatformFunctions.cpp b/src/dawn_native/d3d12/PlatformFunctions.cpp index 6486d42732..5834a2a931 100644 --- a/src/dawn_native/d3d12/PlatformFunctions.cpp +++ b/src/dawn_native/d3d12/PlatformFunctions.cpp @@ -44,7 +44,7 @@ namespace dawn_native { namespace d3d12 { "D3D12SerializeVersionedRootSignature", &error) || !mD3D12Lib.GetProc(&d3d12CreateVersionedRootSignatureDeserializer, "D3D12CreateVersionedRootSignatureDeserializer", &error)) { - DAWN_RETURN_ERROR(error.c_str()); + return DAWN_CONTEXT_LOST_ERROR(error.c_str()); } return {}; @@ -55,7 +55,7 @@ namespace dawn_native { namespace d3d12 { if (!mDXGILib.Open("dxgi.dll", &error) || !mDXGILib.GetProc(&dxgiGetDebugInterface1, "DXGIGetDebugInterface1", &error) || !mDXGILib.GetProc(&createDxgiFactory2, "CreateDXGIFactory2", &error)) { - DAWN_RETURN_ERROR(error.c_str()); + return DAWN_CONTEXT_LOST_ERROR(error.c_str()); } return {}; @@ -65,7 +65,7 @@ namespace dawn_native { namespace d3d12 { std::string error; if (!mD3DCompilerLib.Open("d3dcompiler_47.dll", &error) || !mD3DCompilerLib.GetProc(&d3dCompile, "D3DCompile", &error)) { - DAWN_RETURN_ERROR(error.c_str()); + return DAWN_CONTEXT_LOST_ERROR(error.c_str()); } return {}; diff --git a/src/tests/unittests/ErrorTests.cpp b/src/tests/unittests/ErrorTests.cpp index e95bbcf18e..a5aec6ce21 100644 --- a/src/tests/unittests/ErrorTests.cpp +++ b/src/tests/unittests/ErrorTests.cpp @@ -34,10 +34,10 @@ TEST(ErrorTests, Error_Success) { ASSERT_TRUE(result.IsSuccess()); } -// Check returning an error MaybeError with DAWN_RETURN_ERROR +// Check returning an error MaybeError with "return DAWN_VALIDATION_ERROR" TEST(ErrorTests, Error_Error) { auto ReturnError = []() -> MaybeError { - DAWN_RETURN_ERROR(dummyErrorMessage); + return DAWN_VALIDATION_ERROR(dummyErrorMessage); }; MaybeError result = ReturnError(); @@ -59,10 +59,10 @@ TEST(ErrorTests, ResultOrError_Success) { ASSERT_EQ(result.AcquireSuccess(), &dummySuccess); } -// Check returning an error ResultOrError with DAWN_RETURN_ERROR +// Check returning an error ResultOrError with "return DAWN_VALIDATION_ERROR" TEST(ErrorTests, ResultOrError_Error) { auto ReturnError = []() -> ResultOrError { - DAWN_RETURN_ERROR(dummyErrorMessage); + return DAWN_VALIDATION_ERROR(dummyErrorMessage); }; ResultOrError result = ReturnError(); @@ -96,7 +96,7 @@ TEST(ErrorTests, TRY_Success) { // Check DAWN_TRY handles errors correctly. TEST(ErrorTests, TRY_Error) { auto ReturnError = []() -> MaybeError { - DAWN_RETURN_ERROR(dummyErrorMessage); + return DAWN_VALIDATION_ERROR(dummyErrorMessage); }; auto Try = [ReturnError]() -> MaybeError { @@ -117,7 +117,7 @@ TEST(ErrorTests, TRY_Error) { // Check DAWN_TRY adds to the backtrace. TEST(ErrorTests, TRY_AddsToBacktrace) { auto ReturnError = []() -> MaybeError { - DAWN_RETURN_ERROR(dummyErrorMessage); + return DAWN_VALIDATION_ERROR(dummyErrorMessage); }; auto SingleTry = [ReturnError]() -> MaybeError { @@ -172,7 +172,7 @@ TEST(ErrorTests, TRY_RESULT_Success) { // Check DAWN_TRY_ASSIGN handles errors correctly. TEST(ErrorTests, TRY_RESULT_Error) { auto ReturnError = []() -> ResultOrError { - DAWN_RETURN_ERROR(dummyErrorMessage); + return DAWN_VALIDATION_ERROR(dummyErrorMessage); }; auto Try = [ReturnError]() -> ResultOrError { @@ -196,7 +196,7 @@ TEST(ErrorTests, TRY_RESULT_Error) { // Check DAWN_TRY_ASSIGN adds to the backtrace. TEST(ErrorTests, TRY_RESULT_AddsToBacktrace) { auto ReturnError = []() -> ResultOrError { - DAWN_RETURN_ERROR(dummyErrorMessage); + return DAWN_VALIDATION_ERROR(dummyErrorMessage); }; auto SingleTry = [ReturnError]() -> ResultOrError { @@ -227,7 +227,7 @@ TEST(ErrorTests, TRY_RESULT_AddsToBacktrace) { // Check a ResultOrError can be DAWN_TRY_ASSIGNED in a function that returns an Error TEST(ErrorTests, TRY_RESULT_ConversionToError) { auto ReturnError = []() -> ResultOrError { - DAWN_RETURN_ERROR(dummyErrorMessage); + return DAWN_VALIDATION_ERROR(dummyErrorMessage); }; auto Try = [ReturnError]() -> MaybeError { @@ -250,7 +250,7 @@ TEST(ErrorTests, TRY_RESULT_ConversionToError) { // Check DAWN_TRY handles errors correctly. TEST(ErrorTests, TRY_ConversionToErrorOrResult) { auto ReturnError = []() -> MaybeError { - DAWN_RETURN_ERROR(dummyErrorMessage); + return DAWN_VALIDATION_ERROR(dummyErrorMessage); }; auto Try = [ReturnError]() -> ResultOrError{