Metal: set both depth/stencil attachments for combined formats
Adds a toggle to workaround another issue where Metal fails to set
the depth/stencil attachment correctly for a combined depth stencil
format if just one of the attachments is used. The workaround forces
both attachments to be set, giving the unused one LoadOp::Load and
StoreOp::Store so its contents are preserved.
Bug: dawn:1389
Change-Id: Iacbefcc57b33bf11ca8fcacb03506301646fe59d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/117175
Reviewed-by: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp
index d227899..50cf5c4 100644
--- a/src/dawn/native/Toggles.cpp
+++ b/src/dawn/native/Toggles.cpp
@@ -328,8 +328,19 @@
"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 62b721b..db9c663 100644
--- a/src/dawn/native/Toggles.h
+++ b/src/dawn/native/Toggles.h
@@ -83,6 +83,7 @@
VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass,
D3D12Allocate2DTexturewithCopyDstAsCommittedResource,
MetalUseCombinedDepthStencilFormatForStencil8,
+ MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats,
UseTempTextureInStencilTextureToBufferCopy,
DisallowDeprecatedAPIs,
diff --git a/src/dawn/native/metal/CommandBufferMTL.mm b/src/dawn/native/metal/CommandBufferMTL.mm
index 17ee0fd..36f708e 100644
--- a/src/dawn/native/metal/CommandBufferMTL.mm
+++ b/src/dawn/native/metal/CommandBufferMTL.mm
@@ -169,6 +169,7 @@
}
NSRef<MTLRenderPassDescriptor> 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 @@
commandContext->EndBlit();
LazyClearRenderPassAttachments(cmd);
+ Device* device = ToBackend(GetDevice());
NSRef<MTLRenderPassDescriptor> 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<MTLRenderCommandEncoder> 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 f599245..77de03e 100644
--- a/src/dawn/native/metal/DeviceMTL.mm
+++ b/src/dawn/native/metal/DeviceMTL.mm
@@ -260,6 +260,8 @@
#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 e082198..f147474 100644
--- a/src/dawn/native/metal/RenderPipelineMTL.mm
+++ b/src/dawn/native/metal/RenderPipelineMTL.mm
@@ -373,14 +373,24 @@
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 a1722aa..bb218e9 100644
--- a/src/dawn/native/metal/UtilsMetal.h
+++ b/src/dawn/native/metal/UtilsMetal.h
@@ -32,6 +32,7 @@
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 ca4692f..a80672b 100644
--- a/src/dawn/native/metal/UtilsMetal.mm
+++ b/src/dawn/native/metal/UtilsMetal.mm
@@ -140,8 +140,29 @@
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 @@
// 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 528efa1..cafb44d 100644
--- a/src/dawn/tests/end2end/DepthStencilCopyTests.cpp
+++ b/src/dawn/tests/end2end/DepthStencilCopyTests.cpp
@@ -875,6 +875,47 @@
}
}
+// 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<uint8_t> 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 @@
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<wgpu::TextureFormat>(utils::kStencilFormats.begin(), utils::kStencilFormats.end()));
diff --git a/src/dawn/tests/end2end/DepthStencilLoadOpTests.cpp b/src/dawn/tests/end2end/DepthStencilLoadOpTests.cpp
index 45e7b68..29bb81d 100644
--- a/src/dawn/tests/end2end/DepthStencilLoadOpTests.cpp
+++ b/src/dawn/tests/end2end/DepthStencilLoadOpTests.cpp
@@ -115,6 +115,8 @@
switch (GetParam().mCheck) {
case Check::SampleDepth: {
+ DAWN_TEST_UNSUPPORTED_IF(utils::IsStencilOnlyFormat(GetParam().mFormat));
+
std::vector<float> expectedDepth(mipSize * mipSize, kDepthValues[mipLevel]);
ExpectSampledDepthData(
texture, mipSize, mipSize, 0, mipLevel,
@@ -124,6 +126,8 @@
}
case Check::CopyDepth: {
+ DAWN_TEST_UNSUPPORTED_IF(utils::IsStencilOnlyFormat(GetParam().mFormat));
+
if (GetParam().mFormat == wgpu::TextureFormat::Depth16Unorm) {
std::vector<uint16_t> expectedDepth(mipSize * mipSize,
kU16DepthValues[mipLevel]);
@@ -150,6 +154,8 @@
}
case Check::DepthTest: {
+ DAWN_TEST_UNSUPPORTED_IF(utils::IsStencilOnlyFormat(GetParam().mFormat));
+
std::vector<float> expectedDepth(mipSize * mipSize, kDepthValues[mipLevel]);
ExpectAttachmentDepthTestData(texture, GetParam().mFormat, mipSize, mipSize, 0,
mipLevel, expectedDepth)
@@ -232,8 +238,12 @@
auto params2 = MakeParamGenerator<DepthStencilLoadOpTestParams>(
{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<DepthStencilLoadOpTestParams> allParams;
@@ -285,9 +295,7 @@
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 9f3f451..fe72857 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,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/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
