Add wgpu::TextureComponentType::DepthComparison

And deprecate using ::Float in the bind group layout for
"shadow textures" in the pipeline (along with a deprecation test).

Adds the ability to be used with DepthComparison only to depth textures,
this could potentially a breaking change if users where doing
depth-comparison on float32 textures but that's not supported in WebGPU.

Bug: dawn:527
Change-Id: Ib28b0443e3002e0aa2811713b9e843c2417e13e7
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/30240
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>

dawn.json

@@ -1549,7 +1549,8 @@
         "values": [
             {"value": 0, "name": "float"},
             {"value": 1, "name": "sint"},
-            {"value": 2, "name": "uint"}
+            {"value": 2, "name": "uint"},
+            {"value": 3, "name": "depth comparison"}
         ]
     },
     "texture copy view": {

src/dawn_native/BindGroupLayout.cpp

@@ -121,6 +121,10 @@ namespace dawn_native {
                     if (viewDimension != wgpu::TextureViewDimension::e2D) {
                         return DAWN_VALIDATION_ERROR("Multisampled binding must be 2D.");
                     }
+                    if (entry.textureComponentType == wgpu::TextureComponentType::DepthComparison) {
+                        return DAWN_VALIDATION_ERROR(
+                            "Multisampled binding must not be DepthComparison.");
+                    }
                     break;
 
                 case wgpu::BindingType::WriteonlyStorageTexture:

src/dawn_native/Format.cpp

@@ -36,6 +36,7 @@ namespace dawn_native {
             case wgpu::TextureComponentType::Float:
             case wgpu::TextureComponentType::Sint:
             case wgpu::TextureComponentType::Uint:
+            case wgpu::TextureComponentType::DepthComparison:
                 // When the compiler complains that you need to add a case statement here, please
                 // also add a corresponding static assert below!
                 break;
@@ -55,6 +56,11 @@ namespace dawn_native {
                           static_cast<ComponentTypeBit>(
                               1 << static_cast<uint32_t>(wgpu::TextureComponentType::Sint)),
                       "");
+        static_assert(
+            ComponentTypeBit::DepthComparison ==
+                static_cast<ComponentTypeBit>(
+                    1 << static_cast<uint32_t>(wgpu::TextureComponentType::DepthComparison)),
+            "");
         return static_cast<ComponentTypeBit>(1 << static_cast<uint32_t>(type));
     }
 
@@ -157,7 +163,8 @@ namespace dawn_native {
             internalFormat.firstAspect.block.height = 1;
             internalFormat.firstAspect.baseType = wgpu::TextureComponentType::Float;
             if (isDepthSampleable) {
-                internalFormat.firstAspect.supportedComponentTypes = ComponentTypeBit::Float;
+                internalFormat.firstAspect.supportedComponentTypes =
+                    ComponentTypeBit::Float | ComponentTypeBit::DepthComparison;
             } else {
                 internalFormat.firstAspect.supportedComponentTypes = ComponentTypeBit::None;
             }

src/dawn_native/Format.h

@@ -34,6 +34,7 @@ namespace dawn_native {
         Float = 0x1,
         Sint = 0x2,
         Uint = 0x4,
+        DepthComparison = 0x8,
     };
 
     // Converts an wgpu::TextureComponentType to its bitmask representation.

src/dawn_native/Pipeline.cpp

@@ -22,7 +22,7 @@
 
 namespace dawn_native {
 
-    MaybeError ValidateProgrammableStageDescriptor(const DeviceBase* device,
+    MaybeError ValidateProgrammableStageDescriptor(DeviceBase* device,
                                                    const ProgrammableStageDescriptor* descriptor,
                                                    const PipelineLayoutBase* layout,
                                                    SingleShaderStage stage) {
@@ -36,7 +36,7 @@ namespace dawn_native {
         if (layout != nullptr) {
             const EntryPointMetadata& metadata =
                 module->GetEntryPoint(descriptor->entryPoint, stage);
-            DAWN_TRY(ValidateCompatibilityWithPipelineLayout(metadata, layout));
+            DAWN_TRY(ValidateCompatibilityWithPipelineLayout(device, metadata, layout));
         }
         return {};
     }

src/dawn_native/Pipeline.h

@@ -28,7 +28,7 @@
 
 namespace dawn_native {
 
-    MaybeError ValidateProgrammableStageDescriptor(const DeviceBase* device,
+    MaybeError ValidateProgrammableStageDescriptor(DeviceBase* device,
                                                    const ProgrammableStageDescriptor* descriptor,
                                                    const PipelineLayoutBase* layout,
                                                    SingleShaderStage stage);

src/dawn_native/PipelineLayout.cpp

@@ -207,7 +207,8 @@ namespace dawn_native {
         for (const StageAndDescriptor& stage : stages) {
             const EntryPointMetadata& metadata =
                 stage.second->module->GetEntryPoint(stage.second->entryPoint, stage.first);
-            ASSERT(ValidateCompatibilityWithPipelineLayout(metadata, pipelineLayout).IsSuccess());
+            ASSERT(ValidateCompatibilityWithPipelineLayout(device, metadata, pipelineLayout)
+                       .IsSuccess());
         }
 
         return pipelineLayout;

src/dawn_native/ShaderModule.cpp

@@ -369,7 +369,8 @@ namespace dawn_native {
             return std::move(result);
         }
 
-        MaybeError ValidateCompatibilityWithBindGroupLayout(BindGroupIndex group,
+        MaybeError ValidateCompatibilityWithBindGroupLayout(DeviceBase* device,
+                                                            BindGroupIndex group,
                                                             const EntryPointMetadata& entryPoint,
                                                             const BindGroupLayoutBase* layout) {
             const BindGroupLayoutBase::BindingMap& layoutBindings = layout->GetBindingMap();
@@ -424,11 +425,23 @@ namespace dawn_native {
                     case wgpu::BindingType::SampledTexture:
                     case wgpu::BindingType::MultisampledTexture: {
                         if (layoutInfo.textureComponentType != shaderInfo.textureComponentType) {
+                            // TODO(dawn:527): Remove once the deprecation timeline is complete.
+                            if (layoutInfo.textureComponentType ==
+                                    wgpu::TextureComponentType::Float &&
+                                shaderInfo.textureComponentType ==
+                                    wgpu::TextureComponentType::DepthComparison) {
+                                device->EmitDeprecationWarning(
+                                    "Using depth texture in the shader with "
+                                    "TextureComponentType::Float is deprecated use "
+                                    "TextureComponentType::DepthComparison in the bind group "
+                                    "layout instead.");
+                            } else {
                                 return DAWN_VALIDATION_ERROR(
                                     "The textureComponentType of the bind group layout entry is "
                                     "different from " +
                                     GetShaderDeclarationString(group, bindingNumber));
                             }
+                        }
 
                         if (layoutInfo.viewDimension != shaderInfo.viewDimension) {
                             return DAWN_VALIDATION_ERROR(
@@ -554,11 +567,26 @@ namespace dawn_native {
                                 SpirvDimToTextureViewDimension(imageType.dim, imageType.arrayed);
                             info->textureComponentType =
                                 SpirvBaseTypeToTextureComponentType(textureComponentType);
+
                             if (imageType.ms) {
                                 info->type = wgpu::BindingType::MultisampledTexture;
                             } else {
                                 info->type = wgpu::BindingType::SampledTexture;
                             }
+
+                            if (imageType.depth) {
+                                if (imageType.ms) {
+                                    return DAWN_VALIDATION_ERROR(
+                                        "Multisampled depth textures aren't supported");
+                                }
+                                if (info->textureComponentType !=
+                                    wgpu::TextureComponentType::Float) {
+                                    return DAWN_VALIDATION_ERROR(
+                                        "Depth textures must have a float type");
+                                }
+                                info->textureComponentType =
+                                    wgpu::TextureComponentType::DepthComparison;
+                            }
                             break;
                         }
                         case wgpu::BindingType::StorageBuffer: {
@@ -600,7 +628,11 @@ namespace dawn_native {
                             }
                             if (imageType.ms) {
                                 return DAWN_VALIDATION_ERROR(
-                                    "Multisampled storage texture aren't supported");
+                                    "Multisampled storage textures aren't supported");
+                            }
+                            if (imageType.depth) {
+                                return DAWN_VALIDATION_ERROR(
+                                    "Depth storage textures aren't supported");
                             }
                             info->storageTextureFormat = storageTextureFormat;
                             info->viewDimension =
@@ -749,10 +781,11 @@ namespace dawn_native {
         return bufferSizes;
     }
 
-    MaybeError ValidateCompatibilityWithPipelineLayout(const EntryPointMetadata& entryPoint,
+    MaybeError ValidateCompatibilityWithPipelineLayout(DeviceBase* device,
+                                                       const EntryPointMetadata& entryPoint,
                                                        const PipelineLayoutBase* layout) {
         for (BindGroupIndex group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {
-            DAWN_TRY(ValidateCompatibilityWithBindGroupLayout(group, entryPoint,
+            DAWN_TRY(ValidateCompatibilityWithBindGroupLayout(device, group, entryPoint,
                                                               layout->GetBindGroupLayout(group)));
         }
 

src/dawn_native/ShaderModule.h

@@ -41,7 +41,8 @@ namespace dawn_native {
 
     MaybeError ValidateShaderModuleDescriptor(DeviceBase* device,
                                               const ShaderModuleDescriptor* descriptor);
-    MaybeError ValidateCompatibilityWithPipelineLayout(const EntryPointMetadata& entryPoint,
+    MaybeError ValidateCompatibilityWithPipelineLayout(DeviceBase* device,
+                                                       const EntryPointMetadata& entryPoint,
                                                        const PipelineLayoutBase* layout);
 
     RequiredBufferSizes ComputeRequiredBufferSizesForLayout(const EntryPointMetadata& entryPoint,

src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp

@@ -71,6 +71,9 @@ namespace dawn_native { namespace d3d12 {
                 case wgpu::TextureComponentType::Float:
                     resolveParameters.ResolveMode = D3D12_RESOLVE_MODE_AVERAGE;
                     break;
+
+                case wgpu::TextureComponentType::DepthComparison:
+                    UNREACHABLE();
             }
 
             resolveParameters.SubresourceCount = 1;

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -949,6 +949,9 @@ namespace dawn_native { namespace opengl {
                             gl.ClearBufferiv(GL_COLOR, i, appliedClearColor.data());
                             break;
                         }
+
+                        case wgpu::TextureComponentType::DepthComparison:
+                            UNREACHABLE();
                     }
                 }
 

src/dawn_native/vulkan/CommandBufferVk.cpp

@@ -334,6 +334,9 @@ namespace dawn_native { namespace vulkan {
                             }
                             break;
                         }
+
+                        case wgpu::TextureComponentType::DepthComparison:
+                            UNREACHABLE();
                     }
                     attachmentCount++;
                 }

src/dawn_native/vulkan/TextureVk.cpp

@@ -1058,6 +1058,8 @@ namespace dawn_native { namespace vulkan {
                             clearColorValue.uint32[2] = uClearColor;
                             clearColorValue.uint32[3] = uClearColor;
                             break;
+                        case wgpu::TextureComponentType::DepthComparison:
+                            UNREACHABLE();
                     }
                     device->fn.CmdClearColorImage(recordingContext->commandBuffer, GetHandle(),
                                                   VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,

src/tests/end2end/DeprecatedAPITests.cpp

@@ -118,6 +118,50 @@ TEST_P(DeprecationTests, BGLEntryMultisampledBooleanTracking) {
     utils::MakeBindGroup(device, bgl, {{0, texture4Sample.CreateView()}});
 }
 
+// Test that compiling a pipeline with TextureComponentType::Float in the BGL when ::DepthComparison
+// is expected emits a deprecation warning but isn't an error.
+TEST_P(DeprecationTests, TextureComponentTypeFloatWhenDepthComparisonIsExpected) {
+    wgpu::ShaderModule module =
+        utils::CreateShaderModule(device, utils::SingleShaderStage::Compute, R"(
+            #version 450
+            layout(set = 0, binding = 0) uniform samplerShadow samp;
+            layout(set = 0, binding = 1) uniform texture2D tex;
+
+            void main() {
+                texture(sampler2DShadow(tex, samp), vec3(0.5, 0.5, 0.5));
+            }
+        )");
+
+    {
+        wgpu::BindGroupLayout goodBgl = utils::MakeBindGroupLayout(
+            device,
+            {{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ComparisonSampler},
+             {1, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture, false, 0, false,
+              wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::DepthComparison}});
+
+        wgpu::ComputePipelineDescriptor goodDesc;
+        goodDesc.layout = utils::MakeBasicPipelineLayout(device, &goodBgl);
+        goodDesc.computeStage.module = module;
+        goodDesc.computeStage.entryPoint = "main";
+
+        device.CreateComputePipeline(&goodDesc);
+    }
+
+    {
+        wgpu::BindGroupLayout badBgl = utils::MakeBindGroupLayout(
+            device, {{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ComparisonSampler},
+                     {1, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture, false, 0,
+                      false, wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::Float}});
+
+        wgpu::ComputePipelineDescriptor badDesc;
+        badDesc.layout = utils::MakeBasicPipelineLayout(device, &badBgl);
+        badDesc.computeStage.module = module;
+        badDesc.computeStage.entryPoint = "main";
+
+        EXPECT_DEPRECATION_WARNING(device.CreateComputePipeline(&badDesc));
+    }
+}
+
 DAWN_INSTANTIATE_TEST(DeprecationTests,
                       D3D12Backend(),
                       MetalBackend(),

src/tests/end2end/DepthSamplingTests.cpp

@@ -151,8 +151,10 @@ class DepthSamplingTest : public DawnTest {
         // TODO(dawn:367): Cannot use GetBindGroupLayout for comparison samplers without shader
         // reflection data.
         wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
-            device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ComparisonSampler},
+            device,
-                     {1, wgpu::ShaderStage::Fragment, wgpu::BindingType::SampledTexture},
+            {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::ComparisonSampler},
+             {1, wgpu::ShaderStage::Fragment, wgpu::BindingType::SampledTexture, false, 0, false,
+              wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::DepthComparison},
              {2, wgpu::ShaderStage::Fragment, wgpu::BindingType::UniformBuffer}});
 
         utils::ComboRenderPipelineDescriptor pipelineDescriptor(device);
@@ -185,8 +187,10 @@ class DepthSamplingTest : public DawnTest {
 
         // TODO(dawn:367): Cannot use GetBindGroupLayout without shader reflection data.
         wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
-            device, {{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ComparisonSampler},
+            device,
-                     {1, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture},
+            {{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ComparisonSampler},
+             {1, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture, false, 0, false,
+              wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::DepthComparison},
              {2, wgpu::ShaderStage::Compute, wgpu::BindingType::UniformBuffer},
              {3, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer}});
 

src/tests/unittests/validation/BindGroupValidationTests.cpp

@@ -345,6 +345,41 @@ TEST_F(BindGroupValidationTest, SamplingDepthTexture) {
     }
 }
 
+// Check that a texture must have a correct format for DepthComparison
+TEST_F(BindGroupValidationTest, TextureComponentTypeDepthComparison) {
+    wgpu::BindGroupLayout depthLayout = utils::MakeBindGroupLayout(
+        device,
+        {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::SampledTexture, false, 0, false,
+          wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::DepthComparison}});
+
+    // Control case: setting a depth texture works.
+    wgpu::Texture depthTexture =
+        CreateTexture(wgpu::TextureUsage::Sampled, wgpu::TextureFormat::Depth32Float, 1);
+    utils::MakeBindGroup(device, depthLayout, {{0, depthTexture.CreateView()}});
+
+    // Error case: setting a Float typed texture view fails.
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroup(device, depthLayout, {{0, mSampledTextureView}}));
+}
+
+// Check that a depth texture is allowed to be used for both TextureComponentType::Float and
+// ::DepthComparison
+TEST_F(BindGroupValidationTest, TextureComponentTypeForDepthTexture) {
+    wgpu::BindGroupLayout depthLayout = utils::MakeBindGroupLayout(
+        device,
+        {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::SampledTexture, false, 0, false,
+          wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::DepthComparison}});
+
+    wgpu::BindGroupLayout floatLayout = utils::MakeBindGroupLayout(
+        device, {{0, wgpu::ShaderStage::Fragment, wgpu::BindingType::SampledTexture, false, 0,
+                  false, wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::Float}});
+
+    wgpu::Texture depthTexture =
+        CreateTexture(wgpu::TextureUsage::Sampled, wgpu::TextureFormat::Depth32Float, 1);
+
+    utils::MakeBindGroup(device, depthLayout, {{0, depthTexture.CreateView()}});
+    utils::MakeBindGroup(device, floatLayout, {{0, depthTexture.CreateView()}});
+}
+
 // Check that a texture must have the correct dimension
 TEST_F(BindGroupValidationTest, TextureDimension) {
     wgpu::BindGroupLayout layout = utils::MakeBindGroupLayout(
@@ -900,7 +935,7 @@ TEST_F(BindGroupLayoutValidationTest, DynamicBufferNumberLimit) {
 }
 
 // Test that multisampled textures must be 2D sampled textures
-TEST_F(BindGroupLayoutValidationTest, MultisampledTextures) {
+TEST_F(BindGroupLayoutValidationTest, MultisampledTextureViewDimension) {
     // Multisampled 2D texture works.
     utils::MakeBindGroupLayout(
         device, {
@@ -958,6 +993,45 @@ TEST_F(BindGroupLayoutValidationTest, MultisampledTextures) {
                 }));
 }
 
+// Test that multisampled textures cannot be DepthComparison
+TEST_F(BindGroupLayoutValidationTest, MultisampledTextureComponentType) {
+    // Multisampled float component type works.
+    utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::MultisampledTexture, false,
+                     0, false, wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::Float},
+                });
+
+    // Multisampled float (defaulted) component type works.
+    utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::MultisampledTexture, false,
+                     0, false, wgpu::TextureViewDimension::e2D},
+                });
+
+    // Multisampled uint component type works.
+    utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::MultisampledTexture, false,
+                     0, false, wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::Uint},
+                });
+
+    // Multisampled sint component type works.
+    utils::MakeBindGroupLayout(
+        device, {
+                    {0, wgpu::ShaderStage::Compute, wgpu::BindingType::MultisampledTexture, false,
+                     0, false, wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::Sint},
+                });
+
+    // Multisampled depth comparison component typeworks.
+    ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
+        device,
+        {
+            {0, wgpu::ShaderStage::Compute, wgpu::BindingType::MultisampledTexture, false, 0, false,
+             wgpu::TextureViewDimension::e2D, wgpu::TextureComponentType::DepthComparison},
+        }));
+}
+
 // Test that it is an error to pass multisampled=true for non-texture bindings
 TEST_F(BindGroupLayoutValidationTest, MultisampledMustBeTexture) {
     // Base: Multisampled 2D texture works.