diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index 3d0ecd2d36..0ab3f1c3dd 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -49,6 +49,12 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("some binding index was specified more than once"); } + if (binding.type == wgpu::BindingType::StorageBuffer && + (binding.visibility & wgpu::ShaderStage::Vertex) != 0) { + return DAWN_VALIDATION_ERROR( + "storage buffer binding is not supported in vertex shader"); + } + switch (binding.type) { case wgpu::BindingType::UniformBuffer: if (binding.hasDynamicOffset) { diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp index b76e553377..a810f37435 100644 --- a/src/dawn_native/PipelineLayout.cpp +++ b/src/dawn_native/PipelineLayout.cpp @@ -132,9 +132,14 @@ namespace dawn_native { BindGroupLayoutBinding bindingSlot; bindingSlot.binding = binding; - bindingSlot.visibility = wgpu::ShaderStage::Vertex | - wgpu::ShaderStage::Fragment | - wgpu::ShaderStage::Compute; + if (bindingInfo.type == wgpu::BindingType::StorageBuffer) { + bindingSlot.visibility = + wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Compute; + } else { + bindingSlot.visibility = wgpu::ShaderStage::Vertex | + wgpu::ShaderStage::Fragment | + wgpu::ShaderStage::Compute; + } bindingSlot.type = bindingInfo.type; bindingSlot.hasDynamicOffset = false; bindingSlot.multisampled = bindingInfo.multisampled; diff --git a/src/tests/end2end/GpuMemorySynchronizationTests.cpp b/src/tests/end2end/GpuMemorySynchronizationTests.cpp index 2917ee8ebd..995d272ab5 100644 --- a/src/tests/end2end/GpuMemorySynchronizationTests.cpp +++ b/src/tests/end2end/GpuMemorySynchronizationTests.cpp @@ -179,7 +179,7 @@ TEST_P(GpuMemorySyncTests, RenderPassToComputePass) { pass0.Draw(1, 1, 0, 0); pass0.EndPass(); - // Read that data in render pass. + // Read that data in compute pass. wgpu::ComputePassEncoder pass1 = encoder.BeginComputePass(); pass1.SetPipeline(compute); pass1.SetBindGroup(0, bindGroup1); @@ -210,7 +210,7 @@ TEST_P(GpuMemorySyncTests, ComputePassToRenderPass) { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - // Write data into a storage buffer in render pass. + // Write data into a storage buffer in compute pass. wgpu::ComputePassEncoder pass0 = encoder.BeginComputePass(); pass0.SetPipeline(compute); pass0.SetBindGroup(0, bindGroup1); diff --git a/src/tests/end2end/OpArrayLengthTests.cpp b/src/tests/end2end/OpArrayLengthTests.cpp index 44283c6893..a20f9e217e 100644 --- a/src/tests/end2end/OpArrayLengthTests.cpp +++ b/src/tests/end2end/OpArrayLengthTests.cpp @@ -38,10 +38,10 @@ class OpArrayLengthTest : public DawnTest { // Put them all in a bind group for tests to bind them easily. wgpu::ShaderStage kAllStages = wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Vertex | wgpu::ShaderStage::Compute; - mBindGroupLayout = - utils::MakeBindGroupLayout(device, {{0, kAllStages, wgpu::BindingType::StorageBuffer}, - {1, kAllStages, wgpu::BindingType::StorageBuffer}, - {2, kAllStages, wgpu::BindingType::StorageBuffer}}); + mBindGroupLayout = utils::MakeBindGroupLayout( + device, {{0, kAllStages, wgpu::BindingType::ReadonlyStorageBuffer}, + {1, kAllStages, wgpu::BindingType::ReadonlyStorageBuffer}, + {2, kAllStages, wgpu::BindingType::ReadonlyStorageBuffer}}); mBindGroup = utils::MakeBindGroup(device, mBindGroupLayout, { @@ -54,19 +54,19 @@ class OpArrayLengthTest : public DawnTest { // 0. mShaderInterface = R"( // The length should be 1 because the buffer is 4-byte long. - layout(std430, set = 0, binding = 0) buffer Buffer1 { + layout(std430, set = 0, binding = 0) readonly buffer Buffer1 { float data[]; } buffer1; // The length should be 64 because the buffer is 256 bytes long. - layout(std430, set = 0, binding = 1) buffer Buffer2 { + layout(std430, set = 0, binding = 1) readonly buffer Buffer2 { float data[]; } buffer2; // The length should be (512 - 16*4) / 8 = 56 because the buffer is 512 bytes long // and the structure is 8 bytes big. struct Buffer3Data {float a; int b;}; - layout(std430, set = 0, binding = 2) buffer Buffer3 { + layout(std430, set = 0, binding = 2) readonly buffer Buffer3 { mat4 garbage; Buffer3Data data[]; } buffer3; diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp index 1e7ce5e943..de5df85dc8 100644 --- a/src/tests/unittests/validation/BindGroupValidationTests.cpp +++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp @@ -493,6 +493,22 @@ class BindGroupLayoutValidationTest : public ValidationTest { } }; +// Tests setting storage buffer and readonly storage buffer bindings in vertex and fragment shader. +TEST_F(BindGroupLayoutValidationTest, BindGroupLayoutStorageBindingsInVertexShader) { + // Checks that storage buffer binding is not supported in vertex shader. + ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::StorageBuffer}})); + + utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::ReadonlyStorageBuffer}}); + + utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::StorageBuffer}}); + + utils::MakeBindGroupLayout( + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ReadonlyStorageBuffer}}); +} + // Tests setting OOB checks for kMaxBindingsPerGroup in bind group layouts. TEST_F(BindGroupLayoutValidationTest, BindGroupLayoutBindingOOB) { // Checks that kMaxBindingsPerGroup - 1 is valid. diff --git a/src/tests/unittests/validation/CommandBufferValidationTests.cpp b/src/tests/unittests/validation/CommandBufferValidationTests.cpp index b5be61a75c..7c36afecb1 100644 --- a/src/tests/unittests/validation/CommandBufferValidationTests.cpp +++ b/src/tests/unittests/validation/CommandBufferValidationTests.cpp @@ -210,7 +210,7 @@ TEST_F(CommandBufferValidationTest, BufferWithReadAndWriteUsage) { // Create the bind group to use the buffer as storage wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout( - device, {{0, wgpu::ShaderStage::Vertex, wgpu::BindingType::StorageBuffer}}); + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::StorageBuffer}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}}); // Use the buffer as both index and storage in the same pass @@ -258,8 +258,8 @@ TEST_F(CommandBufferValidationTest, BufferWithReadAndWriteStorageBufferUsage) { // 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}}); + device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::StorageBuffer}, + {1, wgpu::ShaderStage::Fragment, wgpu::BindingType::ReadonlyStorageBuffer}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, 4}, {1, buffer, 256, 4}}); diff --git a/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp b/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp index 18673eaf60..40a63b3b8e 100644 --- a/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp +++ b/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp @@ -22,14 +22,14 @@ class GetBindGroupLayoutTests : public ValidationTest { static constexpr wgpu::ShaderStage kVisibilityAll = wgpu::ShaderStage::Compute | wgpu::ShaderStage::Fragment | wgpu::ShaderStage::Vertex; - wgpu::RenderPipeline RenderPipelineFromVertexShader(const char* shader) { + wgpu::RenderPipeline RenderPipelineFromFragmentShader(const char* shader) { wgpu::ShaderModule vsModule = - utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex, shader); - wgpu::ShaderModule fsModule = - utils::CreateShaderModule(device, utils::SingleShaderStage::Fragment, R"( + utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex, R"( #version 450 void main() { })"); + wgpu::ShaderModule fsModule = + utils::CreateShaderModule(device, utils::SingleShaderStage::Fragment, shader); utils::ComboRenderPipelineDescriptor descriptor(device); descriptor.layout = nullptr; @@ -93,7 +93,7 @@ TEST_F(GetBindGroupLayoutTests, SameObject) { // - shader stage visibility is All // - dynamic offsets is false TEST_F(GetBindGroupLayoutTests, DefaultShaderStageAndDynamicOffsets) { - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform UniformBuffer { vec4 pos; @@ -167,7 +167,6 @@ TEST_F(GetBindGroupLayoutTests, ComputePipeline) { TEST_F(GetBindGroupLayoutTests, BindingType) { wgpu::BindGroupLayoutBinding binding = {}; binding.binding = 0; - binding.visibility = kVisibilityAll; binding.hasDynamicOffset = false; binding.multisampled = false; @@ -175,9 +174,24 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { desc.bindingCount = 1; desc.bindings = &binding; + { + // Storage buffer binding is not supported in vertex shader. + binding.visibility = wgpu::ShaderStage::Compute | wgpu::ShaderStage::Fragment; + binding.type = wgpu::BindingType::StorageBuffer; + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( + #version 450 + layout(set = 0, binding = 0) buffer Storage { + vec4 pos; + }; + + void main() {})"); + EXPECT_EQ(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); + } + + binding.visibility = kVisibilityAll; { binding.type = wgpu::BindingType::UniformBuffer; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform Buffer { vec4 pos; @@ -187,21 +201,9 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { EXPECT_EQ(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); } - { - binding.type = wgpu::BindingType::StorageBuffer; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( - #version 450 - layout(set = 0, binding = 0) buffer Storage { - vec4 pos; - }; - - void main() {})"); - EXPECT_EQ(device.CreateBindGroupLayout(&desc).Get(), pipeline.GetBindGroupLayout(0).Get()); - } - { binding.type = wgpu::BindingType::ReadonlyStorageBuffer; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) readonly buffer Storage { vec4 pos; @@ -213,7 +215,7 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { { binding.type = wgpu::BindingType::SampledTexture; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform texture2D tex; @@ -223,7 +225,7 @@ TEST_F(GetBindGroupLayoutTests, BindingType) { { binding.type = wgpu::BindingType::Sampler; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform sampler samp; @@ -246,7 +248,7 @@ TEST_F(GetBindGroupLayoutTests, Multisampled) { { binding.multisampled = false; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform texture2D tex; @@ -258,7 +260,7 @@ TEST_F(GetBindGroupLayoutTests, Multisampled) { GTEST_SKIP() << "Multisampling unimplemented"; { binding.multisampled = true; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform texture2DMS tex; @@ -282,7 +284,7 @@ TEST_F(GetBindGroupLayoutTests, TextureDimension) { { binding.textureDimension = wgpu::TextureViewDimension::e1D; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform texture1D tex; @@ -292,7 +294,7 @@ TEST_F(GetBindGroupLayoutTests, TextureDimension) { { binding.textureDimension = wgpu::TextureViewDimension::e2D; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform texture2D tex; @@ -302,7 +304,7 @@ TEST_F(GetBindGroupLayoutTests, TextureDimension) { { binding.textureDimension = wgpu::TextureViewDimension::e2DArray; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform texture2DArray tex; @@ -312,7 +314,7 @@ TEST_F(GetBindGroupLayoutTests, TextureDimension) { { binding.textureDimension = wgpu::TextureViewDimension::e3D; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform texture3D tex; @@ -322,7 +324,7 @@ TEST_F(GetBindGroupLayoutTests, TextureDimension) { { binding.textureDimension = wgpu::TextureViewDimension::Cube; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform textureCube tex; @@ -332,7 +334,7 @@ TEST_F(GetBindGroupLayoutTests, TextureDimension) { { binding.textureDimension = wgpu::TextureViewDimension::CubeArray; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform textureCubeArray tex; @@ -356,7 +358,7 @@ TEST_F(GetBindGroupLayoutTests, TextureComponentType) { { binding.textureComponentType = wgpu::TextureComponentType::Float; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform texture2D tex; @@ -366,7 +368,7 @@ TEST_F(GetBindGroupLayoutTests, TextureComponentType) { { binding.textureComponentType = wgpu::TextureComponentType::Sint; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform itexture2D tex; @@ -376,7 +378,7 @@ TEST_F(GetBindGroupLayoutTests, TextureComponentType) { { binding.textureComponentType = wgpu::TextureComponentType::Uint; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform utexture2D tex; @@ -399,7 +401,7 @@ TEST_F(GetBindGroupLayoutTests, BindingIndices) { { binding.binding = 0; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform Buffer { vec4 pos; @@ -411,7 +413,7 @@ TEST_F(GetBindGroupLayoutTests, BindingIndices) { { binding.binding = 1; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 1) uniform Buffer { vec4 pos; @@ -423,7 +425,7 @@ TEST_F(GetBindGroupLayoutTests, BindingIndices) { { binding.binding = 2; - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 1) uniform Buffer { vec4 pos; @@ -571,7 +573,7 @@ TEST_F(GetBindGroupLayoutTests, ConflictingBindingTextureComponentType) { // Test it is an error to query an out of range bind group layout. TEST_F(GetBindGroupLayoutTests, OutOfRangeIndex) { - ASSERT_DEVICE_ERROR(RenderPipelineFromVertexShader(R"( + ASSERT_DEVICE_ERROR(RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform Buffer1 { vec4 pos1; @@ -579,7 +581,7 @@ TEST_F(GetBindGroupLayoutTests, OutOfRangeIndex) { void main() {})") .GetBindGroupLayout(kMaxBindGroups)); - ASSERT_DEVICE_ERROR(RenderPipelineFromVertexShader(R"( + ASSERT_DEVICE_ERROR(RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform Buffer1 { vec4 pos1; @@ -590,7 +592,7 @@ TEST_F(GetBindGroupLayoutTests, OutOfRangeIndex) { // Test that unused indices return the empty bind group layout. TEST_F(GetBindGroupLayoutTests, UnusedIndex) { - wgpu::RenderPipeline pipeline = RenderPipelineFromVertexShader(R"( + wgpu::RenderPipeline pipeline = RenderPipelineFromFragmentShader(R"( #version 450 layout(set = 0, binding = 0) uniform Buffer1 { vec4 pos1;