diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index d227899db5..50cf5c49fd 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp @@ -328,8 +328,19 @@ static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{ "https://crbug.com/1237175"}}, {Toggle::MetalUseCombinedDepthStencilFormatForStencil8, {"metal_use_combined_depth_stencil_format_for_stencil8", - "Use a combined depth stencil format instead of stencil8. The stencil8 format alone does not " - "work correctly.", + "Use a combined depth stencil format instead of stencil8. Works around an issue where the " + "stencil8 format alone does not work correctly. This toggle also causes depth stencil " + "attachments using a stencil8 format to also set the depth attachment in the Metal render " + "pass. This works around another issue where Metal fails to set the stencil attachment " + "correctly for a combined depth stencil format if the depth attachment is not also set.", + "https://crbug.com/dawn/1389"}}, + {Toggle::MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats, + {"metal_use_both_depth_and_stencil_attachments_for_combined_depth_stencil_formats", + "In Metal, depth and stencil attachments are set separately. Setting just one without the " + "other does not work correctly for combined depth stencil formats on some Metal drivers. " + "This workarounds ensures that both are set. This situation arises during lazy clears, or " + "for stencil8 formats if metal_use_combined_depth_stencil_format_for_stencil8 is also " + "enabled.", "https://crbug.com/dawn/1389"}}, {Toggle::UseTempTextureInStencilTextureToBufferCopy, {"use_temp_texture_in_stencil_texture_to_buffer_copy", diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index 62b721b8b6..db9c663c90 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h @@ -83,6 +83,7 @@ enum class Toggle { VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass, D3D12Allocate2DTexturewithCopyDstAsCommittedResource, MetalUseCombinedDepthStencilFormatForStencil8, + MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats, UseTempTextureInStencilTextureToBufferCopy, DisallowDeprecatedAPIs, diff --git a/src/dawn/native/metal/CommandBufferMTL.mm b/src/dawn/native/metal/CommandBufferMTL.mm index 17ee0fdec7..36f708e595 100644 --- a/src/dawn/native/metal/CommandBufferMTL.mm +++ b/src/dawn/native/metal/CommandBufferMTL.mm @@ -169,6 +169,7 @@ NSRef CreateMTLComputePassDescriptor(BeginComputePassC } NSRef CreateMTLRenderPassDescriptor( + const Device* device, BeginRenderPassCmd* renderPass, bool useCounterSamplingAtStageBoundary) { // Note that this creates a descriptor that's autoreleased so we don't use AcquireNSRef @@ -801,11 +802,11 @@ MaybeError CommandBuffer::FillCommands(CommandRecordingContext* commandContext) commandContext->EndBlit(); LazyClearRenderPassAttachments(cmd); + Device* device = ToBackend(GetDevice()); NSRef descriptor = CreateMTLRenderPassDescriptor( - cmd, ToBackend(GetDevice())->UseCounterSamplingAtStageBoundary()); + device, cmd, device->UseCounterSamplingAtStageBoundary()); DAWN_TRY(EncodeMetalRenderPass( - ToBackend(GetDevice()), commandContext, descriptor.Get(), cmd->width, - cmd->height, + device, commandContext, descriptor.Get(), cmd->width, cmd->height, [this](id encoder, BeginRenderPassCmd* cmd) -> MaybeError { return this->EncodeRenderPass(encoder, cmd); }, cmd)); diff --git a/src/dawn/native/metal/DeviceMTL.mm b/src/dawn/native/metal/DeviceMTL.mm index f599245ba5..77de03e43b 100644 --- a/src/dawn/native/metal/DeviceMTL.mm +++ b/src/dawn/native/metal/DeviceMTL.mm @@ -260,6 +260,8 @@ void Device::InitTogglesFromDriver() { #if DAWN_PLATFORM_IS(MACOS) if (gpu_info::IsIntel(vendorId)) { SetToggle(Toggle::UseTempTextureInStencilTextureToBufferCopy, true); + SetToggle(Toggle::MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats, + true); if ([NSProcessInfo.processInfo isOperatingSystemAtLeastVersion:NSOperatingSystemVersion{12, 0, 0}]) { diff --git a/src/dawn/native/metal/RenderPipelineMTL.mm b/src/dawn/native/metal/RenderPipelineMTL.mm index e082198134..f147474f6e 100644 --- a/src/dawn/native/metal/RenderPipelineMTL.mm +++ b/src/dawn/native/metal/RenderPipelineMTL.mm @@ -373,14 +373,24 @@ MaybeError RenderPipeline::Initialize() { if (HasDepthStencilAttachment()) { wgpu::TextureFormat depthStencilFormat = GetDepthStencilFormat(); - const Format& internalFormat = GetDevice()->GetValidInternalFormat(depthStencilFormat); MTLPixelFormat metalFormat = MetalPixelFormat(GetDevice(), depthStencilFormat); - if (internalFormat.HasDepth()) { - descriptorMTL.depthAttachmentPixelFormat = metalFormat; - } - if (internalFormat.HasStencil()) { - descriptorMTL.stencilAttachmentPixelFormat = metalFormat; + if (GetDevice()->IsToggleEnabled( + Toggle::MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats)) { + if (GetDepthStencilAspects(metalFormat) & Aspect::Depth) { + descriptorMTL.depthAttachmentPixelFormat = metalFormat; + } + if (GetDepthStencilAspects(metalFormat) & Aspect::Stencil) { + descriptorMTL.stencilAttachmentPixelFormat = metalFormat; + } + } else { + const Format& internalFormat = GetDevice()->GetValidInternalFormat(depthStencilFormat); + if (internalFormat.HasDepth()) { + descriptorMTL.depthAttachmentPixelFormat = metalFormat; + } + if (internalFormat.HasStencil()) { + descriptorMTL.stencilAttachmentPixelFormat = metalFormat; + } } } diff --git a/src/dawn/native/metal/UtilsMetal.h b/src/dawn/native/metal/UtilsMetal.h index a1722aab1f..bb218e91e9 100644 --- a/src/dawn/native/metal/UtilsMetal.h +++ b/src/dawn/native/metal/UtilsMetal.h @@ -32,6 +32,7 @@ enum class SingleShaderStage; namespace dawn::native::metal { +Aspect GetDepthStencilAspects(MTLPixelFormat format); MTLCompareFunction ToMetalCompareFunction(wgpu::CompareFunction compareFunction); struct TextureBufferCopySplit { diff --git a/src/dawn/native/metal/UtilsMetal.mm b/src/dawn/native/metal/UtilsMetal.mm index ca4692f706..a80672b9ec 100644 --- a/src/dawn/native/metal/UtilsMetal.mm +++ b/src/dawn/native/metal/UtilsMetal.mm @@ -140,8 +140,29 @@ void ResolveInAnotherRenderPass( commandContext->BeginRender(mtlRenderPassForResolve); commandContext->EndRender(); } + } // anonymous namespace +Aspect GetDepthStencilAspects(MTLPixelFormat format) { + switch (format) { + case MTLPixelFormatDepth16Unorm: + case MTLPixelFormatDepth32Float: + return Aspect::Depth; + +#if DAWN_PLATFORM_IS(MACOS) + case MTLPixelFormatDepth24Unorm_Stencil8: +#endif + case MTLPixelFormatDepth32Float_Stencil8: + return Aspect::Depth | Aspect::Stencil; + + case MTLPixelFormatStencil8: + return Aspect::Stencil; + + default: + UNREACHABLE(); + } +} + MTLCompareFunction ToMetalCompareFunction(wgpu::CompareFunction compareFunction) { switch (compareFunction) { case wgpu::CompareFunction::Never: @@ -347,6 +368,34 @@ MaybeError EncodeMetalRenderPass(Device* device, // workarounds to happen at the same time, it handles workarounds one by one and calls // itself recursively to handle the next workaround if needed. + // Handle the workaround where both depth and stencil attachments must be set for a + // combined depth-stencil format, not just one. + if (device->IsToggleEnabled( + Toggle::MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats)) { + const bool hasDepthAttachment = mtlRenderPass.depthAttachment.texture != nil; + const bool hasStencilAttachment = mtlRenderPass.stencilAttachment.texture != nil; + + if (hasDepthAttachment && !hasStencilAttachment) { + if (GetDepthStencilAspects([mtlRenderPass.depthAttachment.texture pixelFormat]) & + Aspect::Stencil) { + mtlRenderPass.stencilAttachment.texture = mtlRenderPass.depthAttachment.texture; + mtlRenderPass.stencilAttachment.level = mtlRenderPass.depthAttachment.level; + mtlRenderPass.stencilAttachment.slice = mtlRenderPass.depthAttachment.slice; + mtlRenderPass.stencilAttachment.loadAction = MTLLoadActionLoad; + mtlRenderPass.stencilAttachment.storeAction = MTLStoreActionStore; + } + } else if (hasStencilAttachment && !hasDepthAttachment) { + if (GetDepthStencilAspects([mtlRenderPass.stencilAttachment.texture pixelFormat]) & + Aspect::Depth) { + mtlRenderPass.depthAttachment.texture = mtlRenderPass.stencilAttachment.texture; + mtlRenderPass.depthAttachment.level = mtlRenderPass.stencilAttachment.level; + mtlRenderPass.depthAttachment.slice = mtlRenderPass.stencilAttachment.slice; + mtlRenderPass.depthAttachment.loadAction = MTLLoadActionLoad; + mtlRenderPass.depthAttachment.storeAction = MTLStoreActionStore; + } + } + } + // Handles the workaround for r8unorm rg8unorm mipmap rendering being broken on some // devices. Render to a temporary texture instead and then copy back to the attachment. if (device->IsToggleEnabled(Toggle::MetalRenderR8RG8UnormSmallMipToTempTexture)) { diff --git a/src/dawn/tests/end2end/DepthStencilCopyTests.cpp b/src/dawn/tests/end2end/DepthStencilCopyTests.cpp index 528efa153a..cafb44d0c0 100644 --- a/src/dawn/tests/end2end/DepthStencilCopyTests.cpp +++ b/src/dawn/tests/end2end/DepthStencilCopyTests.cpp @@ -875,6 +875,47 @@ TEST_P(StencilCopyTests, ToStencilAspectAtNonZeroOffset) { } } +// Test uploading to the non-zero mip, stencil-only aspect of a texture, +// and then checking the contents with a stencil test. +TEST_P(StencilCopyTests, CopyNonzeroMipThenReadWithStencilTest) { + // Copies to a single aspect are unsupported on OpenGL. + DAWN_TEST_UNSUPPORTED_IF(IsOpenGL()); + DAWN_TEST_UNSUPPORTED_IF(IsOpenGLES()); + + // Create a stencil texture + constexpr uint32_t kWidth = 4; + constexpr uint32_t kHeight = 4; + constexpr uint32_t kMipLevel = 1; + + wgpu::Texture depthStencilTexture = + CreateDepthStencilTexture(kWidth, kHeight, + wgpu::TextureUsage::RenderAttachment | + wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst, + kMipLevel + 1); + + std::vector stencilData = { + 7u, 7u, // + 7u, 7u, // + }; + + // Upload the stencil data. + { + wgpu::TextureDataLayout dataLayout = {}; + dataLayout.bytesPerRow = kWidth >> kMipLevel; + + wgpu::ImageCopyTexture imageCopyTexture = utils::CreateImageCopyTexture( + depthStencilTexture, 1, {0, 0, 0}, wgpu::TextureAspect::StencilOnly); + wgpu::Extent3D copySize = {kWidth >> kMipLevel, kHeight >> kMipLevel, 1}; + + queue.WriteTexture(&imageCopyTexture, stencilData.data(), stencilData.size(), &dataLayout, + ©Size); + } + + // Check the stencil contents. + ExpectAttachmentStencilTestData(depthStencilTexture, GetParam().mTextureFormat, + kWidth >> kMipLevel, kWidth >> kMipLevel, 0u, kMipLevel, 7u); +} + DAWN_INSTANTIATE_TEST_P(DepthStencilCopyTests, {D3D12Backend(), MetalBackend(), MetalBackend({"use_temp_texture_in_stencil_texture_to_buffer_copy"}), @@ -907,8 +948,10 @@ DAWN_INSTANTIATE_TEST_P( D3D12Backend({"d3d12_use_temp_buffer_in_depth_stencil_texture_and_buffer_" "copy_with_non_zero_buffer_offset"}), MetalBackend(), MetalBackend({"metal_use_combined_depth_stencil_format_for_stencil8"}), - MetalBackend({"use_temp_texture_in_stencil_texture_to_buffer_copy"}), OpenGLBackend(), - OpenGLESBackend(), + MetalBackend({"use_temp_texture_in_stencil_texture_to_buffer_copy"}), + MetalBackend( + {"metal_use_both_depth_and_stencil_attachments_for_combined_depth_stencil_formats"}), + OpenGLBackend(), OpenGLESBackend(), // Test with the vulkan_use_s8 toggle forced on and off. VulkanBackend({"vulkan_use_s8"}, {}), VulkanBackend({}, {"vulkan_use_s8"})}, std::vector(utils::kStencilFormats.begin(), utils::kStencilFormats.end())); diff --git a/src/dawn/tests/end2end/DepthStencilLoadOpTests.cpp b/src/dawn/tests/end2end/DepthStencilLoadOpTests.cpp index 45e7b68073..29bb81d9cd 100644 --- a/src/dawn/tests/end2end/DepthStencilLoadOpTests.cpp +++ b/src/dawn/tests/end2end/DepthStencilLoadOpTests.cpp @@ -115,6 +115,8 @@ class DepthStencilLoadOpTests : public DawnTestWithParams expectedDepth(mipSize * mipSize, kDepthValues[mipLevel]); ExpectSampledDepthData( texture, mipSize, mipSize, 0, mipLevel, @@ -124,6 +126,8 @@ class DepthStencilLoadOpTests : public DawnTestWithParams expectedDepth(mipSize * mipSize, kU16DepthValues[mipLevel]); @@ -150,6 +154,8 @@ class DepthStencilLoadOpTests : public DawnTestWithParams expectedDepth(mipSize * mipSize, kDepthValues[mipLevel]); ExpectAttachmentDepthTestData(texture, GetParam().mFormat, mipSize, mipSize, 0, mipLevel, expectedDepth) @@ -232,8 +238,12 @@ auto GenerateParam() { auto params2 = MakeParamGenerator( {D3D12Backend(), D3D12Backend({}, {"use_d3d12_render_pass"}), MetalBackend(), + MetalBackend({"metal_use_combined_depth_stencil_format_for_stencil8"}), + MetalBackend( + {"metal_use_both_depth_and_stencil_attachments_for_combined_depth_stencil_formats"}), OpenGLBackend(), OpenGLESBackend(), VulkanBackend()}, - {wgpu::TextureFormat::Depth24PlusStencil8, wgpu::TextureFormat::Depth32FloatStencil8}, + {wgpu::TextureFormat::Depth24PlusStencil8, wgpu::TextureFormat::Depth32FloatStencil8, + wgpu::TextureFormat::Stencil8}, {Check::CopyStencil, Check::StencilTest, Check::DepthTest, Check::SampleDepth}); std::vector allParams; @@ -285,9 +295,7 @@ TEST_P(StencilClearValueOverflowTest, StencilClearValueOverFlowUint16) { DAWN_INSTANTIATE_TEST_P(StencilClearValueOverflowTest, {D3D12Backend(), D3D12Backend({}, {"use_d3d12_render_pass"}), - MetalBackend(), - MetalBackend({"metal_use_combined_depth_stencil_format_for_stencil8"}), - OpenGLBackend(), OpenGLESBackend(), VulkanBackend()}, + MetalBackend(), OpenGLBackend(), OpenGLESBackend(), VulkanBackend()}, {wgpu::TextureFormat::Depth24PlusStencil8, wgpu::TextureFormat::Depth32FloatStencil8, wgpu::TextureFormat::Stencil8}, {Check::CopyStencil, Check::StencilTest}); diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt index 9f3f4516eb..fe72857461 100644 --- a/webgpu-cts/expectations.txt +++ b/webgpu-cts/expectations.txt @@ -335,10 +335,6 @@ crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,copyTexture crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,copyTextureToTexture:copy_depth_stencil:format="depth24plus-stencil8" [ Failure ] crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,copyTextureToTexture:copy_depth_stencil:format="depth32float" [ Failure ] crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,copyTextureToTexture:copy_depth_stencil:format="depth32float-stencil8" [ Failure ] -crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,image_copy:offsets_and_sizes_copy_depth_stencil:format="stencil8";copyMethod="CopyB2T";aspect="stencil-only" [ Failure ] -crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,image_copy:offsets_and_sizes_copy_depth_stencil:format="stencil8";copyMethod="WriteTexture";aspect="stencil-only" [ Failure ] -crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,image_copy:rowsPerImage_and_bytesPerRow_depth_stencil:format="stencil8";copyMethod="CopyB2T";aspect="stencil-only" [ Failure ] -crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,image_copy:rowsPerImage_and_bytesPerRow_depth_stencil:format="stencil8";copyMethod="WriteTexture";aspect="stencil-only" [ Failure ] ############################################################################ # Flaky on Intel Mac @@ -353,7 +349,6 @@ crbug.com/dawn/1500 [ intel-gen-9 monterey ] webgpu:api,operation,command_buffer crbug.com/dawn/1389 [ monterey ] webgpu:api,operation,resource_init,texture_zero:uninitialized_texture_is_zero:dimension="2d";readMethod="CopyToBuffer";format="depth16unorm" [ Failure ] crbug.com/dawn/1389 [ monterey ] webgpu:api,operation,resource_init,texture_zero:uninitialized_texture_is_zero:dimension="2d";readMethod="CopyToTexture";format="depth16unorm" [ Failure ] crbug.com/dawn/1389 [ monterey ] webgpu:api,operation,resource_init,texture_zero:uninitialized_texture_is_zero:dimension="2d";readMethod="DepthTest";format="depth16unorm" [ Failure ] -crbug.com/dawn/1389 [ monterey ] webgpu:api,operation,resource_init,texture_zero:uninitialized_texture_is_zero:dimension="2d";readMethod="StencilTest";format="stencil8" [ Failure ] ################################################################################ # copyToTexture,canvas:color_space_conversion:* fail with swiftshader