Remove WGSL readonly-storage to storage binding type promotion
Fixed: dawn:1188
Change-Id: Ied8e1fb8a47884456de3d56e049a361fdbcaf640
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/82403
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp
index 3b46d15..ebdaafd 100644
--- a/src/dawn/native/ShaderModule.cpp
+++ b/src/dawn/native/ShaderModule.cpp
@@ -540,13 +540,10 @@
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 828a21b..ddafa93 100644
--- a/src/dawn/tests/end2end/BindGroupTests.cpp
+++ b/src/dawn/tests/end2end/BindGroupTests.cpp
@@ -76,7 +76,7 @@
fs << "\n@group(" << i << ") @binding(0) var<uniform> buffer" << i
<< " : Buffer" << i << ";";
break;
- case wgpu::BufferBindingType::Storage:
+ case wgpu::BufferBindingType::ReadOnlyStorage:
fs << "\n@group(" << i << ") @binding(0) var<storage, read> buffer" << i
<< " : Buffer" << i << ";";
break;
@@ -709,11 +709,11 @@
// 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 @@
// 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 @@
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 33ea7fc..95ffef1 100644
--- a/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -2096,7 +2096,7 @@
}
};
-// 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 @@
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 @@
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 @@
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]}});
}
