Add a toggle to keep Metal depth stencil textures initialized

To avoid uninitialized reads of depth stencil data, where the Metal
driver incorrectly binds/loads the wrong depth stencil subresource,
always keep all depth stencil subresources initialized. This means
that textures are initialized on creation, and StoreOp::Discard is
never used - Store is used instead.

Texture initialized state is still set as-if the Discard occured, so
Dawn will try to zero-initialize the subresource if it is read from.
In many cases, this will work correctly, and the application will
read back 0, as expected. In some cases, Metal will bind the wrong
subresource, and the previous contents will be read. This is wrong,
but at least it is not uninitialized data.

Bug: dawn:838
Change-Id: I3cc87073d52de60283e3b683bbee7809db803018
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/119344
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Shrek Shao <shrekshao@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/dawn/native/Toggles.cpp

@@ -344,6 +344,13 @@ static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{
       "for stencil8 formats if metal_use_combined_depth_stencil_format_for_stencil8 is also "
       "enabled.",
       "https://crbug.com/dawn/1389", ToggleStage::Device}},
+    {Toggle::MetalKeepMultisubresourceDepthStencilTexturesInitialized,
+     {"metal_keep_multisubresource_depth_stencil_textures_initialized",
+      "Some platforms have bugs where the wrong depth stencil subresource is read/written. To "
+      "avoid reads of uninitialized data, ensure that depth stencil textures with more than one "
+      "subresource are completely initialized, and StoreOp::Discard is always translated as a "
+      "Store.",
+      "https://crbug.com/dawn/838", ToggleStage::Device}},
     {Toggle::UseBlitForBufferToDepthTextureCopy,
      {"use_blit_for_buffer_to_depth_texture_copy",
       "Use a blit instead of a copy command to copy buffer data to the depth aspect of a "

src/dawn/native/Toggles.h

@@ -85,6 +85,7 @@ enum class Toggle {
     D3D12Allocate2DTextureWithCopyDstOrRenderAttachmentAsCommittedResource,
     MetalUseCombinedDepthStencilFormatForStencil8,
     MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats,
+    MetalKeepMultisubresourceDepthStencilTexturesInitialized,
     UseBlitForBufferToDepthTextureCopy,
     UseBlitForBufferToStencilTextureCopy,
     UseBlitForDepthTextureToTextureCopyToNonzeroSubresource,

src/dawn/native/metal/BackendMTL.mm

@@ -400,6 +400,8 @@ class Adapter : public AdapterBase {
         }
         if (gpu_info::IsAMD(vendorId) || gpu_info::IsIntel(vendorId)) {
             deviceToggles->Default(Toggle::MetalUseCombinedDepthStencilFormatForStencil8, true);
+            deviceToggles->Default(Toggle::MetalKeepMultisubresourceDepthStencilTexturesInitialized,
+                                   true);
         }
 
         // Local testing shows the workaround is needed on AMD Radeon HD 8870M (gcn-1) MacOS 12.1;

src/dawn/native/metal/CommandBufferMTL.mm

@@ -802,6 +802,15 @@ MaybeError CommandBuffer::FillCommands(CommandRecordingContext* commandContext)
                 commandContext->EndBlit();
 
                 LazyClearRenderPassAttachments(cmd);
+                if (cmd->attachmentState->HasDepthStencilAttachment() &&
+                    ToBackend(cmd->depthStencilAttachment.view->GetTexture())
+                        ->ShouldKeepInitialized()) {
+                    // Ensure that depth and stencil never become uninitialized again if
+                    // the attachment must stay initialized.
+                    // For Toggle MetalKeepMultisubresourceDepthStencilTexturesInitialized.
+                    cmd->depthStencilAttachment.depthStoreOp = wgpu::StoreOp::Store;
+                    cmd->depthStencilAttachment.stencilStoreOp = wgpu::StoreOp::Store;
+                }
                 Device* device = ToBackend(GetDevice());
                 NSRef<MTLRenderPassDescriptor> descriptor = CreateMTLRenderPassDescriptor(
                     device, cmd, device->UseCounterSamplingAtStageBoundary());

src/dawn/native/metal/TextureMTL.h

@@ -55,6 +55,8 @@ class Texture final : public TextureBase {
     IOSurfaceRef GetIOSurface();
     NSPRef<id<MTLTexture>> CreateFormatView(wgpu::TextureFormat format);
 
+    bool ShouldKeepInitialized() const;
+
     MTLBlitOption ComputeMTLBlitOption(Aspect aspect) const;
     void EnsureSubresourceContentInitialized(CommandRecordingContext* commandContext,
                                              const SubresourceRange& range);

src/dawn/native/metal/TextureMTL.mm

@@ -759,6 +759,9 @@ MaybeError Texture::InitializeAsInternalTexture(const TextureDescriptor* descrip
     if (device->IsToggleEnabled(Toggle::NonzeroClearResourcesOnCreationForTesting)) {
         DAWN_TRY(ClearTexture(device->GetPendingCommandContext(), GetAllSubresources(),
                               TextureBase::ClearValue::NonZero));
+    } else if (ShouldKeepInitialized()) {
+        DAWN_TRY(ClearTexture(device->GetPendingCommandContext(), GetAllSubresources(),
+                              TextureBase::ClearValue::Zero));
     }
 
     return {};
@@ -854,6 +857,13 @@ NSPRef<id<MTLTexture>> Texture::CreateFormatView(wgpu::TextureFormat format) {
         [mMtlTexture.Get() newTextureViewWithPixelFormat:MetalPixelFormat(GetDevice(), format)]);
 }
 
+bool Texture::ShouldKeepInitialized() const {
+    return GetDevice()->IsToggleEnabled(Toggle::LazyClearResourceOnFirstUse) &&
+           GetDevice()->IsToggleEnabled(
+               Toggle::MetalKeepMultisubresourceDepthStencilTexturesInitialized) &&
+           GetFormat().HasDepthOrStencil() && (GetArrayLayers() > 1 || GetNumMipLevels() > 1);
+}
+
 MaybeError Texture::ClearTexture(CommandRecordingContext* commandContext,
                                  const SubresourceRange& range,
                                  TextureBase::ClearValue clearValue) {
@@ -1043,11 +1053,6 @@ MaybeError Texture::ClearTexture(CommandRecordingContext* commandContext,
             }
         }
     }
-
-    if (clearValue == TextureBase::ClearValue::Zero) {
-        SetIsSubresourceContentInitialized(true, range);
-        device->IncrementLazyClearCountForTesting();
-    }
     return {};
 }
 
@@ -1081,6 +1086,8 @@ void Texture::EnsureSubresourceContentInitialized(CommandRecordingContext* comma
         // contain dirty bits from recycled memory
         GetDevice()->ConsumedError(
             ClearTexture(commandContext, range, TextureBase::ClearValue::Zero));
+        SetIsSubresourceContentInitialized(true, range);
+        GetDevice()->IncrementLazyClearCountForTesting();
     }
 }
 

src/dawn/tests/end2end/TextureZeroInitTests.cpp

@@ -1876,13 +1876,19 @@ TEST_P(TextureZeroInitTest, ErrorTextureIsUninitialized) {
     }
 }
 
-DAWN_INSTANTIATE_TEST(TextureZeroInitTest,
+DAWN_INSTANTIATE_TEST(
+    TextureZeroInitTest,
     D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}),
-                      D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"},
+    D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}, {"use_d3d12_render_pass"}),
-                                   {"use_d3d12_render_pass"}),
     OpenGLBackend({"nonzero_clear_resources_on_creation_for_testing"}),
     OpenGLESBackend({"nonzero_clear_resources_on_creation_for_testing"}),
-                      MetalBackend({"nonzero_clear_resources_on_creation_for_testing"}),
+    MetalBackend({"nonzero_clear_resources_on_creation_for_testing",
+                  "metal_keep_multisubresource_depth_stencil_textures_initialized"}),
+    MetalBackend({"nonzero_clear_resources_on_creation_for_testing"},
+                 {"metal_keep_multisubresource_depth_stencil_textures_initialized"}),
+    MetalBackend({"nonzero_clear_resources_on_creation_for_testing",
+                  "use_blit_for_buffer_to_depth_texture_copy",
+                  "use_blit_for_buffer_to_stencil_texture_copy"}),
     VulkanBackend({"nonzero_clear_resources_on_creation_for_testing"}));
 
 class CompressedTextureZeroInitTest : public TextureZeroInitTest {
@@ -2318,9 +2324,6 @@ 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_blit_for_buffer_to_depth_texture_copy",
-                                    "use_blit_for_buffer_to_stencil_texture_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"}));