commit d757c300a49888b4a4eb51dee75173faf0b65f07
author Corentin Wallez <cwallez@chromium.org> Mon Jul 15 10:29:08 2019 +0000
committer Commit Bot service account <commit-bot@chromium.org> Mon Jul 15 10:29:08 2019 +0000
tree 71ab30bd43f9cac9c18cdea4bb01efe6d955b47b
parent 5de3e0aaa49ad4e5128f05eceaa9ecf8ec10fb23

Validate usage of dynamic at the BGL level.

Previously validation of the combination of dynamic with the binding
type was only done when creating the bind group when it should have been
done when the bind group layout is created.

BUG=dawn:55

Change-Id: I976f7e052f9737929fc05908af50e6ad5ceef397
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/8861
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/BindGroup.cpp

diff --git a/src/dawn_native/BindGroup.cpp b/src/dawn_native/BindGroup.cpp
index a6db7cf..205a158 100644
--- a/src/dawn_native/BindGroup.cpp
+++ b/src/dawn_native/BindGroup.cpp
@@ -132,17 +132,10 @@
                     DAWN_TRY(ValidateBufferBinding(device, binding, dawn::BufferUsageBit::Storage));
                     break;
                 case dawn::BindingType::SampledTexture:
-                    if (layoutInfo.dynamic[bindingIndex]) {
-                        return DAWN_VALIDATION_ERROR(
-                            "SampledTextures are expected to be not dynamic");
-                    }
                     DAWN_TRY(
                         ValidateTextureBinding(device, binding, dawn::TextureUsageBit::Sampled));
                     break;
                 case dawn::BindingType::Sampler:
-                    if (layoutInfo.dynamic[bindingIndex]) {
-                        return DAWN_VALIDATION_ERROR("Samplers are expected to be not dynamic");
-                    }
                     DAWN_TRY(ValidateSamplerBinding(device, binding));
                     break;
             }

src/dawn_native/BindGroupLayout.cpp

diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp
index 9dcdce4..e646359 100644
--- a/src/dawn_native/BindGroupLayout.cpp
+++ b/src/dawn_native/BindGroupLayout.cpp
@@ -31,7 +31,7 @@
 
         std::bitset<kMaxBindingsPerGroup> bindingsSet;
         for (uint32_t i = 0; i < descriptor->bindingCount; ++i) {
-            auto& binding = descriptor->bindings[i];
+            const BindGroupLayoutBinding& binding = descriptor->bindings[i];
             DAWN_TRY(ValidateShaderStageBit(binding.visibility));
             DAWN_TRY(ValidateBindingType(binding.type));
 
@@ -41,6 +41,19 @@
             if (bindingsSet[binding.binding]) {
                 return DAWN_VALIDATION_ERROR("some binding index was specified more than once");
             }
+
+            switch (binding.type) {
+                case dawn::BindingType::UniformBuffer:
+                case dawn::BindingType::StorageBuffer:
+                    break;
+                case dawn::BindingType::SampledTexture:
+                case dawn::BindingType::Sampler:
+                    if (binding.dynamic) {
+                        return DAWN_VALIDATION_ERROR("Samplers and textures cannot be dynamic");
+                    }
+                    break;
+            }
+
             bindingsSet.set(binding.binding);
         }
         return {};

src/tests/unittests/validation/BindGroupValidationTests.cpp

diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp
index a222a57..4b73cda 100644
--- a/src/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -422,13 +422,35 @@
 // This test verifies that the BindGroupLayout bindings are correctly validated, even if the
 // binding ids are out-of-order.
 TEST_F(BindGroupLayoutValidationTest, BindGroupBinding) {
-    auto layout = utils::MakeBindGroupLayout(
+    utils::MakeBindGroupLayout(
         device, {
                     {1, dawn::ShaderStageBit::Vertex, dawn::BindingType::UniformBuffer},
                     {0, dawn::ShaderStageBit::Vertex, dawn::BindingType::UniformBuffer},
                 });
 }
 
+// Check that dynamic = true is only allowed with buffer bindings.
+TEST_F(BindGroupLayoutValidationTest, DynamicAndTypeCompatibility) {
+    utils::MakeBindGroupLayout(
+        device, {
+                    {0, dawn::ShaderStageBit::Compute, dawn::BindingType::UniformBuffer, true},
+                });
+
+    utils::MakeBindGroupLayout(
+        device, {
+                    {0, dawn::ShaderStageBit::Compute, dawn::BindingType::StorageBuffer, true},
+                });
+
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+        device, {
+                    {0, dawn::ShaderStageBit::Compute, dawn::BindingType::SampledTexture, true},
+                }));
+
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+        device, {
+                    {0, dawn::ShaderStageBit::Compute, dawn::BindingType::Sampler, true},
+                }));
+}
 
 // This test verifies that the BindGroupLayout cache is successfully caching/deduplicating objects.
 //