encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Disallow storage buffer binding in vertex shader 

Writable storage buffer in vertex shader is an optional feature.
It is not supported in many devices/OSes. WebGPU doesn't support
writable storage buffer in vertex shader. This change generates an
error for storage buffer binding for vertex shader stage, in order
to disallow writable storage buffer in vertex shader.

This change also adds a validation test and revises existing
end2end tests and validation tests accordingly.

BUG=dawn:180

Change-Id: I9def918d19f65aab45a31acb985c1a0a09c97ca8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/14521
Commit-Queue: Yunchao He <yunchao.he@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
This commit is contained in:
Yunchao He 2019-12-19 18:50:18 +00:00 committed by Commit Bot service account
parent bfb2a5740f
commit d28b578b6b
7 changed files with 83 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/dawn_native/BindGroupLayout.cpp
View File

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

11
src/dawn_native/PipelineLayout.cpp
View File

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

4
src/tests/end2end/GpuMemorySynchronizationTests.cpp
View File

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

14
src/tests/end2end/OpArrayLengthTests.cpp
View File

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

16
src/tests/unittests/validation/BindGroupValidationTests.cpp
View File

@ -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.

6
src/tests/unittests/validation/CommandBufferValidationTests.cpp
View File

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

80
src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp
View File

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