From a594f8fdb4d78c06ae1cf5fa4adbcc1210af9b4d Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Wed, 13 Feb 2019 13:09:18 +0000 Subject: [PATCH] WebGPU error handling 1: Return error objects on errors. Dawn used to return "nullptr" when an error happened while creating an object. To match WebGPU we want to return valid pointers but to "error" objects. This commit implements WebGPU error handling for all "descriptorized" objects and changes the nullptr error checks into "ValidateObject" checks. This method is used both to check that the object isn't an error, but also that all objects in a function call are from the same device. New validation is added to objects with methods (apart from Device) so they check they aren't error objects. A large number of ASSERTs were added to check that frontend objects aren't accessed when they are errors, so that missing validation would hit the asserts instead of crashing randomly. The bind group validation tests were modified to test the behavior with both nullptrs and error objects. Future commits will change the CommandBufferBuilder to not be a builder anymore but an "Encoder" instead with special-cased error handling. Then once all objects are descriptorized, the notion of builders will be removed from the code. BUG=dawn:8 Change-Id: I8647712d5de3deb0e99e3bc58f34496f67710edd Reviewed-on: https://dawn-review.googlesource.com/c/4360 Reviewed-by: Kai Ninomiya Commit-Queue: Corentin Wallez --- src/dawn_native/BindGroup.cpp | 43 +++++++++---- src/dawn_native/BindGroup.h | 4 ++ src/dawn_native/BindGroupLayout.cpp | 11 ++++ src/dawn_native/BindGroupLayout.h | 4 ++ src/dawn_native/Buffer.cpp | 54 ++++++++++++++++ src/dawn_native/Buffer.h | 6 +- src/dawn_native/CommandBuffer.cpp | 8 +++ src/dawn_native/ComputePassEncoder.cpp | 9 +-- src/dawn_native/ComputePipeline.cpp | 20 +++--- src/dawn_native/ComputePipeline.h | 5 ++ src/dawn_native/Device.cpp | 37 +++++++---- src/dawn_native/Device.h | 2 + src/dawn_native/Error.h | 45 ++++++------- src/dawn_native/Fence.cpp | 17 +++++ src/dawn_native/Fence.h | 7 +- src/dawn_native/ObjectBase.cpp | 9 ++- src/dawn_native/ObjectBase.h | 9 +++ src/dawn_native/Pipeline.cpp | 15 +++-- src/dawn_native/Pipeline.h | 8 +-- src/dawn_native/PipelineLayout.cpp | 20 ++++-- src/dawn_native/PipelineLayout.h | 4 ++ src/dawn_native/ProgrammablePassEncoder.cpp | 9 +-- src/dawn_native/Queue.cpp | 13 +++- src/dawn_native/RenderPassDescriptor.cpp | 8 +++ src/dawn_native/RenderPassEncoder.cpp | 20 ++---- src/dawn_native/RenderPipeline.cpp | 40 ++++++++---- src/dawn_native/RenderPipeline.h | 4 ++ src/dawn_native/Sampler.cpp | 9 +++ src/dawn_native/Sampler.h | 5 ++ src/dawn_native/ShaderModule.cpp | 19 ++++++ src/dawn_native/ShaderModule.h | 4 ++ src/dawn_native/SwapChain.cpp | 1 + src/dawn_native/Texture.cpp | 42 +++++++++++- src/dawn_native/Texture.h | 8 +++ .../validation/BindGroupValidationTests.cpp | 64 +++++++++++++++++++ 35 files changed, 471 insertions(+), 112 deletions(-) diff --git a/src/dawn_native/BindGroup.cpp b/src/dawn_native/BindGroup.cpp index 5a5a1778f7..8732c064cb 100644 --- a/src/dawn_native/BindGroup.cpp +++ b/src/dawn_native/BindGroup.cpp @@ -28,12 +28,14 @@ namespace dawn_native { // Helper functions to perform binding-type specific validation - MaybeError ValidateBufferBinding(const BindGroupBinding& binding, + MaybeError ValidateBufferBinding(const DeviceBase* device, + const BindGroupBinding& binding, dawn::BufferUsageBit requiredUsage) { if (binding.buffer == nullptr || binding.sampler != nullptr || binding.textureView != nullptr) { return DAWN_VALIDATION_ERROR("expected buffer binding"); } + DAWN_TRY(device->ValidateObject(binding.buffer)); uint32_t bufferSize = binding.buffer->GetSize(); if (binding.size > bufferSize) { @@ -58,12 +60,14 @@ namespace dawn_native { return {}; } - MaybeError ValidateTextureBinding(const BindGroupBinding& binding, + MaybeError ValidateTextureBinding(const DeviceBase* device, + const BindGroupBinding& binding, dawn::TextureUsageBit requiredUsage) { if (binding.textureView == nullptr || binding.sampler != nullptr || binding.buffer != nullptr) { return DAWN_VALIDATION_ERROR("expected texture binding"); } + DAWN_TRY(device->ValidateObject(binding.textureView)); if (!(binding.textureView->GetTexture()->GetUsage() & requiredUsage)) { return DAWN_VALIDATION_ERROR("texture binding usage mismatch"); @@ -72,24 +76,26 @@ namespace dawn_native { return {}; } - MaybeError ValidateSamplerBinding(const BindGroupBinding& binding) { + MaybeError ValidateSamplerBinding(const DeviceBase* device, + const BindGroupBinding& binding) { if (binding.sampler == nullptr || binding.textureView != nullptr || binding.buffer != nullptr) { return DAWN_VALIDATION_ERROR("expected sampler binding"); } + DAWN_TRY(device->ValidateObject(binding.sampler)); + return {}; } } // anonymous namespace - MaybeError ValidateBindGroupDescriptor(DeviceBase*, const BindGroupDescriptor* descriptor) { + MaybeError ValidateBindGroupDescriptor(DeviceBase* device, + const BindGroupDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); } - if (descriptor->layout == nullptr) { - return DAWN_VALIDATION_ERROR("layout cannot be null"); - } + DAWN_TRY(device->ValidateObject(descriptor->layout)); const BindGroupLayoutBase::LayoutBindingInfo& layoutInfo = descriptor->layout->GetBindingInfo(); @@ -120,16 +126,17 @@ namespace dawn_native { // Perform binding-type specific validation. switch (layoutInfo.types[bindingIndex]) { case dawn::BindingType::UniformBuffer: - DAWN_TRY(ValidateBufferBinding(binding, dawn::BufferUsageBit::Uniform)); + DAWN_TRY(ValidateBufferBinding(device, binding, dawn::BufferUsageBit::Uniform)); break; case dawn::BindingType::StorageBuffer: - DAWN_TRY(ValidateBufferBinding(binding, dawn::BufferUsageBit::Storage)); + DAWN_TRY(ValidateBufferBinding(device, binding, dawn::BufferUsageBit::Storage)); break; case dawn::BindingType::SampledTexture: - DAWN_TRY(ValidateTextureBinding(binding, dawn::TextureUsageBit::Sampled)); + DAWN_TRY( + ValidateTextureBinding(device, binding, dawn::TextureUsageBit::Sampled)); break; case dawn::BindingType::Sampler: - DAWN_TRY(ValidateSamplerBinding(binding)); + DAWN_TRY(ValidateSamplerBinding(device, binding)); break; } } @@ -179,11 +186,22 @@ namespace dawn_native { } } + BindGroupBase::BindGroupBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : ObjectBase(device, tag) { + } + + // static + BindGroupBase* BindGroupBase::MakeError(DeviceBase* device) { + return new BindGroupBase(device, ObjectBase::kError); + } + const BindGroupLayoutBase* BindGroupBase::GetLayout() const { + ASSERT(!IsError()); return mLayout.Get(); } BufferBinding BindGroupBase::GetBindingAsBufferBinding(size_t binding) { + ASSERT(!IsError()); ASSERT(binding < kMaxBindingsPerGroup); ASSERT(mLayout->GetBindingInfo().mask[binding]); ASSERT(mLayout->GetBindingInfo().types[binding] == dawn::BindingType::UniformBuffer || @@ -193,6 +211,7 @@ namespace dawn_native { } SamplerBase* BindGroupBase::GetBindingAsSampler(size_t binding) { + ASSERT(!IsError()); ASSERT(binding < kMaxBindingsPerGroup); ASSERT(mLayout->GetBindingInfo().mask[binding]); ASSERT(mLayout->GetBindingInfo().types[binding] == dawn::BindingType::Sampler); @@ -200,9 +219,11 @@ namespace dawn_native { } TextureViewBase* BindGroupBase::GetBindingAsTextureView(size_t binding) { + ASSERT(!IsError()); ASSERT(binding < kMaxBindingsPerGroup); ASSERT(mLayout->GetBindingInfo().mask[binding]); ASSERT(mLayout->GetBindingInfo().types[binding] == dawn::BindingType::SampledTexture); return reinterpret_cast(mBindings[binding].Get()); } + } // namespace dawn_native diff --git a/src/dawn_native/BindGroup.h b/src/dawn_native/BindGroup.h index 050747b107..32988ea970 100644 --- a/src/dawn_native/BindGroup.h +++ b/src/dawn_native/BindGroup.h @@ -42,12 +42,16 @@ namespace dawn_native { public: BindGroupBase(DeviceBase* device, const BindGroupDescriptor* descriptor); + static BindGroupBase* MakeError(DeviceBase* device); + const BindGroupLayoutBase* GetLayout() const; BufferBinding GetBindingAsBufferBinding(size_t binding); SamplerBase* GetBindingAsSampler(size_t binding); TextureViewBase* GetBindingAsTextureView(size_t binding); private: + BindGroupBase(DeviceBase* device, ObjectBase::ErrorTag tag); + Ref mLayout; std::array, kMaxBindingsPerGroup> mBindings; std::array mOffsets; diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 0098fa6708..9774340722 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -92,14 +92,25 @@ namespace dawn_native { } } + BindGroupLayoutBase::BindGroupLayoutBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : ObjectBase(device, tag), mIsBlueprint(true) { + } + BindGroupLayoutBase::~BindGroupLayoutBase() { // Do not uncache the actual cached object if we are a blueprint if (!mIsBlueprint) { + ASSERT(!IsError()); GetDevice()->UncacheBindGroupLayout(this); } } + // static + BindGroupLayoutBase* BindGroupLayoutBase::MakeError(DeviceBase* device) { + return new BindGroupLayoutBase(device, ObjectBase::kError); + } + const BindGroupLayoutBase::LayoutBindingInfo& BindGroupLayoutBase::GetBindingInfo() const { + ASSERT(!IsError()); return mBindingInfo; } diff --git a/src/dawn_native/BindGroupLayout.h b/src/dawn_native/BindGroupLayout.h index 3a1f4540ac..08f38faf61 100644 --- a/src/dawn_native/BindGroupLayout.h +++ b/src/dawn_native/BindGroupLayout.h @@ -37,6 +37,8 @@ namespace dawn_native { bool blueprint = false); ~BindGroupLayoutBase() override; + static BindGroupLayoutBase* MakeError(DeviceBase* device); + struct LayoutBindingInfo { std::array visibilities; std::array types; @@ -45,6 +47,8 @@ namespace dawn_native { const LayoutBindingInfo& GetBindingInfo() const; private: + BindGroupLayoutBase(DeviceBase* device, ObjectBase::ErrorTag tag); + LayoutBindingInfo mBindingInfo; bool mIsBlueprint = false; }; diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index 543d62e95a..97ba85a4d5 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp @@ -23,6 +23,33 @@ namespace dawn_native { + namespace { + + class ErrorBuffer : public BufferBase { + public: + ErrorBuffer(DeviceBase* device) : BufferBase(device, ObjectBase::kError) { + } + + private: + MaybeError SetSubDataImpl(uint32_t start, + uint32_t count, + const uint8_t* data) override { + UNREACHABLE(); + return {}; + } + void MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t size) override { + UNREACHABLE(); + } + void MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t size) override { + UNREACHABLE(); + } + void UnmapImpl() override { + UNREACHABLE(); + } + }; + + } // anonymous namespace + MaybeError ValidateBufferDescriptor(DeviceBase*, const BufferDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); @@ -53,22 +80,35 @@ namespace dawn_native { : ObjectBase(device), mSize(descriptor->size), mUsage(descriptor->usage) { } + BufferBase::BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag) : ObjectBase(device, tag) { + } + BufferBase::~BufferBase() { if (mIsMapped) { + ASSERT(!IsError()); CallMapReadCallback(mMapSerial, DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr); CallMapWriteCallback(mMapSerial, DAWN_BUFFER_MAP_ASYNC_STATUS_UNKNOWN, nullptr); } } + // static + BufferBase* BufferBase::MakeError(DeviceBase* device) { + return new ErrorBuffer(device); + } + uint32_t BufferBase::GetSize() const { + ASSERT(!IsError()); return mSize; } dawn::BufferUsageBit BufferBase::GetUsage() const { + ASSERT(!IsError()); return mUsage; } MaybeError BufferBase::ValidateCanUseInSubmitNow() const { + ASSERT(!IsError()); + if (mIsMapped) { return DAWN_VALIDATION_ERROR("Buffer used in a submit while mapped"); } @@ -78,6 +118,8 @@ namespace dawn_native { void BufferBase::CallMapReadCallback(uint32_t serial, dawnBufferMapAsyncStatus status, const void* pointer) { + ASSERT(!IsError()); + if (mMapReadCallback != nullptr && serial == mMapSerial) { ASSERT(mMapWriteCallback == nullptr); // Tag the callback as fired before firing it, otherwise it could fire a second time if @@ -91,6 +133,8 @@ namespace dawn_native { void BufferBase::CallMapWriteCallback(uint32_t serial, dawnBufferMapAsyncStatus status, void* pointer) { + ASSERT(!IsError()); + if (mMapWriteCallback != nullptr && serial == mMapSerial) { ASSERT(mMapReadCallback == nullptr); // Tag the callback as fired before firing it, otherwise it could fire a second time if @@ -105,6 +149,7 @@ namespace dawn_native { if (GetDevice()->ConsumedError(ValidateSetSubData(start, count))) { return; } + ASSERT(!IsError()); if (GetDevice()->ConsumedError(SetSubDataImpl(start, count, data))) { return; @@ -119,6 +164,7 @@ namespace dawn_native { callback(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); return; } + ASSERT(!IsError()); ASSERT(mMapWriteCallback == nullptr); @@ -139,6 +185,7 @@ namespace dawn_native { callback(DAWN_BUFFER_MAP_ASYNC_STATUS_ERROR, nullptr, userdata); return; } + ASSERT(!IsError()); ASSERT(mMapReadCallback == nullptr); @@ -155,6 +202,7 @@ namespace dawn_native { if (GetDevice()->ConsumedError(ValidateUnmap())) { return; } + ASSERT(!IsError()); // A map request can only be called once, so this will fire only if the request wasn't // completed before the Unmap @@ -168,6 +216,8 @@ namespace dawn_native { } MaybeError BufferBase::ValidateSetSubData(uint32_t start, uint32_t count) const { + DAWN_TRY(GetDevice()->ValidateObject(this)); + if (count > GetSize()) { return DAWN_VALIDATION_ERROR("Buffer subdata with too much data"); } @@ -187,6 +237,8 @@ namespace dawn_native { MaybeError BufferBase::ValidateMap(uint32_t start, uint32_t size, dawn::BufferUsageBit requiredUsage) const { + DAWN_TRY(GetDevice()->ValidateObject(this)); + if (size > GetSize()) { return DAWN_VALIDATION_ERROR("Buffer mapping with too big a region"); } @@ -208,6 +260,8 @@ namespace dawn_native { } MaybeError BufferBase::ValidateUnmap() const { + DAWN_TRY(GetDevice()->ValidateObject(this)); + if (!mIsMapped) { return DAWN_VALIDATION_ERROR("Buffer wasn't mapped"); } diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h index 4472cecbbc..8f6bc7cf0c 100644 --- a/src/dawn_native/Buffer.h +++ b/src/dawn_native/Buffer.h @@ -39,6 +39,8 @@ namespace dawn_native { BufferBase(DeviceBase* device, const BufferDescriptor* descriptor); ~BufferBase(); + static BufferBase* MakeError(DeviceBase* device); + uint32_t GetSize() const; dawn::BufferUsageBit GetUsage() const; @@ -57,6 +59,8 @@ namespace dawn_native { void Unmap(); protected: + BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag); + void CallMapReadCallback(uint32_t serial, dawnBufferMapAsyncStatus status, const void* pointer); @@ -74,7 +78,7 @@ namespace dawn_native { dawn::BufferUsageBit requiredUsage) const; MaybeError ValidateUnmap() const; - uint32_t mSize; + uint32_t mSize = 0; dawn::BufferUsageBit mUsage = dawn::BufferUsageBit::None; dawnBufferMapReadCallback mMapReadCallback = nullptr; diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp index 97b5678b88..db88ab3322 100644 --- a/src/dawn_native/CommandBuffer.cpp +++ b/src/dawn_native/CommandBuffer.cpp @@ -387,6 +387,8 @@ namespace dawn_native { case Command::CopyBufferToBuffer: { CopyBufferToBufferCmd* copy = mIterator.NextCommand(); + DAWN_TRY(GetDevice()->ValidateObject(copy->source.buffer.Get())); + DAWN_TRY(GetDevice()->ValidateObject(copy->destination.buffer.Get())); DAWN_TRY(ValidateCopySizeFitsInBuffer(copy->source, copy->size)); DAWN_TRY(ValidateCopySizeFitsInBuffer(copy->destination, copy->size)); @@ -402,6 +404,9 @@ namespace dawn_native { case Command::CopyBufferToTexture: { CopyBufferToTextureCmd* copy = mIterator.NextCommand(); + DAWN_TRY(GetDevice()->ValidateObject(copy->source.buffer.Get())); + DAWN_TRY(GetDevice()->ValidateObject(copy->destination.texture.Get())); + uint32_t bufferCopySize = 0; DAWN_TRY(ValidateRowPitch(copy->destination.texture->GetFormat(), copy->copySize, copy->source.rowPitch)); @@ -427,6 +432,9 @@ namespace dawn_native { case Command::CopyTextureToBuffer: { CopyTextureToBufferCmd* copy = mIterator.NextCommand(); + DAWN_TRY(GetDevice()->ValidateObject(copy->source.texture.Get())); + DAWN_TRY(GetDevice()->ValidateObject(copy->destination.buffer.Get())); + uint32_t bufferCopySize = 0; DAWN_TRY(ValidateRowPitch(copy->source.texture->GetFormat(), copy->copySize, copy->destination.rowPitch)); diff --git a/src/dawn_native/ComputePassEncoder.cpp b/src/dawn_native/ComputePassEncoder.cpp index bc2510c1ba..27a31ce73b 100644 --- a/src/dawn_native/ComputePassEncoder.cpp +++ b/src/dawn_native/ComputePassEncoder.cpp @@ -17,6 +17,7 @@ #include "dawn_native/CommandBuffer.h" #include "dawn_native/Commands.h" #include "dawn_native/ComputePipeline.h" +#include "dawn_native/Device.h" namespace dawn_native { @@ -39,12 +40,8 @@ namespace dawn_native { } void ComputePassEncoderBase::SetPipeline(ComputePipelineBase* pipeline) { - if (mTopLevelBuilder->ConsumedError(ValidateCanRecordCommands())) { - return; - } - - if (pipeline == nullptr) { - mTopLevelBuilder->HandleError("Pipeline cannot be null"); + if (mTopLevelBuilder->ConsumedError(ValidateCanRecordCommands()) || + mTopLevelBuilder->ConsumedError(GetDevice()->ValidateObject(pipeline))) { return; } diff --git a/src/dawn_native/ComputePipeline.cpp b/src/dawn_native/ComputePipeline.cpp index fb7c8618e8..c95115b458 100644 --- a/src/dawn_native/ComputePipeline.cpp +++ b/src/dawn_native/ComputePipeline.cpp @@ -18,19 +18,14 @@ namespace dawn_native { - MaybeError ValidateComputePipelineDescriptor(DeviceBase*, + MaybeError ValidateComputePipelineDescriptor(DeviceBase* device, const ComputePipelineDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); } - if (descriptor->module == nullptr) { - return DAWN_VALIDATION_ERROR("module cannot be null"); - } - - if (descriptor->layout == nullptr) { - return DAWN_VALIDATION_ERROR("layout cannot be null"); - } + DAWN_TRY(device->ValidateObject(descriptor->module)); + DAWN_TRY(device->ValidateObject(descriptor->layout)); if (descriptor->entryPoint != std::string("main")) { return DAWN_VALIDATION_ERROR("Currently the entry point has to be main()"); @@ -55,4 +50,13 @@ namespace dawn_native { ExtractModuleData(dawn::ShaderStage::Compute, descriptor->module); } + ComputePipelineBase::ComputePipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : PipelineBase(device, tag) { + } + + // static + ComputePipelineBase* ComputePipelineBase::MakeError(DeviceBase* device) { + return new ComputePipelineBase(device, ObjectBase::kError); + } + } // namespace dawn_native diff --git a/src/dawn_native/ComputePipeline.h b/src/dawn_native/ComputePipeline.h index 1cea676af8..c1450d3a9c 100644 --- a/src/dawn_native/ComputePipeline.h +++ b/src/dawn_native/ComputePipeline.h @@ -27,6 +27,11 @@ namespace dawn_native { class ComputePipelineBase : public PipelineBase { public: ComputePipelineBase(DeviceBase* device, const ComputePipelineDescriptor* descriptor); + + static ComputePipelineBase* MakeError(DeviceBase* device); + + private: + ComputePipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag); }; } // namespace dawn_native diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index 8b33268278..095fb0aaa0 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp @@ -70,6 +70,16 @@ namespace dawn_native { mErrorUserdata = userdata; } + MaybeError DeviceBase::ValidateObject(const ObjectBase* object) const { + if (DAWN_UNLIKELY(object->GetDevice() != this)) { + return DAWN_VALIDATION_ERROR("Object from a different device."); + } + if (DAWN_UNLIKELY(object->IsError())) { + return DAWN_VALIDATION_ERROR("Object is an error."); + } + return {}; + } + AdapterBase* DeviceBase::GetAdapter() const { return mAdapter; } @@ -108,7 +118,7 @@ namespace dawn_native { BindGroupBase* result = nullptr; if (ConsumedError(CreateBindGroupInternal(&result, descriptor))) { - return nullptr; + return BindGroupBase::MakeError(this); } return result; @@ -118,7 +128,7 @@ namespace dawn_native { BindGroupLayoutBase* result = nullptr; if (ConsumedError(CreateBindGroupLayoutInternal(&result, descriptor))) { - return nullptr; + return BindGroupLayoutBase::MakeError(this); } return result; @@ -127,7 +137,7 @@ namespace dawn_native { BufferBase* result = nullptr; if (ConsumedError(CreateBufferInternal(&result, descriptor))) { - return nullptr; + return BufferBase::MakeError(this); } return result; @@ -140,7 +150,7 @@ namespace dawn_native { ComputePipelineBase* result = nullptr; if (ConsumedError(CreateComputePipelineInternal(&result, descriptor))) { - return nullptr; + return ComputePipelineBase::MakeError(this); } return result; @@ -149,7 +159,7 @@ namespace dawn_native { FenceBase* result = nullptr; if (ConsumedError(CreateFenceInternal(&result, descriptor))) { - return nullptr; + return FenceBase::MakeError(this); } return result; @@ -162,7 +172,7 @@ namespace dawn_native { PipelineLayoutBase* result = nullptr; if (ConsumedError(CreatePipelineLayoutInternal(&result, descriptor))) { - return nullptr; + return PipelineLayoutBase::MakeError(this); } return result; @@ -171,6 +181,9 @@ namespace dawn_native { QueueBase* result = nullptr; if (ConsumedError(CreateQueueInternal(&result))) { + // If queue creation failure ever becomes possible, we should implement MakeError and + // friends for them. + UNREACHABLE(); return nullptr; } @@ -183,7 +196,7 @@ namespace dawn_native { SamplerBase* result = nullptr; if (ConsumedError(CreateSamplerInternal(&result, descriptor))) { - return nullptr; + return SamplerBase::MakeError(this); } return result; @@ -193,7 +206,7 @@ namespace dawn_native { RenderPipelineBase* result = nullptr; if (ConsumedError(CreateRenderPipelineInternal(&result, descriptor))) { - return nullptr; + return RenderPipelineBase::MakeError(this); } return result; @@ -202,7 +215,7 @@ namespace dawn_native { ShaderModuleBase* result = nullptr; if (ConsumedError(CreateShaderModuleInternal(&result, descriptor))) { - return nullptr; + return ShaderModuleBase::MakeError(this); } return result; @@ -214,8 +227,9 @@ namespace dawn_native { TextureBase* result = nullptr; if (ConsumedError(CreateTextureInternal(&result, descriptor))) { - return nullptr; + return TextureBase::MakeError(this); } + return result; } TextureViewBase* DeviceBase::CreateTextureView(TextureBase* texture, @@ -223,8 +237,9 @@ namespace dawn_native { TextureViewBase* result = nullptr; if (ConsumedError(CreateTextureViewInternal(&result, texture, descriptor))) { - return nullptr; + return TextureViewBase::MakeError(this); } + return result; } diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h index 83c40c4e6a..94f91d0dec 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h @@ -49,6 +49,8 @@ namespace dawn_native { return false; } + MaybeError ValidateObject(const ObjectBase* object) const; + AdapterBase* GetAdapter() const; // Used by autogenerated code, returns itself diff --git a/src/dawn_native/Error.h b/src/dawn_native/Error.h index f76e6382e9..9e073c27b2 100644 --- a/src/dawn_native/Error.h +++ b/src/dawn_native/Error.h @@ -43,7 +43,8 @@ namespace dawn_native { // // 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_MAKE_ERROR(TYPE, MESSAGE) \ + ::dawn_native::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_UNIMPLEMENTED_ERROR(MESSAGE) DAWN_MAKE_ERROR(ErrorType::Unimplemented, MESSAGE) @@ -55,31 +56,31 @@ namespace dawn_native { // When Errors aren't handled explicitly, calls to functions returning errors should be // wrapped in an DAWN_TRY. It will return the error if any, otherwise keep executing // the current function. -#define DAWN_TRY(EXPR) \ - { \ - auto DAWN_LOCAL_VAR = EXPR; \ - if (DAWN_UNLIKELY(DAWN_LOCAL_VAR.IsError())) { \ - ErrorData* error = DAWN_LOCAL_VAR.AcquireError(); \ - AppendBacktrace(error, __FILE__, __func__, __LINE__); \ - return {std::move(error)}; \ - } \ - } \ - for (;;) \ +#define DAWN_TRY(EXPR) \ + { \ + auto DAWN_LOCAL_VAR = EXPR; \ + if (DAWN_UNLIKELY(DAWN_LOCAL_VAR.IsError())) { \ + ErrorData* error = DAWN_LOCAL_VAR.AcquireError(); \ + ::dawn_native::AppendBacktrace(error, __FILE__, __func__, __LINE__); \ + return {std::move(error)}; \ + } \ + } \ + for (;;) \ break // DAWN_TRY_ASSIGN is the same as DAWN_TRY for ResultOrError and assigns the success value, if // any, to VAR. -#define DAWN_TRY_ASSIGN(VAR, EXPR) \ - { \ - auto DAWN_LOCAL_VAR = EXPR; \ - if (DAWN_UNLIKELY(DAWN_LOCAL_VAR.IsError())) { \ - ErrorData* error = DAWN_LOCAL_VAR.AcquireError(); \ - AppendBacktrace(error, __FILE__, __func__, __LINE__); \ - return {std::move(error)}; \ - } \ - VAR = DAWN_LOCAL_VAR.AcquireSuccess(); \ - } \ - for (;;) \ +#define DAWN_TRY_ASSIGN(VAR, EXPR) \ + { \ + auto DAWN_LOCAL_VAR = EXPR; \ + if (DAWN_UNLIKELY(DAWN_LOCAL_VAR.IsError())) { \ + ErrorData* error = DAWN_LOCAL_VAR.AcquireError(); \ + ::dawn_native::AppendBacktrace(error, __FILE__, __func__, __LINE__); \ + return {std::move(error)}; \ + } \ + VAR = DAWN_LOCAL_VAR.AcquireSuccess(); \ + } \ + for (;;) \ break // Implementation detail of DAWN_TRY and DAWN_TRY_ASSIGN's adding to the Error's backtrace. diff --git a/src/dawn_native/Fence.cpp b/src/dawn_native/Fence.cpp index 54ed9a9453..4ab4c7b0e0 100644 --- a/src/dawn_native/Fence.cpp +++ b/src/dawn_native/Fence.cpp @@ -38,14 +38,26 @@ namespace dawn_native { mCompletedValue(descriptor->initialValue) { } + FenceBase::FenceBase(DeviceBase* device, ObjectBase::ErrorTag tag) : ObjectBase(device, tag) { + } + FenceBase::~FenceBase() { for (auto& request : mRequests.IterateAll()) { + ASSERT(!IsError()); request.completionCallback(DAWN_FENCE_COMPLETION_STATUS_UNKNOWN, request.userdata); } mRequests.Clear(); } + // static + FenceBase* FenceBase::MakeError(DeviceBase* device) { + return new FenceBase(device, ObjectBase::kError); + } + uint64_t FenceBase::GetCompletedValue() const { + if (IsError()) { + return 0; + } return mCompletedValue; } @@ -56,6 +68,7 @@ namespace dawn_native { callback(DAWN_FENCE_COMPLETION_STATUS_ERROR, userdata); return; } + ASSERT(!IsError()); if (value <= mCompletedValue) { callback(DAWN_FENCE_COMPLETION_STATUS_SUCCESS, userdata); @@ -69,15 +82,18 @@ namespace dawn_native { } uint64_t FenceBase::GetSignaledValue() const { + ASSERT(!IsError()); return mSignalValue; } void FenceBase::SetSignaledValue(uint64_t signalValue) { + ASSERT(!IsError()); ASSERT(signalValue > mSignalValue); mSignalValue = signalValue; } void FenceBase::SetCompletedValue(uint64_t completedValue) { + ASSERT(!IsError()); ASSERT(completedValue <= mSignalValue); ASSERT(completedValue > mCompletedValue); mCompletedValue = completedValue; @@ -89,6 +105,7 @@ namespace dawn_native { } MaybeError FenceBase::ValidateOnCompletion(uint64_t value) const { + DAWN_TRY(GetDevice()->ValidateObject(this)); if (value > mSignalValue) { return DAWN_VALIDATION_ERROR("Value greater than fence signaled value"); } diff --git a/src/dawn_native/Fence.h b/src/dawn_native/Fence.h index e2f4c40f76..0b1c7d948f 100644 --- a/src/dawn_native/Fence.h +++ b/src/dawn_native/Fence.h @@ -33,12 +33,15 @@ namespace dawn_native { FenceBase(DeviceBase* device, const FenceDescriptor* descriptor); ~FenceBase(); + static FenceBase* MakeError(DeviceBase* device); + + uint64_t GetSignaledValue() const; + // Dawn API uint64_t GetCompletedValue() const; void OnCompletion(uint64_t value, dawn::FenceOnCompletionCallback callback, dawn::CallbackUserdata userdata); - uint64_t GetSignaledValue() const; protected: friend class QueueBase; @@ -47,6 +50,8 @@ namespace dawn_native { void SetCompletedValue(uint64_t completedValue); private: + FenceBase(DeviceBase* device, ObjectBase::ErrorTag tag); + MaybeError ValidateOnCompletion(uint64_t value) const; struct OnCompletionData { diff --git a/src/dawn_native/ObjectBase.cpp b/src/dawn_native/ObjectBase.cpp index 2d6b56f922..3e40af4de6 100644 --- a/src/dawn_native/ObjectBase.cpp +++ b/src/dawn_native/ObjectBase.cpp @@ -16,7 +16,10 @@ namespace dawn_native { - ObjectBase::ObjectBase(DeviceBase* device) : mDevice(device) { + ObjectBase::ObjectBase(DeviceBase* device) : mDevice(device), mIsError(false) { + } + + ObjectBase::ObjectBase(DeviceBase* device, ErrorTag) : mDevice(device), mIsError(true) { } ObjectBase::~ObjectBase() { @@ -26,4 +29,8 @@ namespace dawn_native { return mDevice; } + bool ObjectBase::IsError() const { + return mIsError; + } + } // namespace dawn_native diff --git a/src/dawn_native/ObjectBase.h b/src/dawn_native/ObjectBase.h index 019938a2c9..02dd7ec6ba 100644 --- a/src/dawn_native/ObjectBase.h +++ b/src/dawn_native/ObjectBase.h @@ -23,13 +23,22 @@ namespace dawn_native { class ObjectBase : public RefCounted { public: + struct ErrorTag {}; + static constexpr ErrorTag kError = {}; + ObjectBase(DeviceBase* device); + ObjectBase(DeviceBase* device, ErrorTag tag); virtual ~ObjectBase(); DeviceBase* GetDevice() const; + bool IsError() const; private: DeviceBase* mDevice; + // TODO(cwallez@chromium.org): This most likely adds 4 bytes to most Dawn objects, see if + // that bit can be hidden in the refcount once it is a single 64bit refcount. + // See https://bugs.chromium.org/p/dawn/issues/detail?id=105 + bool mIsError; }; } // namespace dawn_native diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp index fb2a3112e6..6ddb742143 100644 --- a/src/dawn_native/Pipeline.cpp +++ b/src/dawn_native/Pipeline.cpp @@ -26,10 +26,16 @@ namespace dawn_native { PipelineBase::PipelineBase(DeviceBase* device, PipelineLayoutBase* layout, dawn::ShaderStageBit stages) - : ObjectBase(device), mStageMask(stages), mLayout(layout), mDevice(device) { + : ObjectBase(device), mStageMask(stages), mLayout(layout) { + } + + PipelineBase::PipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : ObjectBase(device, tag) { } void PipelineBase::ExtractModuleData(dawn::ShaderStage stage, ShaderModuleBase* module) { + ASSERT(!IsError()); + PushConstantInfo* info = &mPushConstants[stage]; const auto& moduleInfo = module->GetPushConstants(); @@ -50,19 +56,18 @@ namespace dawn_native { const PipelineBase::PushConstantInfo& PipelineBase::GetPushConstants( dawn::ShaderStage stage) const { + ASSERT(!IsError()); return mPushConstants[stage]; } dawn::ShaderStageBit PipelineBase::GetStageMask() const { + ASSERT(!IsError()); return mStageMask; } PipelineLayoutBase* PipelineBase::GetLayout() { + ASSERT(!IsError()); return mLayout.Get(); } - DeviceBase* PipelineBase::GetDevice() const { - return mDevice; - } - } // namespace dawn_native diff --git a/src/dawn_native/Pipeline.h b/src/dawn_native/Pipeline.h index 47c1079607..cc1c542833 100644 --- a/src/dawn_native/Pipeline.h +++ b/src/dawn_native/Pipeline.h @@ -37,26 +37,24 @@ namespace dawn_native { class PipelineBase : public ObjectBase { public: - PipelineBase(DeviceBase* device, PipelineLayoutBase* layout, dawn::ShaderStageBit stages); - struct PushConstantInfo { std::bitset mask; std::array types; }; const PushConstantInfo& GetPushConstants(dawn::ShaderStage stage) const; dawn::ShaderStageBit GetStageMask() const; - PipelineLayoutBase* GetLayout(); - DeviceBase* GetDevice() const; protected: + PipelineBase(DeviceBase* device, PipelineLayoutBase* layout, dawn::ShaderStageBit stages); + PipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag); + void ExtractModuleData(dawn::ShaderStage stage, ShaderModuleBase* module); private: dawn::ShaderStageBit mStageMask; Ref mLayout; PerStage mPushConstants; - DeviceBase* mDevice; }; } // namespace dawn_native diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp index 96772bd0e8..720273a6d0 100644 --- a/src/dawn_native/PipelineLayout.cpp +++ b/src/dawn_native/PipelineLayout.cpp @@ -20,7 +20,7 @@ namespace dawn_native { - MaybeError ValidatePipelineLayoutDescriptor(DeviceBase*, + MaybeError ValidatePipelineLayoutDescriptor(DeviceBase* device, const PipelineLayoutDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); @@ -31,9 +31,7 @@ namespace dawn_native { } for (uint32_t i = 0; i < descriptor->numBindGroupLayouts; ++i) { - if (descriptor->bindGroupLayouts[i] == nullptr) { - return DAWN_VALIDATION_ERROR("bind group layouts may not be null"); - } + DAWN_TRY(device->ValidateObject(descriptor->bindGroupLayouts[i])); } return {}; } @@ -50,22 +48,36 @@ namespace dawn_native { } } + PipelineLayoutBase::PipelineLayoutBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : ObjectBase(device, tag) { + } + + // static + PipelineLayoutBase* PipelineLayoutBase::MakeError(DeviceBase* device) { + return new PipelineLayoutBase(device, ObjectBase::kError); + } + const BindGroupLayoutBase* PipelineLayoutBase::GetBindGroupLayout(size_t group) const { + ASSERT(!IsError()); ASSERT(group < kMaxBindGroups); ASSERT(mMask[group]); return mBindGroupLayouts[group].Get(); } const std::bitset PipelineLayoutBase::GetBindGroupLayoutsMask() const { + ASSERT(!IsError()); return mMask; } std::bitset PipelineLayoutBase::InheritedGroupsMask( const PipelineLayoutBase* other) const { + ASSERT(!IsError()); return {(1 << GroupsInheritUpTo(other)) - 1u}; } uint32_t PipelineLayoutBase::GroupsInheritUpTo(const PipelineLayoutBase* other) const { + ASSERT(!IsError()); + for (uint32_t i = 0; i < kMaxBindGroups; ++i) { if (!mMask[i] || mBindGroupLayouts[i].Get() != other->mBindGroupLayouts[i].Get()) { return i; diff --git a/src/dawn_native/PipelineLayout.h b/src/dawn_native/PipelineLayout.h index 0d5e932d19..aa1e3e5d55 100644 --- a/src/dawn_native/PipelineLayout.h +++ b/src/dawn_native/PipelineLayout.h @@ -36,6 +36,8 @@ namespace dawn_native { public: PipelineLayoutBase(DeviceBase* device, const PipelineLayoutDescriptor* descriptor); + static PipelineLayoutBase* MakeError(DeviceBase* device); + const BindGroupLayoutBase* GetBindGroupLayout(size_t group) const; const std::bitset GetBindGroupLayoutsMask() const; @@ -48,6 +50,8 @@ namespace dawn_native { uint32_t GroupsInheritUpTo(const PipelineLayoutBase* other) const; protected: + PipelineLayoutBase(DeviceBase* device, ObjectBase::ErrorTag tag); + BindGroupLayoutArray mBindGroupLayouts; std::bitset mMask; }; diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp index fdfcf42f68..bc9f19efb9 100644 --- a/src/dawn_native/ProgrammablePassEncoder.cpp +++ b/src/dawn_native/ProgrammablePassEncoder.cpp @@ -17,6 +17,7 @@ #include "dawn_native/BindGroup.h" #include "dawn_native/CommandBuffer.h" #include "dawn_native/Commands.h" +#include "dawn_native/Device.h" #include @@ -38,12 +39,8 @@ namespace dawn_native { } void ProgrammablePassEncoder::SetBindGroup(uint32_t groupIndex, BindGroupBase* group) { - if (mTopLevelBuilder->ConsumedError(ValidateCanRecordCommands())) { - return; - } - - if (group == nullptr) { - mTopLevelBuilder->HandleError("BindGroup cannot be null"); + if (mTopLevelBuilder->ConsumedError(ValidateCanRecordCommands()) || + mTopLevelBuilder->ConsumedError(GetDevice()->ValidateObject(group))) { return; } diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index 426c963c10..6fc9af92e4 100644 --- a/src/dawn_native/Queue.cpp +++ b/src/dawn_native/Queue.cpp @@ -32,6 +32,7 @@ namespace dawn_native { if (GetDevice()->ConsumedError(ValidateSubmit(numCommands, commands))) { return; } + ASSERT(!IsError()); SubmitImpl(numCommands, commands); } @@ -40,13 +41,20 @@ namespace dawn_native { if (GetDevice()->ConsumedError(ValidateSignal(fence, signalValue))) { return; } + ASSERT(!IsError()); fence->SetSignaledValue(signalValue); GetDevice()->GetFenceSignalTracker()->UpdateFenceOnComplete(fence, signalValue); } MaybeError QueueBase::ValidateSubmit(uint32_t numCommands, CommandBufferBase* const* commands) { + DAWN_TRY(GetDevice()->ValidateObject(this)); + for (uint32_t i = 0; i < numCommands; ++i) { + DAWN_TRY(GetDevice()->ValidateObject(commands[i])); + + // TODO(cwallez@chromium.org): Remove this once CommandBufferBuilder doesn't use the + // builder mechanism anymore. if (commands[i] == nullptr) { return DAWN_VALIDATION_ERROR("Command buffers cannot be null"); } @@ -74,9 +82,8 @@ namespace dawn_native { } MaybeError QueueBase::ValidateSignal(const FenceBase* fence, uint64_t signalValue) { - if (fence == nullptr) { - return DAWN_VALIDATION_ERROR("Fence cannot be null"); - } + DAWN_TRY(GetDevice()->ValidateObject(this)); + DAWN_TRY(GetDevice()->ValidateObject(fence)); if (signalValue <= fence->GetSignaledValue()) { return DAWN_VALIDATION_ERROR("Signal value less than or equal to fence signaled value"); diff --git a/src/dawn_native/RenderPassDescriptor.cpp b/src/dawn_native/RenderPassDescriptor.cpp index 3a40a83452..99730bacaa 100644 --- a/src/dawn_native/RenderPassDescriptor.cpp +++ b/src/dawn_native/RenderPassDescriptor.cpp @@ -164,6 +164,10 @@ namespace dawn_native { continue; } + // TODO(cwallez@chromium.org): Once RenderPassDescriptor doesn't use a builder, check + // that the textureView is a valid object. + // See https://bugs.chromium.org/p/dawn/issues/detail?id=6 + if (!IsColorRenderableTextureFormat(textureView->GetFormat())) { HandleError( "The format of the texture view used as color attachment is not color " @@ -190,6 +194,10 @@ namespace dawn_native { void RenderPassDescriptorBuilder::SetDepthStencilAttachment( const RenderPassDepthStencilAttachmentDescriptor* attachment) { TextureViewBase* textureView = attachment->attachment; + + // TODO(cwallez@chromium.org): Once RenderPassDescriptor doesn't use a builder, check that + // the textureView is a valid object. + // See https://bugs.chromium.org/p/dawn/issues/detail?id=6 if (textureView == nullptr) { HandleError("Texture view cannot be nullptr"); return; diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp index f48b053709..c392bb37fa 100644 --- a/src/dawn_native/RenderPassEncoder.cpp +++ b/src/dawn_native/RenderPassEncoder.cpp @@ -17,6 +17,7 @@ #include "dawn_native/Buffer.h" #include "dawn_native/CommandBuffer.h" #include "dawn_native/Commands.h" +#include "dawn_native/Device.h" #include "dawn_native/RenderPipeline.h" #include @@ -64,12 +65,8 @@ namespace dawn_native { } void RenderPassEncoderBase::SetPipeline(RenderPipelineBase* pipeline) { - if (mTopLevelBuilder->ConsumedError(ValidateCanRecordCommands())) { - return; - } - - if (pipeline == nullptr) { - mTopLevelBuilder->HandleError("Pipeline cannot be null"); + if (mTopLevelBuilder->ConsumedError(ValidateCanRecordCommands()) || + mTopLevelBuilder->ConsumedError(GetDevice()->ValidateObject(pipeline))) { return; } @@ -117,12 +114,8 @@ namespace dawn_native { } void RenderPassEncoderBase::SetIndexBuffer(BufferBase* buffer, uint32_t offset) { - if (mTopLevelBuilder->ConsumedError(ValidateCanRecordCommands())) { - return; - } - - if (buffer == nullptr) { - mTopLevelBuilder->HandleError("Buffer cannot be null"); + if (mTopLevelBuilder->ConsumedError(ValidateCanRecordCommands()) || + mTopLevelBuilder->ConsumedError(GetDevice()->ValidateObject(buffer))) { return; } @@ -141,8 +134,7 @@ namespace dawn_native { } for (size_t i = 0; i < count; ++i) { - if (buffers[i] == nullptr) { - mTopLevelBuilder->HandleError("Buffers cannot be null"); + if (mTopLevelBuilder->ConsumedError(GetDevice()->ValidateObject(buffers[i]))) { return; } } diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index b760939ede..19b9bd572d 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/src/dawn_native/RenderPipeline.cpp @@ -25,9 +25,12 @@ namespace dawn_native { // Helper functions namespace { - MaybeError ValidatePipelineStageDescriptor(const PipelineStageDescriptor* descriptor, + MaybeError ValidatePipelineStageDescriptor(DeviceBase* device, + const PipelineStageDescriptor* descriptor, const PipelineLayoutBase* layout, dawn::ShaderStage stage) { + DAWN_TRY(device->ValidateObject(descriptor->module)); + if (descriptor->entryPoint != std::string("main")) { return DAWN_VALIDATION_ERROR("Entry point must be \"main\""); } @@ -110,28 +113,22 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); } - if (descriptor->layout == nullptr) { - return DAWN_VALIDATION_ERROR("Layout must not be null"); - } + DAWN_TRY(device->ValidateObject(descriptor->layout)); if (descriptor->inputState == nullptr) { return DAWN_VALIDATION_ERROR("Input state must not be null"); } - if (descriptor->depthStencilState == nullptr) { - return DAWN_VALIDATION_ERROR("Depth stencil state must not be null"); - } - for (uint32_t i = 0; i < descriptor->numBlendStates; ++i) { DAWN_TRY(ValidateBlendStateDescriptor(&descriptor->blendStates[i])); } DAWN_TRY(ValidateIndexFormat(descriptor->indexFormat)); DAWN_TRY(ValidatePrimitiveTopology(descriptor->primitiveTopology)); - DAWN_TRY(ValidatePipelineStageDescriptor(descriptor->vertexStage, descriptor->layout, - dawn::ShaderStage::Vertex)); - DAWN_TRY(ValidatePipelineStageDescriptor(descriptor->fragmentStage, descriptor->layout, - dawn::ShaderStage::Fragment)); + DAWN_TRY(ValidatePipelineStageDescriptor(device, descriptor->vertexStage, + descriptor->layout, dawn::ShaderStage::Vertex)); + DAWN_TRY(ValidatePipelineStageDescriptor(device, descriptor->fragmentStage, + descriptor->layout, dawn::ShaderStage::Fragment)); DAWN_TRY(ValidateAttachmentsStateDescriptor(descriptor->attachmentsState)); if ((descriptor->vertexStage->module->GetUsedVertexAttributes() & @@ -206,46 +203,65 @@ namespace dawn_native { // attachment are set? } + RenderPipelineBase::RenderPipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : PipelineBase(device, tag) { + } + + // static + RenderPipelineBase* RenderPipelineBase::MakeError(DeviceBase* device) { + return new RenderPipelineBase(device, ObjectBase::kError); + } + const BlendStateDescriptor* RenderPipelineBase::GetBlendStateDescriptor( uint32_t attachmentSlot) { + ASSERT(!IsError()); ASSERT(attachmentSlot < mBlendStates.size()); return &mBlendStates[attachmentSlot]; } const DepthStencilStateDescriptor* RenderPipelineBase::GetDepthStencilStateDescriptor() { + ASSERT(!IsError()); return &mDepthStencilState; } dawn::IndexFormat RenderPipelineBase::GetIndexFormat() const { + ASSERT(!IsError()); return mIndexFormat; } InputStateBase* RenderPipelineBase::GetInputState() { + ASSERT(!IsError()); return mInputState.Get(); } dawn::PrimitiveTopology RenderPipelineBase::GetPrimitiveTopology() const { + ASSERT(!IsError()); return mPrimitiveTopology; } std::bitset RenderPipelineBase::GetColorAttachmentsMask() const { + ASSERT(!IsError()); return mColorAttachmentsSet; } bool RenderPipelineBase::HasDepthStencilAttachment() const { + ASSERT(!IsError()); return mHasDepthStencilAttachment; } dawn::TextureFormat RenderPipelineBase::GetColorAttachmentFormat(uint32_t attachment) const { + ASSERT(!IsError()); return mColorAttachmentFormats[attachment]; } dawn::TextureFormat RenderPipelineBase::GetDepthStencilFormat() const { + ASSERT(!IsError()); ASSERT(mHasDepthStencilAttachment); return mDepthStencilFormat; } bool RenderPipelineBase::IsCompatibleWith(const RenderPassDescriptorBase* renderPass) const { + ASSERT(!IsError()); // TODO(cwallez@chromium.org): This is called on every SetPipeline command. Optimize it for // example by caching some "attachment compatibility" object that would make the // compatibility check a single pointer comparison. diff --git a/src/dawn_native/RenderPipeline.h b/src/dawn_native/RenderPipeline.h index 03d7df82fd..ab2da1b3cc 100644 --- a/src/dawn_native/RenderPipeline.h +++ b/src/dawn_native/RenderPipeline.h @@ -36,6 +36,8 @@ namespace dawn_native { public: RenderPipelineBase(DeviceBase* device, const RenderPipelineDescriptor* descriptor); + static RenderPipelineBase* MakeError(DeviceBase* device); + const BlendStateDescriptor* GetBlendStateDescriptor(uint32_t attachmentSlot); const DepthStencilStateDescriptor* GetDepthStencilStateDescriptor(); dawn::IndexFormat GetIndexFormat() const; @@ -52,6 +54,8 @@ namespace dawn_native { bool IsCompatibleWith(const RenderPassDescriptorBase* renderPass) const; private: + RenderPipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag); + DepthStencilStateDescriptor mDepthStencilState; dawn::IndexFormat mIndexFormat; Ref mInputState; diff --git a/src/dawn_native/Sampler.cpp b/src/dawn_native/Sampler.cpp index 3e688943ad..4d462a5862 100644 --- a/src/dawn_native/Sampler.cpp +++ b/src/dawn_native/Sampler.cpp @@ -49,4 +49,13 @@ namespace dawn_native { SamplerBase::SamplerBase(DeviceBase* device, const SamplerDescriptor*) : ObjectBase(device) { } + SamplerBase::SamplerBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : ObjectBase(device, tag) { + } + + // static + SamplerBase* SamplerBase::MakeError(DeviceBase* device) { + return new SamplerBase(device, ObjectBase::kError); + } + } // namespace dawn_native diff --git a/src/dawn_native/Sampler.h b/src/dawn_native/Sampler.h index a9d547a63c..cde32dd662 100644 --- a/src/dawn_native/Sampler.h +++ b/src/dawn_native/Sampler.h @@ -29,6 +29,11 @@ namespace dawn_native { class SamplerBase : public ObjectBase { public: SamplerBase(DeviceBase* device, const SamplerDescriptor* descriptor); + + static SamplerBase* MakeError(DeviceBase* device); + + private: + SamplerBase(DeviceBase* device, ObjectBase::ErrorTag tag); }; } // namespace dawn_native diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index 7a0e49e655..d32899fd9f 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -69,7 +69,18 @@ namespace dawn_native { : ObjectBase(device) { } + ShaderModuleBase::ShaderModuleBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : ObjectBase(device, tag) { + } + + // static + ShaderModuleBase* ShaderModuleBase::MakeError(DeviceBase* device) { + return new ShaderModuleBase(device, ObjectBase::kError); + } + void ShaderModuleBase::ExtractSpirvInfo(const spirv_cross::Compiler& compiler) { + ASSERT(!IsError()); + DeviceBase* device = GetDevice(); // TODO(cwallez@chromium.org): make errors here builder-level // currently errors here do not prevent the shadermodule from being used @@ -211,22 +222,28 @@ namespace dawn_native { } const ShaderModuleBase::PushConstantInfo& ShaderModuleBase::GetPushConstants() const { + ASSERT(!IsError()); return mPushConstants; } const ShaderModuleBase::ModuleBindingInfo& ShaderModuleBase::GetBindingInfo() const { + ASSERT(!IsError()); return mBindingInfo; } const std::bitset& ShaderModuleBase::GetUsedVertexAttributes() const { + ASSERT(!IsError()); return mUsedVertexAttributes; } dawn::ShaderStage ShaderModuleBase::GetExecutionModel() const { + ASSERT(!IsError()); return mExecutionModel; } bool ShaderModuleBase::IsCompatibleWithPipelineLayout(const PipelineLayoutBase* layout) { + ASSERT(!IsError()); + for (uint32_t group : IterateBitSet(layout->GetBindGroupLayoutsMask())) { if (!IsCompatibleWithBindGroupLayout(group, layout->GetBindGroupLayout(group))) { return false; @@ -246,6 +263,8 @@ namespace dawn_native { bool ShaderModuleBase::IsCompatibleWithBindGroupLayout(size_t group, const BindGroupLayoutBase* layout) { + ASSERT(!IsError()); + const auto& layoutInfo = layout->GetBindingInfo(); for (size_t i = 0; i < kMaxBindingsPerGroup; ++i) { const auto& moduleInfo = mBindingInfo[group][i]; diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h index 1d0fcf186d..dbeeb7be27 100644 --- a/src/dawn_native/ShaderModule.h +++ b/src/dawn_native/ShaderModule.h @@ -40,6 +40,8 @@ namespace dawn_native { public: ShaderModuleBase(DeviceBase* device, const ShaderModuleDescriptor* descriptor); + static ShaderModuleBase* MakeError(DeviceBase* device); + void ExtractSpirvInfo(const spirv_cross::Compiler& compiler); struct PushConstantInfo { @@ -68,6 +70,8 @@ namespace dawn_native { bool IsCompatibleWithPipelineLayout(const PipelineLayoutBase* layout); private: + ShaderModuleBase(DeviceBase* device, ObjectBase::ErrorTag tag); + bool IsCompatibleWithBindGroupLayout(size_t group, const BindGroupLayoutBase* layout); PushConstantInfo mPushConstants = {}; diff --git a/src/dawn_native/SwapChain.cpp b/src/dawn_native/SwapChain.cpp index c2ea095c20..fd1cb51645 100644 --- a/src/dawn_native/SwapChain.cpp +++ b/src/dawn_native/SwapChain.cpp @@ -72,6 +72,7 @@ namespace dawn_native { } void SwapChainBase::Present(TextureBase* texture) { + // This also checks that the texture is valid since mLastNextTexture is always valid. if (texture != mLastNextTexture) { GetDevice()->HandleError("Tried to present something other than the last NextTexture"); return; diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp index 44b416093e..74cc6963aa 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -163,13 +163,14 @@ namespace dawn_native { return {}; } - MaybeError ValidateTextureViewDescriptor(const DeviceBase*, + MaybeError ValidateTextureViewDescriptor(const DeviceBase* device, const TextureBase* texture, const TextureViewDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); } + DAWN_TRY(device->ValidateObject(texture)); DAWN_TRY(ValidateTextureViewDimension(descriptor->dimension)); DAWN_TRY(ValidateTextureFormat(descriptor->format)); @@ -292,31 +293,52 @@ namespace dawn_native { mUsage(descriptor->usage) { } + TextureBase::TextureBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : ObjectBase(device, tag) { + } + + // static + TextureBase* TextureBase::MakeError(DeviceBase* device) { + return new TextureBase(device, ObjectBase::kError); + } + dawn::TextureDimension TextureBase::GetDimension() const { + ASSERT(!IsError()); return mDimension; } dawn::TextureFormat TextureBase::GetFormat() const { + ASSERT(!IsError()); return mFormat; } const Extent3D& TextureBase::GetSize() const { + ASSERT(!IsError()); return mSize; } uint32_t TextureBase::GetArrayLayers() const { + ASSERT(!IsError()); return mArrayLayers; } uint32_t TextureBase::GetNumMipLevels() const { + ASSERT(!IsError()); return mNumMipLevels; } dawn::TextureUsageBit TextureBase::GetUsage() const { + ASSERT(!IsError()); return mUsage; } MaybeError TextureBase::ValidateCanUseInSubmitNow() const { + ASSERT(!IsError()); return {}; } TextureViewBase* TextureBase::CreateDefaultTextureView() { - TextureViewDescriptor descriptor = MakeDefaultTextureViewDescriptor(this); + TextureViewDescriptor descriptor = {}; + + if (!IsError()) { + descriptor = MakeDefaultTextureViewDescriptor(this); + } + return GetDevice()->CreateTextureView(this, &descriptor); } @@ -336,31 +358,47 @@ namespace dawn_native { mLayerCount(descriptor->layerCount) { } + TextureViewBase::TextureViewBase(DeviceBase* device, ObjectBase::ErrorTag tag) + : ObjectBase(device, tag) { + } + + // static + TextureViewBase* TextureViewBase::MakeError(DeviceBase* device) { + return new TextureViewBase(device, ObjectBase::kError); + } + const TextureBase* TextureViewBase::GetTexture() const { + ASSERT(!IsError()); return mTexture.Get(); } TextureBase* TextureViewBase::GetTexture() { + ASSERT(!IsError()); return mTexture.Get(); } dawn::TextureFormat TextureViewBase::GetFormat() const { + ASSERT(!IsError()); return mFormat; } uint32_t TextureViewBase::GetBaseMipLevel() const { + ASSERT(!IsError()); return mBaseMipLevel; } uint32_t TextureViewBase::GetLevelCount() const { + ASSERT(!IsError()); return mLevelCount; } uint32_t TextureViewBase::GetBaseArrayLayer() const { + ASSERT(!IsError()); return mBaseArrayLayer; } uint32_t TextureViewBase::GetLayerCount() const { + ASSERT(!IsError()); return mLayerCount; } } // namespace dawn_native diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h index 12edb3cf6b..5967cde3e0 100644 --- a/src/dawn_native/Texture.h +++ b/src/dawn_native/Texture.h @@ -47,6 +47,8 @@ namespace dawn_native { public: TextureBase(DeviceBase* device, const TextureDescriptor* descriptor); + static TextureBase* MakeError(DeviceBase* device); + dawn::TextureDimension GetDimension() const; dawn::TextureFormat GetFormat() const; const Extent3D& GetSize() const; @@ -61,6 +63,8 @@ namespace dawn_native { TextureViewBase* CreateTextureView(const TextureViewDescriptor* descriptor); private: + TextureBase(DeviceBase* device, ObjectBase::ErrorTag tag); + dawn::TextureDimension mDimension; dawn::TextureFormat mFormat; Extent3D mSize; @@ -73,6 +77,8 @@ namespace dawn_native { public: TextureViewBase(TextureBase* texture, const TextureViewDescriptor* descriptor); + static TextureViewBase* MakeError(DeviceBase* device); + const TextureBase* GetTexture() const; TextureBase* GetTexture(); @@ -83,6 +89,8 @@ namespace dawn_native { uint32_t GetLayerCount() const; private: + TextureViewBase(DeviceBase* device, ObjectBase::ErrorTag tag); + Ref mTexture; dawn::TextureFormat mFormat; diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp index c76c46d339..e78ccb5941 100644 --- a/src/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp @@ -161,6 +161,19 @@ TEST_F(BindGroupValidationTest, SamplerBindingType) { binding.buffer = mUBO; ASSERT_DEVICE_ERROR(device.CreateBindGroup(&descriptor)); binding.buffer = nullptr; + + // Setting the sampler to an error sampler is an error. + { + dawn::SamplerDescriptor samplerDesc = utils::GetDefaultSamplerDescriptor(); + samplerDesc.minFilter = static_cast(0xFFFFFFFF); + + dawn::Sampler errorSampler; + ASSERT_DEVICE_ERROR(errorSampler = device.CreateSampler(&samplerDesc)); + + binding.sampler = errorSampler; + ASSERT_DEVICE_ERROR(device.CreateBindGroup(&descriptor)); + binding.sampler = nullptr; + } } // Check that a texture binding must contain exactly a texture view @@ -198,6 +211,24 @@ TEST_F(BindGroupValidationTest, TextureBindingType) { binding.buffer = mUBO; ASSERT_DEVICE_ERROR(device.CreateBindGroup(&descriptor)); binding.buffer = nullptr; + + // Setting the texture view to an error texture view is an error. + { + dawn::TextureViewDescriptor viewDesc; + viewDesc.format = dawn::TextureFormat::R8G8B8A8Unorm; + viewDesc.dimension = dawn::TextureViewDimension::e2D; + viewDesc.baseMipLevel = 0; + viewDesc.levelCount = 0; + viewDesc.baseArrayLayer = 0; + viewDesc.layerCount = 0; + + dawn::TextureView errorView; + ASSERT_DEVICE_ERROR(errorView = mSampledTexture.CreateTextureView(&viewDesc)); + + binding.textureView = errorView; + ASSERT_DEVICE_ERROR(device.CreateBindGroup(&descriptor)); + binding.textureView = nullptr; + } } // Check that a buffer binding must contain exactly a buffer @@ -235,6 +266,20 @@ TEST_F(BindGroupValidationTest, BufferBindingType) { binding.sampler = mSampler; ASSERT_DEVICE_ERROR(device.CreateBindGroup(&descriptor)); binding.sampler = nullptr; + + // Setting the buffer to an error buffer is an error. + { + dawn::BufferDescriptor bufferDesc; + bufferDesc.size = 1024; + bufferDesc.usage = static_cast(0xFFFFFFFF); + + dawn::Buffer errorBuffer; + ASSERT_DEVICE_ERROR(errorBuffer = device.CreateBuffer(&bufferDesc)); + + binding.buffer = errorBuffer; + ASSERT_DEVICE_ERROR(device.CreateBindGroup(&descriptor)); + binding.buffer = nullptr; + } } // Check that a texture must have the correct usage @@ -338,6 +383,25 @@ TEST_F(BindGroupValidationTest, BufferBindingOOB) { ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, buffer, 256, uint32_t(0) - uint32_t(256)}})); } +// Test what happens when the layout is an error. +TEST_F(BindGroupValidationTest, ErrorLayout) { + dawn::BindGroupLayout goodLayout = utils::MakeBindGroupLayout(device, { + {0, dawn::ShaderStageBit::Vertex, dawn::BindingType::UniformBuffer}, + }); + + dawn::BindGroupLayout errorLayout; + ASSERT_DEVICE_ERROR(errorLayout = utils::MakeBindGroupLayout(device, { + {0, dawn::ShaderStageBit::Vertex, dawn::BindingType::UniformBuffer}, + {0, dawn::ShaderStageBit::Vertex, dawn::BindingType::UniformBuffer}, + })); + + // Control case, creating with the good layout works + utils::MakeBindGroup(device, goodLayout, {{0, mUBO, 0, 256}}); + + // Control case, creating with the good layout works + ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, errorLayout, {{0, mUBO, 0, 256}})); +} + class BindGroupLayoutValidationTest : public ValidationTest { };