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;