diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp index 3b46d156ba..ebdaafd9eb 100644 --- a/src/dawn/native/ShaderModule.cpp +++ b/src/dawn/native/ShaderModule.cpp @@ -540,13 +540,10 @@ namespace dawn::native { case BindingInfoType::Buffer: { // Binding mismatch between shader and bind group is invalid. For example, a // writable binding in the shader with a readonly storage buffer in the bind - // group layout is invalid. However, a readonly binding in the shader with a - // writable storage buffer in the bind group layout is valid, a storage + // group layout is invalid. For internal usage with internal shaders, a storage // binding in the shader with an internal storage buffer in the bind group // layout is also valid. bool validBindingConversion = - (layoutInfo.buffer.type == wgpu::BufferBindingType::Storage && - shaderInfo.buffer.type == wgpu::BufferBindingType::ReadOnlyStorage) || (layoutInfo.buffer.type == kInternalStorageBufferBinding && shaderInfo.buffer.type == wgpu::BufferBindingType::Storage); diff --git a/src/dawn/tests/end2end/BindGroupTests.cpp b/src/dawn/tests/end2end/BindGroupTests.cpp index 828a21b75b..ddafa933c9 100644 --- a/src/dawn/tests/end2end/BindGroupTests.cpp +++ b/src/dawn/tests/end2end/BindGroupTests.cpp @@ -76,7 +76,7 @@ class BindGroupTests : public DawnTest { fs << "\n@group(" << i << ") @binding(0) var buffer" << i << " : Buffer" << i << ";"; break; - case wgpu::BufferBindingType::Storage: + case wgpu::BufferBindingType::ReadOnlyStorage: fs << "\n@group(" << i << ") @binding(0) var buffer" << i << " : Buffer" << i << ";"; break; @@ -709,11 +709,11 @@ TEST_P(BindGroupTests, BindGroupsPersistAfterPipelineChange) { // Create a bind group layout which uses a single dynamic storage buffer. wgpu::BindGroupLayout storageLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BufferBindingType::Storage, true}}); + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BufferBindingType::ReadOnlyStorage, true}}); // Create a pipeline which uses the uniform buffer and storage buffer bind groups. wgpu::RenderPipeline pipeline0 = MakeTestPipeline( - renderPass, {wgpu::BufferBindingType::Uniform, wgpu::BufferBindingType::Storage}, + renderPass, {wgpu::BufferBindingType::Uniform, wgpu::BufferBindingType::ReadOnlyStorage}, {uniformLayout, storageLayout}); // Create a pipeline which uses the uniform buffer bind group twice. @@ -787,21 +787,21 @@ TEST_P(BindGroupTests, DrawThenChangePipelineAndBindGroup) { // Create a bind group layout which uses a single dynamic storage buffer. wgpu::BindGroupLayout storageLayout = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BufferBindingType::Storage, true}}); + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BufferBindingType::ReadOnlyStorage, true}}); // Create a pipeline with pipeline layout (uniform, uniform, storage). wgpu::RenderPipeline pipeline0 = MakeTestPipeline(renderPass, {wgpu::BufferBindingType::Uniform, wgpu::BufferBindingType::Uniform, - wgpu::BufferBindingType::Storage}, + wgpu::BufferBindingType::ReadOnlyStorage}, {uniformLayout, uniformLayout, storageLayout}); // Create a pipeline with pipeline layout (uniform, storage, storage). - wgpu::RenderPipeline pipeline1 = - MakeTestPipeline(renderPass, - {wgpu::BufferBindingType::Uniform, wgpu::BufferBindingType::Storage, - wgpu::BufferBindingType::Storage}, - {uniformLayout, storageLayout, storageLayout}); + wgpu::RenderPipeline pipeline1 = MakeTestPipeline( + renderPass, + {wgpu::BufferBindingType::Uniform, wgpu::BufferBindingType::ReadOnlyStorage, + wgpu::BufferBindingType::ReadOnlyStorage}, + {uniformLayout, storageLayout, storageLayout}); // Prepare color data. // The first draw will use { color0, color1, color2 }. @@ -1400,7 +1400,7 @@ TEST_P(BindGroupTests, ReadonlyStorage) { pipelineDescriptor.cTargets[0].format = renderPass.colorFormat; wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Fragment, wgpu::BufferBindingType::Storage}}); + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BufferBindingType::ReadOnlyStorage}}); pipelineDescriptor.layout = utils::MakeBasicPipelineLayout(device, &bgl); diff --git a/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp b/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp index 33ea7fcadf..95ffef1871 100644 --- a/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp @@ -2096,7 +2096,7 @@ class BindGroupLayoutCompatibilityTest : public ValidationTest { } }; -// Test that it is valid to pass a writable storage buffer in the pipeline layout when the shader +// Test that it is invalid to pass a writable storage buffer in the pipeline layout when the shader // uses the binding as a readonly storage buffer. TEST_F(BindGroupLayoutCompatibilityTest, RWStorageInBGLWithROStorageInShader) { // Set up the bind group layout. @@ -2107,9 +2107,9 @@ TEST_F(BindGroupLayoutCompatibilityTest, RWStorageInBGLWithROStorageInShader) { device, {{0, wgpu::ShaderStage::Compute | wgpu::ShaderStage::Fragment, wgpu::BufferBindingType::Storage}}); - CreateRenderPipeline({bgl0, bgl1}); + ASSERT_DEVICE_ERROR(CreateRenderPipeline({bgl0, bgl1})); - CreateComputePipeline({bgl0, bgl1}); + ASSERT_DEVICE_ERROR(CreateComputePipeline({bgl0, bgl1})); } // Test that it is invalid to pass a readonly storage buffer in the pipeline layout when the shader @@ -2349,7 +2349,8 @@ TEST_F(BindingsValidationTest, BindGroupsWithMoreBindingsThanPipelineLayout) { for (uint32_t i = 0; i < kBindingNum + 1; ++i) { bgl[i] = utils::MakeBindGroupLayout( device, {{0, wgpu::ShaderStage::Compute | wgpu::ShaderStage::Fragment, - wgpu::BufferBindingType::Storage}}); + i == 1 ? wgpu::BufferBindingType::ReadOnlyStorage + : wgpu::BufferBindingType::Storage}}); buffer[i] = CreateBuffer(mBufferSize, wgpu::BufferUsage::Storage); bg[i] = utils::MakeBindGroup(device, bgl[i], {{0, buffer[i]}}); } @@ -2390,7 +2391,8 @@ TEST_F(BindingsValidationTest, BindGroupsWithLessBindingsThanPipelineLayout) { for (uint32_t i = 0; i < kBindingNum; ++i) { bgl[i] = utils::MakeBindGroupLayout( device, {{0, wgpu::ShaderStage::Compute | wgpu::ShaderStage::Fragment, - wgpu::BufferBindingType::Storage}}); + i == 1 ? wgpu::BufferBindingType::ReadOnlyStorage + : wgpu::BufferBindingType::Storage}}); buffer[i] = CreateBuffer(mBufferSize, wgpu::BufferUsage::Storage); bg[i] = utils::MakeBindGroup(device, bgl[i], {{0, buffer[i]}}); }