From e2be18a7fd6779d9e395b820d1fed24ae2755dfe Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Wed, 18 Jan 2023 17:55:14 +0000 Subject: [PATCH] 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 Reviewed-by: Loko Kung Kokoro: Kokoro --- src/dawn/native/CommandEncoder.cpp | 53 ++++-- src/dawn/native/Commands.cpp | 1 + src/dawn/native/Commands.h | 1 + src/dawn/native/Toggles.cpp | 6 + src/dawn/native/Toggles.h | 1 + src/dawn/native/metal/DeviceMTL.mm | 2 + .../tests/end2end/DepthStencilCopyTests.cpp | 7 +- .../tests/end2end/TextureZeroInitTests.cpp | 158 ++++++++++++++++++ webgpu-cts/expectations.txt | 4 - 9 files changed, 217 insertions(+), 16 deletions(-) diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp index 0d9480e5f3..119aece7cb 100644 --- a/src/dawn/native/CommandEncoder.cpp +++ b/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 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(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(Command::CopyTextureToBuffer); - copy->source.texture = source->texture; - copy->source.origin = source->origin; - copy->source.mipLevel = source->mipLevel; - copy->source.aspect = ConvertAspect(source->texture->GetFormat(), source->aspect); - copy->destination.buffer = destination->buffer; - copy->destination.offset = dstLayout.offset; - copy->destination.bytesPerRow = dstLayout.bytesPerRow; - copy->destination.rowsPerImage = dstLayout.rowsPerImage; - copy->copySize = *copySize; + t2b->source = copySrc; + t2b->destination.buffer = destination->buffer; + t2b->destination.offset = dstLayout.offset; + t2b->destination.bytesPerRow = dstLayout.bytesPerRow; + t2b->destination.rowsPerImage = dstLayout.rowsPerImage; + t2b->copySize = *copySize; return {}; }, diff --git a/src/dawn/native/Commands.cpp b/src/dawn/native/Commands.cpp index c1be63922f..748882b735 100644 --- a/src/dawn/native/Commands.cpp +++ b/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; diff --git a/src/dawn/native/Commands.h b/src/dawn/native/Commands.h index 456a1b5961..06ea16ea8b 100644 --- a/src/dawn/native/Commands.h +++ b/src/dawn/native/Commands.h @@ -150,6 +150,7 @@ struct BufferCopy { struct TextureCopy { TextureCopy(); TextureCopy(const TextureCopy&); + TextureCopy& operator=(const TextureCopy&); ~TextureCopy(); Ref texture; diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index 8a2b11727e..d227899db5 100644 --- a/src/dawn/native/Toggles.cpp +++ b/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 " diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index 9aa6b86c2d..62b721b8b6 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h @@ -83,6 +83,7 @@ enum class Toggle { VulkanSplitCommandBufferOnDepthStencilComputeSampleAfterRenderPass, D3D12Allocate2DTexturewithCopyDstAsCommittedResource, MetalUseCombinedDepthStencilFormatForStencil8, + UseTempTextureInStencilTextureToBufferCopy, DisallowDeprecatedAPIs, // Unresolved issues. diff --git a/src/dawn/native/metal/DeviceMTL.mm b/src/dawn/native/metal/DeviceMTL.mm index 5e075e8805..10448d0233 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); + if ([NSProcessInfo.processInfo isOperatingSystemAtLeastVersion:NSOperatingSystemVersion{12, 0, 0}]) { ForceSetToggle(Toggle::NoWorkaroundSampleMaskBecomesZeroForAllButLastColorTarget, true); diff --git a/src/dawn/tests/end2end/DepthStencilCopyTests.cpp b/src/dawn/tests/end2end/DepthStencilCopyTests.cpp index 2defaae896..528efa153a 100644 --- a/src/dawn/tests/end2end/DepthStencilCopyTests.cpp +++ b/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(utils::kStencilFormats.begin(), utils::kStencilFormats.end())); diff --git a/src/dawn/tests/end2end/TextureZeroInitTests.cpp b/src/dawn/tests/end2end/TextureZeroInitTests.cpp index 9e904dd62e..f883ef2d31 100644 --- a/src/dawn/tests/end2end/TextureZeroInitTests.cpp +++ b/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 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 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 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"})); diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt index 7a0f9041c6..720e706d6d 100644 --- a/webgpu-cts/expectations.txt +++ b/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 ]