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>
diff --git a/dawn.json b/dawn.json
index 6dd1304..6f22218 100644
--- a/dawn.json
+++ b/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": {
diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp
index e59aa9d..93e6777 100644
--- a/src/dawn_native/BindGroupLayout.cpp
+++ b/src/dawn_native/BindGroupLayout.cpp
@@ -121,6 +121,10 @@
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:
diff --git a/src/dawn_native/Format.cpp b/src/dawn_native/Format.cpp
index 5bf672c..90d3719 100644
--- a/src/dawn_native/Format.cpp
+++ b/src/dawn_native/Format.cpp
@@ -36,6 +36,7 @@
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 @@
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 @@
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;
}
diff --git a/src/dawn_native/Format.h b/src/dawn_native/Format.h
index 392254e..ec4d6b4 100644
--- a/src/dawn_native/Format.h
+++ b/src/dawn_native/Format.h
@@ -34,6 +34,7 @@
Float = 0x1,
Sint = 0x2,
Uint = 0x4,
+ DepthComparison = 0x8,
};
// Converts an wgpu::TextureComponentType to its bitmask representation.
diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp
index d7ebbe2..928f7f2 100644
--- a/src/dawn_native/Pipeline.cpp
+++ b/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 @@
if (layout != nullptr) {
const EntryPointMetadata& metadata =
module->GetEntryPoint(descriptor->entryPoint, stage);
- DAWN_TRY(ValidateCompatibilityWithPipelineLayout(metadata, layout));
+ DAWN_TRY(ValidateCompatibilityWithPipelineLayout(device, metadata, layout));
}
return {};
}
diff --git a/src/dawn_native/Pipeline.h b/src/dawn_native/Pipeline.h
index 9bc249c..395410a 100644
--- a/src/dawn_native/Pipeline.h
+++ b/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);
diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp
index 2998194..883d9f9 100644
--- a/src/dawn_native/PipelineLayout.cpp
+++ b/src/dawn_native/PipelineLayout.cpp
@@ -207,7 +207,8 @@
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;
diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index 25c9407..f81461a 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -369,7 +369,8 @@
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,10 +425,22 @@
case wgpu::BindingType::SampledTexture:
case wgpu::BindingType::MultisampledTexture: {
if (layoutInfo.textureComponentType != shaderInfo.textureComponentType) {
- return DAWN_VALIDATION_ERROR(
- "The textureComponentType of the bind group layout entry is "
- "different from " +
- GetShaderDeclarationString(group, bindingNumber));
+ // 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) {
@@ -554,11 +567,26 @@
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 @@
}
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 @@
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)));
}
diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h
index 5178412..ef1f8ad 100644
--- a/src/dawn_native/ShaderModule.h
+++ b/src/dawn_native/ShaderModule.h
@@ -41,7 +41,8 @@
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,
diff --git a/src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp b/src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp
index 12216f5..f24a39f 100644
--- a/src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp
+++ b/src/dawn_native/d3d12/RenderPassBuilderD3D12.cpp
@@ -71,6 +71,9 @@
case wgpu::TextureComponentType::Float:
resolveParameters.ResolveMode = D3D12_RESOLVE_MODE_AVERAGE;
break;
+
+ case wgpu::TextureComponentType::DepthComparison:
+ UNREACHABLE();
}
resolveParameters.SubresourceCount = 1;
diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp
index f75f854..65dd9e0 100644
--- a/src/dawn_native/opengl/CommandBufferGL.cpp
+++ b/src/dawn_native/opengl/CommandBufferGL.cpp
@@ -949,6 +949,9 @@
gl.ClearBufferiv(GL_COLOR, i, appliedClearColor.data());
break;
}
+
+ case wgpu::TextureComponentType::DepthComparison:
+ UNREACHABLE();
}
}
diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp
index 2138f98..7b02f62 100644
--- a/src/dawn_native/vulkan/CommandBufferVk.cpp
+++ b/src/dawn_native/vulkan/CommandBufferVk.cpp
@@ -334,6 +334,9 @@
}
break;
}
+
+ case wgpu::TextureComponentType::DepthComparison:
+ UNREACHABLE();
}
attachmentCount++;
}
diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp
index 5c0bff9..8de770a 100644
--- a/src/dawn_native/vulkan/TextureVk.cpp
+++ b/src/dawn_native/vulkan/TextureVk.cpp
@@ -1058,6 +1058,8 @@
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,
diff --git a/src/tests/end2end/DeprecatedAPITests.cpp b/src/tests/end2end/DeprecatedAPITests.cpp
index cd92d5b..4b3222d 100644
--- a/src/tests/end2end/DeprecatedAPITests.cpp
+++ b/src/tests/end2end/DeprecatedAPITests.cpp
@@ -118,6 +118,50 @@
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(),
diff --git a/src/tests/end2end/DepthSamplingTests.cpp b/src/tests/end2end/DepthSamplingTests.cpp
index 29ec5e4..14d6d4f 100644
--- a/src/tests/end2end/DepthSamplingTests.cpp
+++ b/src/tests/end2end/DepthSamplingTests.cpp
@@ -151,9 +151,11 @@
// 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},
- {1, wgpu::ShaderStage::Fragment, wgpu::BindingType::SampledTexture},
- {2, wgpu::ShaderStage::Fragment, wgpu::BindingType::UniformBuffer}});
+ device,
+ {{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);
pipelineDescriptor.vertexStage.module = vsModule;
@@ -185,10 +187,12 @@
// TODO(dawn:367): Cannot use GetBindGroupLayout without shader reflection data.
wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
- device, {{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ComparisonSampler},
- {1, wgpu::ShaderStage::Compute, wgpu::BindingType::SampledTexture},
- {2, wgpu::ShaderStage::Compute, wgpu::BindingType::UniformBuffer},
- {3, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer}});
+ device,
+ {{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}});
wgpu::ComputePipelineDescriptor pipelineDescriptor;
pipelineDescriptor.layout = utils::MakeBasicPipelineLayout(device, &bgl);
diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp
index d1f9cec..28d54e4 100644
--- a/src/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -345,6 +345,41 @@
}
}
+// 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 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 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.