commit 8ab3719fc48688d7a641b4bc3298b761c8a81749
author Austin Eng <enga@chromium.org> Tue Jun 23 00:22:24 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Tue Jun 23 00:22:24 2020 +0000
tree e9d0fc02aac44575b364f0b7b44db5a94cda4c22
parent 4cf65bcab596fe12b5b72f1f8eab902cea84a5c7

Fix validation of multisampled BGL bindings

Bug: chromium:1097501, chromium:1097514
Change-Id: I18f03398488cc3b5adf6755989e787e117d8a004
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/23640
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Commit-Queue: Austin Eng <enga@chromium.org>

src/dawn_native/BindGroupLayout.cpp

diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp
index f63fb71..fce8b90 100644
--- a/src/dawn_native/BindGroupLayout.cpp
+++ b/src/dawn_native/BindGroupLayout.cpp
@@ -141,18 +141,22 @@
     MaybeError ValidateBindingCanBeMultisampled(wgpu::BindingType bindingType,
                                                 wgpu::TextureViewDimension viewDimension) {
         switch (bindingType) {
-            case wgpu::BindingType::ReadonlyStorageTexture:
-            case wgpu::BindingType::WriteonlyStorageTexture:
-                return DAWN_VALIDATION_ERROR("Storage textures may not be multisampled");
-
             case wgpu::BindingType::SampledTexture:
                 break;
 
+            case wgpu::BindingType::ReadonlyStorageTexture:
+            case wgpu::BindingType::WriteonlyStorageTexture:
+                return DAWN_VALIDATION_ERROR("Storage texture bindings may not be multisampled");
+
             case wgpu::BindingType::StorageBuffer:
             case wgpu::BindingType::UniformBuffer:
             case wgpu::BindingType::ReadonlyStorageBuffer:
+                return DAWN_VALIDATION_ERROR("Buffer bindings may not be multisampled");
+
             case wgpu::BindingType::Sampler:
             case wgpu::BindingType::ComparisonSampler:
+                return DAWN_VALIDATION_ERROR("Sampler bindings may not be multisampled");
+
             case wgpu::BindingType::StorageTexture:
             default:
                 UNREACHABLE();
@@ -164,14 +168,14 @@
                 break;
 
             case wgpu::TextureViewDimension::e2DArray:
-                return DAWN_VALIDATION_ERROR("2D array textures may not be multisampled");
+                return DAWN_VALIDATION_ERROR("2D array texture bindings may not be multisampled");
 
             case wgpu::TextureViewDimension::Cube:
             case wgpu::TextureViewDimension::CubeArray:
-                return DAWN_VALIDATION_ERROR("Cube textures may not be multisampled");
+                return DAWN_VALIDATION_ERROR("Cube texture bindings may not be multisampled");
 
             case wgpu::TextureViewDimension::e3D:
-                return DAWN_VALIDATION_ERROR("3D textures may not be multisampled");
+                return DAWN_VALIDATION_ERROR("3D texture bindings may not be multisampled");
 
             case wgpu::TextureViewDimension::e1D:
             case wgpu::TextureViewDimension::Undefined:

src/tests/unittests/validation/BindGroupValidationTests.cpp

diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp
index 4f8d783..a3bd47c 100644
--- a/src/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -727,6 +727,7 @@
     }
 }
 
+// Test that multisampled textures must be 2D sampled textures
 TEST_F(BindGroupLayoutValidationTest, MultisampledTextures) {
     // Multisampled 2D texture works.
     utils::MakeBindGroupLayout(
@@ -742,6 +743,13 @@
                      wgpu::TextureViewDimension::Undefined},
                 });
 
+    // Multisampled 2D storage texture is invalid.
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageTexture,
+                     false, true, wgpu::TextureViewDimension::e2D},
+                }));
+
     // Multisampled 2D array texture is invalid.
     ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
         device, {
@@ -771,6 +779,34 @@
                 }));
 }
 
+// Test that it is an error to pass multisampled=true for non-texture bindings
+TEST_F(BindGroupLayoutValidationTest, MultisampledMustBeTexture) {
+    // Base: Multisampled 2D texture works.
+    utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture, false, true,
+                     wgpu::TextureViewDimension::e2D},
+                });
+
+    // Multisampled uniform buffer binding is invalid
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::UniformBuffer, false, true},
+                }));
+
+    // Multisampled storage buffer binding is invalid
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer, false, true},
+                }));
+
+    // Multisampled sampler binding is invalid
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::Sampler, false, true},
+                }));
+}
+
 constexpr uint64_t kBufferSize = 3 * kMinDynamicBufferOffsetAlignment + 8;
 constexpr uint32_t kBindingSize = 9;