Workaround stencil to buffer copies on Metal Intel

Stencil to buffer copies don't capture contents
written in a rendering stage. Copying through an
intermediate texture fixes this problem on Metal Intel.

Add test for the code path that found the issue:
Nonzero-mip stencil copy, discard, then read

Bug: dawn:1389
Change-Id: I63d982df6bace4b5053d3c643b8abda1682490d1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/116851
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/dawn/native/CommandEncoder.cpp

@@ -1277,17 +1277,50 @@ void CommandEncoder::APICopyTextureToBuffer(const ImageCopyTexture* source,
             TextureDataLayout dstLayout = destination->layout;
             ApplyDefaultTextureDataLayoutOptions(&dstLayout, blockInfo, *copySize);
 
-            CopyTextureToBufferCmd* copy =
+            TextureCopy copySrc;
+            copySrc.texture = source->texture;
+            copySrc.origin = source->origin;
+            copySrc.mipLevel = source->mipLevel;
+            copySrc.aspect = ConvertAspect(source->texture->GetFormat(), source->aspect);
+
+            if (copySrc.aspect == Aspect::Stencil &&
+                GetDevice()->IsToggleEnabled(Toggle::UseTempTextureInStencilTextureToBufferCopy)) {
+                // Encode a copy to an intermediate texture.
+                TextureDescriptor desc = {};
+                desc.format = source->texture->GetFormat().format;
+                desc.usage = wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst;
+                desc.size = *copySize;
+
+                Ref<TextureBase> intermediateTexture;
+                DAWN_TRY_ASSIGN(intermediateTexture, GetDevice()->CreateTexture(&desc));
+
+                // Allocate the intermediate t2t command.
+                Aspect aspect =
+                    ConvertAspect(source->texture->GetFormat(), wgpu::TextureAspect::All);
+                CopyTextureToTextureCmd* t2t =
+                    allocator->Allocate<CopyTextureToTextureCmd>(Command::CopyTextureToTexture);
+                t2t->source = copySrc;
+                t2t->source.aspect = aspect;
+                t2t->destination.texture = intermediateTexture;
+                t2t->destination.origin = {};
+                t2t->destination.mipLevel = 0;
+                t2t->destination.aspect = aspect;
+                t2t->copySize = *copySize;
+
+                // Replace the `copySrc` with the intermediate texture.
+                copySrc.texture = intermediateTexture;
+                copySrc.mipLevel = 0;
+                copySrc.origin = {};
+            }
+
+            CopyTextureToBufferCmd* t2b =
                 allocator->Allocate<CopyTextureToBufferCmd>(Command::CopyTextureToBuffer);
-            copy->source.texture = source->texture;
+            t2b->source = copySrc;
-            copy->source.origin = source->origin;
+            t2b->destination.buffer = destination->buffer;
-            copy->source.mipLevel = source->mipLevel;
+            t2b->destination.offset = dstLayout.offset;
-            copy->source.aspect = ConvertAspect(source->texture->GetFormat(), source->aspect);
+            t2b->destination.bytesPerRow = dstLayout.bytesPerRow;
-            copy->destination.buffer = destination->buffer;
+            t2b->destination.rowsPerImage = dstLayout.rowsPerImage;
-            copy->destination.offset = dstLayout.offset;
+            t2b->copySize = *copySize;
-            copy->destination.bytesPerRow = dstLayout.bytesPerRow;
-            copy->destination.rowsPerImage = dstLayout.rowsPerImage;
-            copy->copySize = *copySize;
 
             return {};
         },

src/dawn/native/Commands.cpp

@@ -384,6 +384,7 @@ BufferCopy::~BufferCopy() = default;
 
 TextureCopy::TextureCopy() = default;
 TextureCopy::TextureCopy(const TextureCopy&) = default;
+TextureCopy& TextureCopy::operator=(const TextureCopy&) = default;
 TextureCopy::~TextureCopy() = default;
 
 CopyBufferToBufferCmd::CopyBufferToBufferCmd() = default;

src/dawn/native/Commands.h

@@ -150,6 +150,7 @@ struct BufferCopy {
 struct TextureCopy {
     TextureCopy();
     TextureCopy(const TextureCopy&);
+    TextureCopy& operator=(const TextureCopy&);
     ~TextureCopy();
 
     Ref<TextureBase> texture;

src/dawn/native/Toggles.cpp

@@ -331,6 +331,12 @@ static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{
       "Use a combined depth stencil format instead of stencil8. The stencil8 format alone does not "
       "work correctly.",
       "https://crbug.com/dawn/1389"}},
+    {Toggle::UseTempTextureInStencilTextureToBufferCopy,
+     {"use_temp_texture_in_stencil_texture_to_buffer_copy",
+      "Use an intermediate temporary texture when copying the stencil aspect of a texture to a "
+      "buffer. Works around an issue where stencil writes from a render pass are not reflected in "
+      "the destination buffer.",
+      "https://crbug.com/dawn/1389"}},
     {Toggle::DisallowDeprecatedAPIs,
      {"disallow_deprecated_apis",
       "Disallow all deprecated paths by changing the deprecation warnings to validation error for "

src/dawn/native/Toggles.h

@@ -83,6 +83,7 @@ enum class Toggle {
     VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass,
     D3D12Allocate2DTexturewithCopyDstAsCommittedResource,
     MetalUseCombinedDepthStencilFormatForStencil8,
+    UseTempTextureInStencilTextureToBufferCopy,
     DisallowDeprecatedAPIs,
 
     // Unresolved issues.

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);
+
         if ([NSProcessInfo.processInfo
                 isOperatingSystemAtLeastVersion:NSOperatingSystemVersion{12, 0, 0}]) {
             ForceSetToggle(Toggle::NoWorkaroundSampleMaskBecomesZeroForAllButLastColorTarget, true);

src/dawn/tests/end2end/DepthStencilCopyTests.cpp

@@ -876,7 +876,9 @@ TEST_P(StencilCopyTests, ToStencilAspectAtNonZeroOffset) {
 }
 
 DAWN_INSTANTIATE_TEST_P(DepthStencilCopyTests,
-                        {D3D12Backend(), MetalBackend(), OpenGLBackend(), OpenGLESBackend(),
+                        {D3D12Backend(), MetalBackend(),
+                         MetalBackend({"use_temp_texture_in_stencil_texture_to_buffer_copy"}),
+                         OpenGLBackend(), OpenGLESBackend(),
                          // Test with the vulkan_use_s8 toggle forced on and off.
                          VulkanBackend({"vulkan_use_s8"}, {}),
                          VulkanBackend({}, {"vulkan_use_s8"})},
@@ -905,7 +907,8 @@ 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"}),
-     OpenGLBackend(), OpenGLESBackend(),
+     MetalBackend({"use_temp_texture_in_stencil_texture_to_buffer_copy"}), 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()));

src/dawn/tests/end2end/TextureZeroInitTests.cpp

@@ -115,6 +115,36 @@ class TextureZeroInitTest : public DawnTest {
         )");
     }
 
+    wgpu::Texture CreateAndFillStencilTexture(wgpu::TextureFormat format) {
+        // Create the texture.
+        wgpu::TextureDescriptor depthStencilDescriptor =
+            CreateTextureDescriptor(1, 1,
+                                    wgpu::TextureUsage::RenderAttachment |
+                                        wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst,
+                                    format);
+        wgpu::Texture depthStencilTexture = device.CreateTexture(&depthStencilDescriptor);
+
+        // Prepare stencil data
+        const uint64_t dataSize =
+            utils::RequiredBytesInCopy(kSize, 0, {kSize, kSize, 1}, wgpu::TextureFormat::Stencil8);
+        std::vector<uint8_t> stencilData(dataSize);
+        for (size_t i = 0; i < stencilData.size(); ++i) {
+            stencilData[i] = i % 255;
+        }
+
+        wgpu::ImageCopyTexture imageCopyTexture = utils::CreateImageCopyTexture(
+            depthStencilTexture, 0u, {0, 0, 0}, wgpu::TextureAspect::StencilOnly);
+
+        wgpu::TextureDataLayout textureDataLayout = {};
+        textureDataLayout.bytesPerRow = kSize;
+
+        // Write the stencil data
+        queue.WriteTexture(&imageCopyTexture, stencilData.data(), stencilData.size(),
+                           &textureDataLayout, &depthStencilDescriptor.size);
+
+        return depthStencilTexture;
+    }
+
     constexpr static uint32_t kSize = 128;
     constexpr static uint32_t kUnalignedSize = 127;
     // All texture formats used (RGBA8Unorm, Depth24PlusStencil8, and RGBA8Snorm, BC formats)
@@ -757,6 +787,132 @@ TEST_P(TextureZeroInitTest, IndependentDepthStencilLoadAfterDiscard) {
     }
 }
 
+// Test that a stencil texture that is written via copy, then discarded, sees
+// zero contents when it is read by sampling.
+TEST_P(TextureZeroInitTest, StencilCopyThenDiscardAndReadBySampling) {
+    // Copies to a single aspect are unsupported on OpenGL.
+    DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
+
+    for (wgpu::TextureFormat format :
+         {wgpu::TextureFormat::Stencil8, wgpu::TextureFormat::Depth24PlusStencil8}) {
+        wgpu::Texture depthStencilTexture = CreateAndFillStencilTexture(format);
+
+        // Discard the stencil data.
+        {
+            utils::ComboRenderPassDescriptor renderPassDescriptor({},
+                                                                  depthStencilTexture.CreateView());
+            renderPassDescriptor.UnsetDepthStencilLoadStoreOpsForFormat(format);
+            renderPassDescriptor.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+            renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp =
+                wgpu::StoreOp::Discard;
+
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            auto pass = encoder.BeginRenderPass(&renderPassDescriptor);
+            pass.End();
+            wgpu::CommandBuffer commandBuffer = encoder.Finish();
+            queue.Submit(1, &commandBuffer);
+        }
+
+        // Data should now be zero.
+        ExpectAttachmentStencilTestData(depthStencilTexture, format, kSize, kSize, 0u, 0u, 0u);
+    }
+}
+
+// Test that a stencil texture that is written via copy, then discarded, sees
+// zero contents when it is read via copy.
+TEST_P(TextureZeroInitTest, StencilCopyThenDiscardAndReadByCopy) {
+    // Copies to a single aspect are unsupported on OpenGL.
+    DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
+
+    for (wgpu::TextureFormat format :
+         {wgpu::TextureFormat::Stencil8, wgpu::TextureFormat::Depth24PlusStencil8}) {
+        wgpu::Texture depthStencilTexture = CreateAndFillStencilTexture(format);
+
+        // Discard the stencil data.
+        {
+            utils::ComboRenderPassDescriptor renderPassDescriptor({},
+                                                                  depthStencilTexture.CreateView());
+            renderPassDescriptor.UnsetDepthStencilLoadStoreOpsForFormat(format);
+            renderPassDescriptor.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+            renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp =
+                wgpu::StoreOp::Discard;
+
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            auto pass = encoder.BeginRenderPass(&renderPassDescriptor);
+            pass.End();
+            wgpu::CommandBuffer commandBuffer = encoder.Finish();
+            queue.Submit(1, &commandBuffer);
+        }
+
+        // Data should now be zero.
+        std::vector<uint8_t> stencilData(kSize * kSize, 0);
+        EXPECT_TEXTURE_EQ(stencilData.data(), depthStencilTexture, {0, 0}, {kSize, kSize}, 0u,
+                          wgpu::TextureAspect::StencilOnly);
+    }
+}
+
+// Test that a stencil texture that is written via copy, then discarded, then copied to
+// another texture, sees zero contents when it is read via copy.
+TEST_P(TextureZeroInitTest, StencilCopyThenDiscardAndCopyToTextureThenReadByCopy) {
+    // Copies to a single aspect are unsupported on OpenGL.
+    DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
+
+    for (wgpu::TextureFormat format :
+         {wgpu::TextureFormat::Stencil8, wgpu::TextureFormat::Depth24PlusStencil8}) {
+        // Create the texture.
+        wgpu::TextureDescriptor depthStencilDescriptor =
+            CreateTextureDescriptor(1, 1,
+                                    wgpu::TextureUsage::RenderAttachment |
+                                        wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst,
+                                    format);
+        wgpu::Texture depthStencilTexture = device.CreateTexture(&depthStencilDescriptor);
+
+        // Prepare stencil data
+        const uint64_t dataSize =
+            utils::RequiredBytesInCopy(kSize, 0, {kSize, kSize, 1}, wgpu::TextureFormat::Stencil8);
+        std::vector<uint8_t> stencilData(dataSize);
+        for (size_t i = 0; i < stencilData.size(); ++i) {
+            stencilData[i] = i % 255;
+        }
+
+        wgpu::ImageCopyTexture imageCopyTexture = utils::CreateImageCopyTexture(
+            depthStencilTexture, 0, {0, 0, 0}, wgpu::TextureAspect::StencilOnly);
+
+        wgpu::TextureDataLayout textureDataLayout = {};
+        textureDataLayout.bytesPerRow = kSize;
+
+        // Write the stencil data
+        queue.WriteTexture(&imageCopyTexture, stencilData.data(), stencilData.size(),
+                           &textureDataLayout, &depthStencilDescriptor.size);
+
+        wgpu::Texture intermediate = device.CreateTexture(&depthStencilDescriptor);
+
+        // Discard the stencil data and copy to an intermediate texture.
+        {
+            utils::ComboRenderPassDescriptor renderPassDescriptor({},
+                                                                  depthStencilTexture.CreateView());
+            renderPassDescriptor.UnsetDepthStencilLoadStoreOpsForFormat(format);
+            renderPassDescriptor.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+            renderPassDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp =
+                wgpu::StoreOp::Discard;
+
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            auto pass = encoder.BeginRenderPass(&renderPassDescriptor);
+            pass.End();
+            wgpu::ImageCopyTexture src = utils::CreateImageCopyTexture(depthStencilTexture);
+            wgpu::ImageCopyTexture dst = utils::CreateImageCopyTexture(intermediate);
+            encoder.CopyTextureToTexture(&src, &dst, &depthStencilDescriptor.size);
+            wgpu::CommandBuffer commandBuffer = encoder.Finish();
+            queue.Submit(1, &commandBuffer);
+        }
+
+        // Data should now be zero.
+        std::fill(stencilData.begin(), stencilData.end(), 0);
+        EXPECT_TEXTURE_EQ(stencilData.data(), intermediate, {0, 0}, {kSize, kSize}, 0u,
+                          wgpu::TextureAspect::StencilOnly);
+    }
+}
+
 // Test that clear state is tracked independently for depth/stencil textures.
 // Lazy clear of the stencil aspect via copy should not touch depth.
 TEST_P(TextureZeroInitTest, IndependentDepthStencilCopyAfterDiscard) {
@@ -2162,6 +2318,8 @@ TEST_P(CompressedTextureZeroInitTest, Copy2DArrayCompressedB2T2B) {
 DAWN_INSTANTIATE_TEST(CompressedTextureZeroInitTest,
                       D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}),
                       MetalBackend({"nonzero_clear_resources_on_creation_for_testing"}),
+                      MetalBackend({"nonzero_clear_resources_on_creation_for_testing",
+                                    "use_temp_texture_in_stencil_texture_to_buffer_copy"}),
                       OpenGLBackend({"nonzero_clear_resources_on_creation_for_testing"}),
                       OpenGLESBackend({"nonzero_clear_resources_on_creation_for_testing"}),
                       VulkanBackend({"nonzero_clear_resources_on_creation_for_testing"}));

webgpu-cts/expectations.txt

@@ -334,7 +334,6 @@ crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,command_buffer,image_copy:
 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 ]
-crbug.com/dawn/1083 [ monterey ] webgpu:api,operation,render_pass,storeOp:render_pass_store_op,depth_stencil_attachment_only:depthStencilFormat="stencil8";storeOperation="discard" [ Failure ]
 
 ############################################################################
 # Flaky on Intel Mac
@@ -347,8 +346,6 @@ crbug.com/dawn/1500 [ intel-gen-9 monterey ] webgpu:api,operation,command_buffer
 # Failures on Mac Intel, likely also due to crbug.com/dawn/1083
 ################################################################################
 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="CopyToBuffer";format="depth32float-stencil8" [ Failure ]
-crbug.com/dawn/1389 [ monterey ] webgpu:api,operation,resource_init,texture_zero:uninitialized_texture_is_zero:dimension="2d";readMethod="CopyToBuffer";format="stencil8" [ 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 ]
@@ -399,7 +396,6 @@ crbug.com/dawn/0000 [ monterey ] webgpu:web_platform,canvas,readbackFromWebGPUCa
 crbug.com/dawn/0000 [ monterey ] webgpu:web_platform,canvas,readbackFromWebGPUCanvas:onscreenCanvas,uploadToWebGL:format="rgba8unorm";alphaMode="premultiplied";webgl="webgl";upload="texSubImage2D" [ Failure ]
 crbug.com/dawn/0000 [ monterey ] webgpu:web_platform,canvas,readbackFromWebGPUCanvas:onscreenCanvas,uploadToWebGL:format="rgba8unorm";alphaMode="premultiplied";webgl="webgl2";upload="texImage2D" [ Failure ]
 crbug.com/dawn/0000 [ monterey ] webgpu:web_platform,canvas,readbackFromWebGPUCanvas:onscreenCanvas,uploadToWebGL:format="rgba8unorm";alphaMode="premultiplied";webgl="webgl2";upload="texSubImage2D" [ Failure ]
-crbug.com/dawn/0000 [ monterey ] worker_webgpu:api,operation,render_pass,storeOp:render_pass_store_op,depth_stencil_attachment_only:depthStencilFormat="stencil8";storeOperation="discard" [ Failure ]
 crbug.com/dawn/0000 [ dawn-backend-validation nvidia-0x2184 target-cpu-64 win10 ] worker_webgpu:api,validation,buffer,mapping:mapAsync,invalidBuffer: [ Failure ]
 crbug.com/dawn/0000 [ dawn-no-backend-validation nvidia-0x2184 target-cpu-32 win10 ] worker_webgpu:api,validation,buffer,mapping:mapAsync,invalidBuffer: [ Failure ]
 crbug.com/dawn/0000 [ dawn-no-backend-validation nvidia-0x2184 target-cpu-64 win10 ] worker_webgpu:api,validation,buffer,mapping:mapAsync,invalidBuffer: [ Failure ]