diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index 6d33f0ae2d..390d16d9cb 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -312,13 +312,7 @@ namespace dawn_native { MaybeError ValidatePassResourceUsage(const PassResourceUsage& pass) { // Buffers can only be used as single-write or multiple read. for (size_t i = 0; i < pass.buffers.size(); ++i) { - const BufferBase* buffer = pass.buffers[i]; wgpu::BufferUsage usage = pass.bufferUsages[i]; - - if (usage & ~buffer->GetUsage()) { - return DAWN_VALIDATION_ERROR("Buffer missing usage for the pass"); - } - bool readOnly = IsSubset(usage, kReadOnlyBufferUsages); bool singleUse = wgpu::HasZeroOrOneBits(usage); diff --git a/src/dawn_native/ComputePassEncoder.cpp b/src/dawn_native/ComputePassEncoder.cpp index afa9df0a38..39af70f7c5 100644 --- a/src/dawn_native/ComputePassEncoder.cpp +++ b/src/dawn_native/ComputePassEncoder.cpp @@ -69,6 +69,7 @@ namespace dawn_native { void ComputePassEncoder::DispatchIndirect(BufferBase* indirectBuffer, uint64_t indirectOffset) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer)); + DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect)); // Indexed dispatches need a compute-shader based validation to check that the dispatch // sizes aren't too big. Disallow them as unsafe until the validation is implemented. diff --git a/src/dawn_native/RenderEncoderBase.cpp b/src/dawn_native/RenderEncoderBase.cpp index 2bc3c8d374..fac7822cc4 100644 --- a/src/dawn_native/RenderEncoderBase.cpp +++ b/src/dawn_native/RenderEncoderBase.cpp @@ -17,6 +17,7 @@ #include "common/Constants.h" #include "dawn_native/Buffer.h" #include "dawn_native/CommandEncoder.h" +#include "dawn_native/CommandValidation.h" #include "dawn_native/Commands.h" #include "dawn_native/Device.h" #include "dawn_native/RenderPipeline.h" @@ -87,6 +88,7 @@ namespace dawn_native { void RenderEncoderBase::DrawIndirect(BufferBase* indirectBuffer, uint64_t indirectOffset) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer)); + DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect)); if (indirectOffset % 4 != 0) { return DAWN_VALIDATION_ERROR("Indirect offset must be a multiple of 4"); @@ -111,6 +113,7 @@ namespace dawn_native { uint64_t indirectOffset) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer)); + DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect)); // Indexed indirect draws need a compute-shader based validation check that the range of // indices is contained inside the index buffer on Metal. Disallow them as unsafe until @@ -167,6 +170,7 @@ namespace dawn_native { uint64_t size) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { DAWN_TRY(GetDevice()->ValidateObject(buffer)); + DAWN_TRY(ValidateCanUseAs(buffer, wgpu::BufferUsage::Index)); DAWN_TRY(ValidateIndexFormat(format)); if (format == wgpu::IndexFormat::Undefined) { @@ -206,6 +210,7 @@ namespace dawn_native { uint64_t size) { mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError { DAWN_TRY(GetDevice()->ValidateObject(buffer)); + DAWN_TRY(ValidateCanUseAs(buffer, wgpu::BufferUsage::Vertex)); if (slot >= kMaxVertexBuffers) { return DAWN_VALIDATION_ERROR("Vertex buffer slot out of bounds"); diff --git a/src/tests/unittests/validation/ComputeIndirectValidationTests.cpp b/src/tests/unittests/validation/ComputeIndirectValidationTests.cpp index 06693df7d0..32248190f4 100644 --- a/src/tests/unittests/validation/ComputeIndirectValidationTests.cpp +++ b/src/tests/unittests/validation/ComputeIndirectValidationTests.cpp @@ -46,9 +46,10 @@ class ComputeIndirectValidationTest : public ValidationTest { void TestIndirectOffset(utils::Expectation expectation, std::initializer_list bufferList, - uint64_t indirectOffset) { + uint64_t indirectOffset, + wgpu::BufferUsage usage = wgpu::BufferUsage::Indirect) { wgpu::Buffer indirectBuffer = - utils::CreateBufferFromData(device, wgpu::BufferUsage::Indirect, bufferList); + utils::CreateBufferFromData(device, usage, bufferList); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::ComputePassEncoder pass = encoder.BeginComputePass(); @@ -85,3 +86,12 @@ TEST_F(ComputeIndirectValidationTest, IndirectOffsetBounds) { uint64_t offset = std::numeric_limits::max(); TestIndirectOffset(utils::Expectation::Failure, {1, 2, 3, 4, 5, 6}, offset); } + +// Check that the buffer must have the indirect usage +TEST_F(ComputeIndirectValidationTest, IndirectUsage) { + // Control case: using a buffer with the indirect usage is valid. + TestIndirectOffset(utils::Expectation::Success, {1, 2, 3}, 0, wgpu::BufferUsage::Indirect); + + // Error case: using a buffer with the vertex usage is an error. + TestIndirectOffset(utils::Expectation::Failure, {1, 2, 3}, 0, wgpu::BufferUsage::Vertex); +} diff --git a/src/tests/unittests/validation/DrawIndirectValidationTests.cpp b/src/tests/unittests/validation/DrawIndirectValidationTests.cpp index c030202d7b..9d1894af0b 100644 --- a/src/tests/unittests/validation/DrawIndirectValidationTests.cpp +++ b/src/tests/unittests/validation/DrawIndirectValidationTests.cpp @@ -69,9 +69,10 @@ class DrawIndirectValidationTest : public ValidationTest { void TestIndirectOffset(utils::Expectation expectation, std::initializer_list bufferList, uint64_t indirectOffset, - bool indexed) { + bool indexed, + wgpu::BufferUsage usage = wgpu::BufferUsage::Indirect) { wgpu::Buffer indirectBuffer = - utils::CreateBufferFromData(device, wgpu::BufferUsage::Indirect, bufferList); + utils::CreateBufferFromData(device, usage, bufferList); DummyRenderPass renderPass(device); wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); @@ -146,3 +147,18 @@ TEST_F(DrawIndirectValidationTest, DrawIndexedIndirectOffsetBounds) { TestIndirectOffsetDrawIndexed(utils::Expectation::Failure, {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, offset); } + +// Check that the buffer must have the indirect usage +TEST_F(DrawIndirectValidationTest, IndirectUsage) { + // Control cases: using a buffer with the indirect usage is valid. + TestIndirectOffset(utils::Expectation::Success, {1, 2, 3, 4}, 0, false, + wgpu::BufferUsage::Indirect); + TestIndirectOffset(utils::Expectation::Success, {1, 2, 3, 4, 5}, 0, true, + wgpu::BufferUsage::Indirect); + + // Error cases: using a buffer with the vertex usage is an error. + TestIndirectOffset(utils::Expectation::Failure, {1, 2, 3, 4}, 0, false, + wgpu::BufferUsage::Vertex); + TestIndirectOffset(utils::Expectation::Failure, {1, 2, 3, 4, 5}, 0, true, + wgpu::BufferUsage::Vertex); +} diff --git a/src/tests/unittests/validation/IndexBufferValidationTests.cpp b/src/tests/unittests/validation/IndexBufferValidationTests.cpp index d849a29c7a..8213baf2c2 100644 --- a/src/tests/unittests/validation/IndexBufferValidationTests.cpp +++ b/src/tests/unittests/validation/IndexBufferValidationTests.cpp @@ -184,3 +184,45 @@ TEST_F(IndexBufferValidationTest, IndexBufferFormatMatchesPipelineStripFormat) { encoder.Finish(); } } + +// Check that the index buffer must have the Index usage. +TEST_F(IndexBufferValidationTest, InvalidUsage) { + wgpu::Buffer indexBuffer = + utils::CreateBufferFromData(device, wgpu::BufferUsage::Index, {0, 1, 2}); + wgpu::Buffer copyBuffer = + utils::CreateBufferFromData(device, wgpu::BufferUsage::CopySrc, {0, 1, 2}); + + DummyRenderPass renderPass(device); + // Control case: using the index buffer is valid. + { + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); + pass.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint32); + pass.EndPass(); + encoder.Finish(); + } + // Error case: using the copy buffer is an error. + { + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); + pass.SetIndexBuffer(copyBuffer, wgpu::IndexFormat::Uint32); + pass.EndPass(); + ASSERT_DEVICE_ERROR(encoder.Finish()); + } + + utils::ComboRenderBundleEncoderDescriptor renderBundleDesc = {}; + renderBundleDesc.colorFormatsCount = 1; + renderBundleDesc.cColorFormats[0] = wgpu::TextureFormat::RGBA8Unorm; + // Control case: using the index buffer is valid. + { + wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc); + encoder.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint32); + encoder.Finish(); + } + // Error case: using the copy buffer is an error. + { + wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc); + encoder.SetIndexBuffer(copyBuffer, wgpu::IndexFormat::Uint32); + ASSERT_DEVICE_ERROR(encoder.Finish()); + } +} diff --git a/src/tests/unittests/validation/VertexBufferValidationTests.cpp b/src/tests/unittests/validation/VertexBufferValidationTests.cpp index 50e0f6226f..3015774994 100644 --- a/src/tests/unittests/validation/VertexBufferValidationTests.cpp +++ b/src/tests/unittests/validation/VertexBufferValidationTests.cpp @@ -83,6 +83,7 @@ class VertexBufferValidationTest : public ValidationTest { wgpu::ShaderModule fsModule; }; +// Check that vertex buffers still count as bound if we switch the pipeline. TEST_F(VertexBufferValidationTest, VertexBuffersInheritedBetweenPipelines) { DummyRenderPass renderPass(device); wgpu::ShaderModule vsModule2 = MakeVertexShader(2); @@ -119,7 +120,8 @@ TEST_F(VertexBufferValidationTest, VertexBuffersInheritedBetweenPipelines) { encoder.Finish(); } -TEST_F(VertexBufferValidationTest, VertexBuffersNotInheritedBetweenRendePasses) { +// Check that vertex buffers that are set are reset between render passes. +TEST_F(VertexBufferValidationTest, VertexBuffersNotInheritedBetweenRenderPasses) { DummyRenderPass renderPass(device); wgpu::ShaderModule vsModule2 = MakeVertexShader(2); wgpu::ShaderModule vsModule1 = MakeVertexShader(1); @@ -168,6 +170,7 @@ TEST_F(VertexBufferValidationTest, VertexBuffersNotInheritedBetweenRendePasses) ASSERT_DEVICE_ERROR(encoder.Finish()); } +// Check validation of the vertex buffer slot for OOB. TEST_F(VertexBufferValidationTest, VertexBufferSlotValidation) { wgpu::Buffer buffer = MakeVertexBuffer(); @@ -281,3 +284,44 @@ TEST_F(VertexBufferValidationTest, VertexBufferOffsetOOBValidation) { ASSERT_DEVICE_ERROR(encoder.Finish()); } } + +// Check that the vertex buffer must have the Vertex usage. +TEST_F(VertexBufferValidationTest, InvalidUsage) { + wgpu::Buffer vertexBuffer = MakeVertexBuffer(); + wgpu::Buffer indexBuffer = + utils::CreateBufferFromData(device, wgpu::BufferUsage::Index, {0, 0, 0}); + + DummyRenderPass renderPass(device); + // Control case: using the vertex buffer is valid. + { + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); + pass.SetVertexBuffer(0, vertexBuffer); + pass.EndPass(); + encoder.Finish(); + } + // Error case: using the index buffer is an error. + { + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); + pass.SetVertexBuffer(0, indexBuffer); + pass.EndPass(); + ASSERT_DEVICE_ERROR(encoder.Finish()); + } + + utils::ComboRenderBundleEncoderDescriptor renderBundleDesc = {}; + renderBundleDesc.colorFormatsCount = 1; + renderBundleDesc.cColorFormats[0] = wgpu::TextureFormat::RGBA8Unorm; + // Control case: using the vertex buffer is valid. + { + wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc); + encoder.SetVertexBuffer(0, vertexBuffer); + encoder.Finish(); + } + // Error case: using the index buffer is an error. + { + wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc); + encoder.SetVertexBuffer(0, indexBuffer); + ASSERT_DEVICE_ERROR(encoder.Finish()); + } +}