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>
This commit is contained in:
Austin Eng 2022-03-05 01:37:25 +00:00 committed by Dawn LUCI CQ
parent 6d205fcb5d
commit ea287f5b48
3 changed files with 19 additions and 20 deletions

View File

@ -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);

View File

@ -76,7 +76,7 @@ class BindGroupTests : public DawnTest {
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 @@ 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,20 +787,20 @@ 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},
wgpu::RenderPipeline pipeline1 = MakeTestPipeline(
renderPass,
{wgpu::BufferBindingType::Uniform, wgpu::BufferBindingType::ReadOnlyStorage,
wgpu::BufferBindingType::ReadOnlyStorage},
{uniformLayout, storageLayout, storageLayout});
// Prepare color data.
@ -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);

View File

@ -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]}});
}