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>
diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp
index 97c6180..e3f8082 100644
--- a/src/dawn_native/BindGroupLayout.cpp
+++ b/src/dawn_native/BindGroupLayout.cpp
@@ -130,18 +130,8 @@
viewDimension = texture.viewDimension;
}
- if (texture.multisampled) {
- if (viewDimension != wgpu::TextureViewDimension::e2D) {
- 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 (texture.multisampled && viewDimension != wgpu::TextureViewDimension::e2D) {
+ return DAWN_VALIDATION_ERROR("Multisampled texture bindings must be 2D.");
}
}
if (entry.storageTexture.access != wgpu::StorageTextureAccess::Undefined) {
diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index cb8704d..2fe2f5c 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -155,6 +155,7 @@
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 @@
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 @@
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 ==
- tint::inspector::ResourceBinding::
- ResourceType::kMultisampledTexture;
+ info->texture.multisampled =
+ resource.resource_type == tint::inspector::ResourceBinding::
+ ResourceType::kMultisampledTexture ||
+ resource.resource_type ==
+ tint::inspector::ResourceBinding::ResourceType::
+ kDepthMultisampledTexture;
break;
case BindingInfoType::StorageTexture:
diff --git a/src/dawn_native/vulkan/AdapterVk.cpp b/src/dawn_native/vulkan/AdapterVk.cpp
index 6250edb..194cc82 100644
--- a/src/dawn_native/vulkan/AdapterVk.cpp
+++ b/src/dawn_native/vulkan/AdapterVk.cpp
@@ -223,6 +223,16 @@
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
diff --git a/src/tests/end2end/MultisampledSamplingTests.cpp b/src/tests/end2end/MultisampledSamplingTests.cpp
index 03fe789..5ad3e8b 100644
--- a/src/tests/end2end/MultisampledSamplingTests.cpp
+++ b/src/tests/end2end/MultisampledSamplingTests.cpp
@@ -50,6 +50,9 @@
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 @@
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 @@
[[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 @@
// 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 @@
wgpu::ComputePassEncoder computePassEncoder = commandEncoder.BeginComputePass();
computePassEncoder.SetPipeline(checkSamplePipeline);
- // TODO(crbug.com/dawn/1021): Disallow using float/unfilterable-float with depth
- // textures.
- wgpu::BindGroup bindGroup;
- EXPECT_DEPRECATION_WARNING(
- bindGroup = utils::MakeBindGroup(
- device, checkSamplePipeline.GetBindGroupLayout(0),
- {{0, colorView},
- {1, depthView},
- {2, outputBuffer, alignedResultSize * sampleOffset, kResultSize}}));
- computePassEncoder.SetBindGroup(0, bindGroup);
+ computePassEncoder.SetBindGroup(
+ 0, utils::MakeBindGroup(
+ device, checkSamplePipeline.GetBindGroupLayout(0),
+ {{0, colorView},
+ {1, depthView},
+ {2, outputBuffer, alignedResultSize * sampleOffset, kResultSize}}));
computePassEncoder.Dispatch(1);
computePassEncoder.EndPass();
}
diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp
index dab809c..305e229 100644
--- a/src/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -1348,35 +1348,35 @@
}));
}
-// Test that multisampled textures cannot be DepthComparison
-TEST_F(BindGroupLayoutValidationTest, MultisampledTextureComponentType) {
- // Multisampled float component type works.
+// Test that multisampled texture bindings are valid
+TEST_F(BindGroupLayoutValidationTest, MultisampledTextureSampleType) {
+ // 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.
- ASSERT_DEVICE_ERROR(utils::MakeBindGroupLayout(
- device, {
- {0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Depth,
- wgpu::TextureViewDimension::e2D, true},
- }));
+ // Multisampled depth sample type works.
+ utils::MakeBindGroupLayout(device,
+ {
+ {0, wgpu::ShaderStage::Compute, wgpu::TextureSampleType::Depth,
+ wgpu::TextureViewDimension::e2D, true},
+ });
}
constexpr uint64_t kBufferSize = 3 * kMinUniformBufferOffsetAlignment + 8;
