Support multisampled depth texture bindings

Adds support for processing texture_depth_multisampled_2d bindings
reflected from Tint, and also removes Dawn restrictions against
multisampled depth. These restrictions were originally added in
https://dawn-review.googlesource.com/c/dawn/+/30240 to validate
against using a multisampled depth texture with a
comparison sampler. This is now disallowed by the language with
distinct binding types and builtins in WGSL. Previously with
SPIR-V, we inferred Depth if the texture was used
with a comparison sampler.

Also check Vulkan limits for supported sample counts.

Bug: dawn:1021, dawn:1030
Change-Id: I7233b16c14dc80d10a851cc4e786d5b05512b57a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/60020
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

@@ -130,18 +130,8 @@ namespace dawn_native {
                     viewDimension = texture.viewDimension;
                 }
 
-                if (texture.multisampled) {
+                if (texture.multisampled && viewDimension != wgpu::TextureViewDimension::e2D) {
-                    if (viewDimension != wgpu::TextureViewDimension::e2D) {
+                    return DAWN_VALIDATION_ERROR("Multisampled texture bindings must be 2D.");
-                        return DAWN_VALIDATION_ERROR("Multisampled texture bindings must be 2D.");
-                    }
-                    // TODO: This check should eventually become obsolete. According to the spec,
-                    // depth can be used with both regular and comparison sampling. As such, during
-                    // pipeline creation we have to check that if a comparison sampler is used
-                    // with a texture, that texture must be both depth and not multisampled.
-                    if (texture.sampleType == wgpu::TextureSampleType::Depth) {
-                        return DAWN_VALIDATION_ERROR(
-                            "Multisampled texture bindings must not be Depth.");
-                    }
                 }
             }
             if (entry.storageTexture.access != wgpu::StorageTextureAccess::Undefined) {

src/dawn_native/ShaderModule.cpp

@@ -155,6 +155,7 @@ namespace dawn_native {
                 case tint::inspector::ResourceBinding::ResourceType::kSampledTexture:
                 case tint::inspector::ResourceBinding::ResourceType::kMultisampledTexture:
                 case tint::inspector::ResourceBinding::ResourceType::kDepthTexture:
+                case tint::inspector::ResourceBinding::ResourceType::kDepthMultisampledTexture:
                     return BindingInfoType::Texture;
                 case tint::inspector::ResourceBinding::ResourceType::kReadOnlyStorageTexture:
                 case tint::inspector::ResourceBinding::ResourceType::kWriteOnlyStorageTexture:
@@ -747,10 +748,6 @@ namespace dawn_native {
                                 SpirvBaseTypeToSampleTypeBit(textureComponentType);
 
                             if (imageType.depth) {
-                                if (imageType.ms) {
-                                    return DAWN_VALIDATION_ERROR(
-                                        "Multisampled depth textures aren't supported");
-                                }
                                 if ((info->texture.compatibleSampleTypes & SampleTypeBit::Float) ==
                                     0) {
                                     return DAWN_VALIDATION_ERROR(
@@ -1129,15 +1126,21 @@ namespace dawn_native {
                             info->texture.viewDimension =
                                 TintTextureDimensionToTextureViewDimension(resource.dim);
                             if (resource.resource_type ==
-                                tint::inspector::ResourceBinding::ResourceType::kDepthTexture) {
+                                    tint::inspector::ResourceBinding::ResourceType::kDepthTexture ||
+                                resource.resource_type ==
+                                    tint::inspector::ResourceBinding::ResourceType::
+                                        kDepthMultisampledTexture) {
                                 info->texture.compatibleSampleTypes = SampleTypeBit::Depth;
                             } else {
                                 info->texture.compatibleSampleTypes =
                                     TintSampledKindToSampleTypeBit(resource.sampled_kind);
                             }
-                            info->texture.multisampled = resource.resource_type ==
+                            info->texture.multisampled =
-                                                         tint::inspector::ResourceBinding::
+                                resource.resource_type == tint::inspector::ResourceBinding::
-                                                             ResourceType::kMultisampledTexture;
+                                                              ResourceType::kMultisampledTexture ||
+                                resource.resource_type ==
+                                    tint::inspector::ResourceBinding::ResourceType::
+                                        kDepthMultisampledTexture;
 
                             break;
                         case BindingInfoType::StorageTexture:

src/dawn_native/vulkan/AdapterVk.cpp

@@ -223,6 +223,16 @@ namespace dawn_native { namespace vulkan {
         if (limits.maxColorAttachments < kMaxColorAttachments) {
             return DAWN_INTERNAL_ERROR("Insufficient Vulkan limits for maxColorAttachments");
         }
+        if (!IsSubset(VkSampleCountFlags(VK_SAMPLE_COUNT_1_BIT | VK_SAMPLE_COUNT_4_BIT),
+                      limits.framebufferColorSampleCounts)) {
+            return DAWN_INTERNAL_ERROR(
+                "Insufficient Vulkan limits for framebufferColorSampleCounts");
+        }
+        if (!IsSubset(VkSampleCountFlags(VK_SAMPLE_COUNT_1_BIT | VK_SAMPLE_COUNT_4_BIT),
+                      limits.framebufferDepthSampleCounts)) {
+            return DAWN_INTERNAL_ERROR(
+                "Insufficient Vulkan limits for framebufferDepthSampleCounts");
+        }
 
         // Only check maxFragmentCombinedOutputResources on mobile GPUs. Desktop GPUs drivers seem
         // to put incorrect values for this limit with things like 8 or 16 when they can do bindless

src/tests/end2end/MultisampledSamplingTests.cpp

@@ -50,6 +50,9 @@ class MultisampledSamplingTest : public DawnTest {
     void SetUp() override {
         DawnTest::SetUp();
 
+        // TODO(crbug.com/dawn/1030): Compute pipeline compilation crashes.
+        DAWN_SUPPRESS_TEST_IF(IsLinux() && IsVulkan() && IsIntel());
+
         {
             utils::ComboRenderPipelineDescriptor desc;
 
@@ -94,7 +97,7 @@ class MultisampledSamplingTest : public DawnTest {
             desc.compute.entryPoint = "main";
             desc.compute.module = utils::CreateShaderModule(device, R"(
                 [[group(0), binding(0)]] var texture0 : texture_multisampled_2d<f32>;
-                [[group(0), binding(1)]] var texture1 : texture_multisampled_2d<f32>;
+                [[group(0), binding(1)]] var texture1 : texture_depth_multisampled_2d;
 
                 [[block]] struct Results {
                     colorSamples : array<f32, 4>;
@@ -105,7 +108,7 @@ class MultisampledSamplingTest : public DawnTest {
                 [[stage(compute), workgroup_size(1)]] fn main() {
                     for (var i : i32 = 0; i < 4; i = i + 1) {
                         results.colorSamples[i] = textureLoad(texture0, vec2<i32>(0, 0), i).x;
-                        results.depthSamples[i] = textureLoad(texture1, vec2<i32>(0, 0), i).x;
+                        results.depthSamples[i] = textureLoad(texture1, vec2<i32>(0, 0), i);
                     }
                 })");
 
@@ -123,6 +126,8 @@ class MultisampledSamplingTest : public DawnTest {
 // must cover both the X and Y coordinates of the sample position (no false positives if
 // it covers the X position but not the Y, or vice versa).
 TEST_P(MultisampledSamplingTest, SamplePositions) {
+    DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator"));
+
     static constexpr wgpu::Extent3D kTextureSize = {1, 1, 1};
 
     wgpu::Texture colorTexture;
@@ -206,16 +211,12 @@ TEST_P(MultisampledSamplingTest, SamplePositions) {
 
             wgpu::ComputePassEncoder computePassEncoder = commandEncoder.BeginComputePass();
             computePassEncoder.SetPipeline(checkSamplePipeline);
-            // TODO(crbug.com/dawn/1021): Disallow using float/unfilterable-float with depth
+            computePassEncoder.SetBindGroup(
-            // textures.
+                0, utils::MakeBindGroup(
-            wgpu::BindGroup bindGroup;
+                       device, checkSamplePipeline.GetBindGroupLayout(0),
-            EXPECT_DEPRECATION_WARNING(
+                       {{0, colorView},
-                bindGroup = utils::MakeBindGroup(
+                        {1, depthView},
-                    device, checkSamplePipeline.GetBindGroupLayout(0),
+                        {2, outputBuffer, alignedResultSize * sampleOffset, kResultSize}}));
-                    {{0, colorView},
-                     {1, depthView},
-                     {2, outputBuffer, alignedResultSize * sampleOffset, kResultSize}}));
-            computePassEncoder.SetBindGroup(0, bindGroup);
             computePassEncoder.Dispatch(1);
             computePassEncoder.EndPass();
         }

src/tests/unittests/validation/BindGroupValidationTests.cpp

@@ -1348,35 +1348,35 @@ TEST_F(BindGroupLayoutValidationTest, MultisampledTextureViewDimension) {
                 }));
 }
 
-// Test that multisampled textures cannot be DepthComparison
+// Test that multisampled texture bindings are valid
-TEST_F(BindGroupLayoutValidationTest, MultisampledTextureComponentType) {
+TEST_F(BindGroupLayoutValidationTest, MultisampledTextureSampleType) {
-    // Multisampled float component type works.
+    // Multisampled float sample type works.
     utils::MakeBindGroupLayout(device,
                                {
                                    {0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Float,
                                     wgpu::TextureViewDimension::e2D, true},
                                });
 
-    // Multisampled uint component type works.
+    // Multisampled uint sample type works.
     utils::MakeBindGroupLayout(device,
                                {
                                    {0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Uint,
                                     wgpu::TextureViewDimension::e2D, true},
                                });
 
-    // Multisampled sint component type works.
+    // Multisampled sint sample type works.
     utils::MakeBindGroupLayout(device,
                                {
                                    {0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Sint,
                                     wgpu::TextureViewDimension::e2D, true},
                                });
 
-    // Multisampled depth comparison component typeworks.
+    // Multisampled depth sample type works.
-    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+    utils::MakeBindGroupLayout(device,
-        device, {
+                               {
-                    {0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Depth,
+                                   {0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Depth,
-                     wgpu::TextureViewDimension::e2D, true},
+                                    wgpu::TextureViewDimension::e2D, true},
-                }));
+                               });
 }
 
 constexpr uint64_t kBufferSize = 3 * kMinUniformBufferOffsetAlignment + 8;