diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index 39d8121fbb..3e26f7780d 100644 --- a/src/dawn/native/Toggles.cpp +++ b/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 " diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index 0910cb64ab..5a493234ce 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h @@ -85,6 +85,7 @@ enum class Toggle { D3D12Allocate2DTextureWithCopyDstOrRenderAttachmentAsCommittedResource, MetalUseCombinedDepthStencilFormatForStencil8, MetalUseBothDepthAndStencilAttachmentsForCombinedDepthStencilFormats, + MetalKeepMultisubresourceDepthStencilTexturesInitialized, UseBlitForBufferToDepthTextureCopy, UseBlitForBufferToStencilTextureCopy, UseBlitForDepthTextureToTextureCopyToNonzeroSubresource, diff --git a/src/dawn/native/metal/BackendMTL.mm b/src/dawn/native/metal/BackendMTL.mm index fb5950a996..5339731e61 100644 --- a/src/dawn/native/metal/BackendMTL.mm +++ b/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; diff --git a/src/dawn/native/metal/CommandBufferMTL.mm b/src/dawn/native/metal/CommandBufferMTL.mm index 36f708e595..5f81387b2d 100644 --- a/src/dawn/native/metal/CommandBufferMTL.mm +++ b/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 descriptor = CreateMTLRenderPassDescriptor( device, cmd, device->UseCounterSamplingAtStageBoundary()); diff --git a/src/dawn/native/metal/TextureMTL.h b/src/dawn/native/metal/TextureMTL.h index 19c23b13c2..4141c2e961 100644 --- a/src/dawn/native/metal/TextureMTL.h +++ b/src/dawn/native/metal/TextureMTL.h @@ -55,6 +55,8 @@ class Texture final : public TextureBase { IOSurfaceRef GetIOSurface(); NSPRef> CreateFormatView(wgpu::TextureFormat format); + bool ShouldKeepInitialized() const; + MTLBlitOption ComputeMTLBlitOption(Aspect aspect) const; void EnsureSubresourceContentInitialized(CommandRecordingContext* commandContext, const SubresourceRange& range); diff --git a/src/dawn/native/metal/TextureMTL.mm b/src/dawn/native/metal/TextureMTL.mm index 21d14b5792..a8d457bbd5 100644 --- a/src/dawn/native/metal/TextureMTL.mm +++ b/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> 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(); } } diff --git a/src/dawn/tests/end2end/TextureZeroInitTests.cpp b/src/dawn/tests/end2end/TextureZeroInitTests.cpp index 15ea7e55f0..9f958e2245 100644 --- a/src/dawn/tests/end2end/TextureZeroInitTests.cpp +++ b/src/dawn/tests/end2end/TextureZeroInitTests.cpp @@ -1876,14 +1876,20 @@ TEST_P(TextureZeroInitTest, ErrorTextureIsUninitialized) { } } -DAWN_INSTANTIATE_TEST(TextureZeroInitTest, - D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}), - D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}, - {"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"}), - VulkanBackend({"nonzero_clear_resources_on_creation_for_testing"})); +DAWN_INSTANTIATE_TEST( + TextureZeroInitTest, + D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}), + D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}, {"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", + "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 { protected: @@ -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"}));