diff --git a/src/dawn_native/BindGroup.cpp b/src/dawn_native/BindGroup.cpp index 32a40dae8f..3765fedc2e 100644 --- a/src/dawn_native/BindGroup.cpp +++ b/src/dawn_native/BindGroup.cpp @@ -147,7 +147,6 @@ namespace dawn_native { DAWN_TRY(ValidateBufferBinding(device, binding, wgpu::BufferUsage::Uniform)); break; case wgpu::BindingType::StorageBuffer: - case wgpu::BindingType::ReadonlyStorageBuffer: DAWN_TRY(ValidateBufferBinding(device, binding, wgpu::BufferUsage::Storage)); break; case wgpu::BindingType::SampledTexture: @@ -160,6 +159,7 @@ namespace dawn_native { DAWN_TRY(ValidateSamplerBinding(device, binding)); break; case wgpu::BindingType::StorageTexture: + case wgpu::BindingType::ReadonlyStorageBuffer: UNREACHABLE(); break; } @@ -231,9 +231,7 @@ namespace dawn_native { ASSERT(binding < kMaxBindingsPerGroup); ASSERT(mLayout->GetBindingInfo().mask[binding]); ASSERT(mLayout->GetBindingInfo().types[binding] == wgpu::BindingType::UniformBuffer || - mLayout->GetBindingInfo().types[binding] == wgpu::BindingType::StorageBuffer || - mLayout->GetBindingInfo().types[binding] == - wgpu::BindingType::ReadonlyStorageBuffer); + mLayout->GetBindingInfo().types[binding] == wgpu::BindingType::StorageBuffer); BufferBase* buffer = static_cast(mBindings[binding].Get()); return {buffer, mOffsets[binding], mSizes[binding]}; } diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 3d0ecd2d36..d15103391c 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -56,7 +56,6 @@ namespace dawn_native { } break; case wgpu::BindingType::StorageBuffer: - case wgpu::BindingType::ReadonlyStorageBuffer: if (binding.hasDynamicOffset) { ++dynamicStorageBufferCount; } @@ -67,6 +66,8 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("Samplers and textures cannot be dynamic"); } break; + case wgpu::BindingType::ReadonlyStorageBuffer: + return DAWN_VALIDATION_ERROR("readonly storage buffers aren't supported (yet)"); case wgpu::BindingType::StorageTexture: return DAWN_VALIDATION_ERROR("storage textures aren't supported (yet)"); } @@ -150,11 +151,11 @@ namespace dawn_native { ++mDynamicUniformBufferCount; break; case wgpu::BindingType::StorageBuffer: - case wgpu::BindingType::ReadonlyStorageBuffer: ++mDynamicStorageBufferCount; break; case wgpu::BindingType::SampledTexture: case wgpu::BindingType::Sampler: + case wgpu::BindingType::ReadonlyStorageBuffer: case wgpu::BindingType::StorageTexture: UNREACHABLE(); break; diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index a44e3c22bd..5712bdf2ee 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp @@ -115,12 +115,6 @@ namespace dawn_native { mSize(descriptor->size), mUsage(descriptor->usage), mState(BufferState::Unmapped) { - // Add readonly storage usage if the buffer has a storage usage. The validation rules in - // PassResourceUsageTracker::ValidateUsages will make sure we don't use both at the same - // time. - if (mUsage & wgpu::BufferUsage::Storage) { - mUsage |= kReadOnlyStorage; - } } BufferBase::BufferBase(DeviceBase* device, ObjectBase::ErrorTag tag) diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h index 054e555045..3a675aac49 100644 --- a/src/dawn_native/Buffer.h +++ b/src/dawn_native/Buffer.h @@ -27,13 +27,9 @@ namespace dawn_native { MaybeError ValidateBufferDescriptor(DeviceBase* device, const BufferDescriptor* descriptor); - // Add an extra buffer usage (readonly storage buffer usage) for render pass resource tracking - static constexpr wgpu::BufferUsage kReadOnlyStorage = - static_cast(0x80000000); - static constexpr wgpu::BufferUsage kReadOnlyBufferUsages = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::Index | - wgpu::BufferUsage::Vertex | wgpu::BufferUsage::Uniform | kReadOnlyStorage; + wgpu::BufferUsage::Vertex | wgpu::BufferUsage::Uniform; static constexpr wgpu::BufferUsage kWritableBufferUsages = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::Storage; diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index 5517cc4ed0..e3bb099462 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -16,7 +16,6 @@ #include "common/BitSetIterator.h" #include "dawn_native/BindGroup.h" -#include "dawn_native/Buffer.h" #include "dawn_native/CommandBufferStateTracker.h" #include "dawn_native/Commands.h" #include "dawn_native/PassResourceUsageTracker.h" @@ -50,15 +49,11 @@ namespace dawn_native { usageTracker->TextureUsedAs(texture, wgpu::TextureUsage::Sampled); } break; - case wgpu::BindingType::ReadonlyStorageBuffer: { - BufferBase* buffer = group->GetBindingAsBufferBinding(i).buffer; - usageTracker->BufferUsedAs(buffer, kReadOnlyStorage); - } break; - case wgpu::BindingType::Sampler: break; case wgpu::BindingType::StorageTexture: + case wgpu::BindingType::ReadonlyStorageBuffer: UNREACHABLE(); break; } diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp index e72ae1406e..1cd4a96f1e 100644 --- a/src/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp @@ -372,18 +372,6 @@ TEST_F(BindGroupValidationTest, BufferUsageSSBO) { ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, mUBO, 0, 256}})); } -// Check that a readonly SSBO must have the correct usage -TEST_F(BindGroupValidationTest, BufferUsageReadonlySSBO) { - wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ReadonlyStorageBuffer}}); - - // Control case: using a buffer with the storage usage works - utils::MakeBindGroup(device, layout, {{0, mSSBO, 0, 256}}); - - // Using a buffer without the storage usage fails - ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, layout, {{0, mUBO, 0, 256}})); -} - // Tests constraints on the buffer offset for bind groups. TEST_F(BindGroupValidationTest, BufferOffsetAlignment) { wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout( @@ -527,11 +515,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicAndTypeCompatibility) { {0, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer, true}, }); - utils::MakeBindGroupLayout( - device, { - {0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true}, - }); - ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout( device, { {0, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture, true}, @@ -563,7 +546,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) { wgpu::BindGroupLayout bgl[2]; std::vector maxUniformDB; std::vector maxStorageDB; - std::vector maxReadonlyStorageDB; for (uint32_t i = 0; i < kMaxDynamicUniformBufferCount; ++i) { maxUniformDB.push_back( @@ -575,11 +557,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) { {i, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer, true}); } - for (uint32_t i = 0; i < kMaxDynamicStorageBufferCount; ++i) { - maxReadonlyStorageDB.push_back( - {i, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true}); - } - auto MakeBindGroupLayout = [&](wgpu::BindGroupLayoutBinding* binding, uint32_t count) -> wgpu::BindGroupLayout { wgpu::BindGroupLayoutDescriptor descriptor; @@ -595,14 +572,7 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) { TestCreatePipelineLayout(bgl, 2, true); } - { - bgl[0] = MakeBindGroupLayout(maxUniformDB.data(), maxUniformDB.size()); - bgl[1] = MakeBindGroupLayout(maxReadonlyStorageDB.data(), maxReadonlyStorageDB.size()); - - TestCreatePipelineLayout(bgl, 2, true); - } - - // Check dynamic uniform buffers exceed maximum in pipeline layout. + // Check dynamic uniform buffers excedd maximum in pipeline layout. { bgl[0] = MakeBindGroupLayout(maxUniformDB.data(), maxUniformDB.size()); bgl[1] = utils::MakeBindGroupLayout( @@ -624,31 +594,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) { TestCreatePipelineLayout(bgl, 2, false); } - // Check dynamic readonly storage buffers exceed maximum in pipeline layout - { - bgl[0] = MakeBindGroupLayout(maxReadonlyStorageDB.data(), maxReadonlyStorageDB.size()); - bgl[1] = utils::MakeBindGroupLayout( - device, - { - {0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true}, - }); - - TestCreatePipelineLayout(bgl, 2, false); - } - - // Check dynamic storage buffers + dynamic readonly storage buffers exceed maximum storage - // buffers in pipeline layout - { - bgl[0] = MakeBindGroupLayout(maxStorageDB.data(), maxStorageDB.size()); - bgl[1] = utils::MakeBindGroupLayout( - device, - { - {0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true}, - }); - - TestCreatePipelineLayout(bgl, 2, false); - } - // Check dynamic uniform buffers exceed maximum in bind group layout. { maxUniformDB.push_back({kMaxDynamicUniformBufferCount, wgpu::ShaderStage::Compute, @@ -662,13 +607,6 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) { wgpu::BindingType::StorageBuffer, true}); TestCreateBindGroupLayout(maxStorageDB.data(), maxStorageDB.size(), false); } - - // Check dynamic readonly storage buffers exceed maximum in bind group layout. - { - maxReadonlyStorageDB.push_back({kMaxDynamicStorageBufferCount, wgpu::ShaderStage::Compute, - wgpu::BindingType::ReadonlyStorageBuffer, true}); - TestCreateBindGroupLayout(maxReadonlyStorageDB.data(), maxReadonlyStorageDB.size(), false); - } } constexpr uint64_t kBufferSize = 2 * kMinDynamicBufferOffsetAlignment + 8; diff --git a/src/tests/unittests/validation/CommandBufferValidationTests.cpp b/src/tests/unittests/validation/CommandBufferValidationTests.cpp index b5be61a75c..cc4ab6798c 100644 --- a/src/tests/unittests/validation/CommandBufferValidationTests.cpp +++ b/src/tests/unittests/validation/CommandBufferValidationTests.cpp @@ -223,55 +223,6 @@ TEST_F(CommandBufferValidationTest, BufferWithReadAndWriteUsage) { ASSERT_DEVICE_ERROR(encoder.Finish()); } -// Test that using a single buffer in multiple read usages which include readonly storage usage in -// the same pass is allowed. -TEST_F(CommandBufferValidationTest, BufferWithMultipleReadAndReadOnlyStorageUsage) { - // Create a buffer that will be used as an index buffer and as a storage buffer - wgpu::BufferDescriptor bufferDescriptor; - bufferDescriptor.usage = wgpu::BufferUsage::Storage | wgpu::BufferUsage::Index; - bufferDescriptor.size = 4; - wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor); - - // Create the bind group to use the buffer as storage - wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::ReadonlyStorageBuffer}}); - wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}}); - - // Use the buffer as both index and storage in the same pass - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - DummyRenderPass dummyRenderPass(device); - wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&dummyRenderPass); - pass.SetIndexBuffer(buffer); - pass.SetBindGroup(0, bg); - pass.EndPass(); - encoder.Finish(); -} - -// Test that using the same storage buffer as both readable and writable in the same pass is -// disallowed -TEST_F(CommandBufferValidationTest, BufferWithReadAndWriteStorageBufferUsage) { - // Create a buffer that will be used as an storage buffer and as a readonly storage buffer - wgpu::BufferDescriptor bufferDescriptor; - bufferDescriptor.usage = wgpu::BufferUsage::Storage; - bufferDescriptor.size = 512; - wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor); - - // Create the bind group to use the buffer as storage - wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::StorageBuffer}, - {1, wgpu::ShaderStage::Vertex, wgpu::BindingType::ReadonlyStorageBuffer}}); - wgpu::BindGroup bg = - utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}, {1, buffer, 256, 4}}); - - // Use the buffer as both index and storage in the same pass - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - DummyRenderPass dummyRenderPass(device); - wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&dummyRenderPass); - pass.SetBindGroup(0, bg); - pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); -} - // Test that using the same texture as both readable and writable in the same pass is disallowed TEST_F(CommandBufferValidationTest, TextureWithReadAndWriteUsage) { // Create a texture that will be used both as a sampled texture and a render target