D3D12: Add RENDER_ATTACHMENT usage on depth stencil textures on Intel GPUs This patch adds RENDER_ATTACHMENT to the internal usage of D3D12 depth stencil textures on Intel GPU because due to a driver bug we have to initialize all the depth stencil textures with clear on Intel GPUs before copying with them. Otherwise, if a depth stencil texture is created without RENDER_ATTACHMENT usage, it will be initialized by copies, which will still trigger the driver bug mentioned above. Bug: 1487 Test: dawn_end2end_test Change-Id: I78b9a3e2bc4098d6f3f2619644c80fd54dafd4e8 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/96985 Reviewed-by: Austin Eng <enga@chromium.org> Kokoro: Kokoro <noreply+kokoro@google.com> Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp index 3580ca7..d170bff 100644 --- a/src/dawn/native/Texture.cpp +++ b/src/dawn/native/Texture.cpp
@@ -650,6 +650,10 @@ ASSERT(!IsError()); return mInternalUsage; } +void TextureBase::AddInternalUsage(wgpu::TextureUsage usage) { + ASSERT(!IsError()); + mInternalUsage |= usage; +} TextureBase::TextureState TextureBase::GetTextureState() const { ASSERT(!IsError());
diff --git a/src/dawn/native/Texture.h b/src/dawn/native/Texture.h index 5958094..84faa69 100644 --- a/src/dawn/native/Texture.h +++ b/src/dawn/native/Texture.h
@@ -112,6 +112,7 @@ ~TextureBase() override; void DestroyImpl() override; + void AddInternalUsage(wgpu::TextureUsage usage); private: TextureBase(DeviceBase* device, const TextureDescriptor* descriptor, ObjectBase::ErrorTag tag);
diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index 17d676d..a04bfce 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp
@@ -268,11 +268,11 @@ "Enables usage of the blob cache (backed by the platform cache if set/passed). Necessary for " "any persistent caching capabilities, i.e. pipeline caching.", "https://crbug.com/dawn/549"}}, - {Toggle::D3D12ForceInitializeCopyableDepthStencilTextureOnCreation, - {"d3d12_force_initialize_copyable_depth_stencil_texture_on_creation", - "Always initializing copyable depth stencil textures when creating them instead of skipping " - "the initialization when the entire subresource is the copy destination as a workaround on " - "Intel D3D12 drivers.", + {Toggle::D3D12ForceClearCopyableDepthStencilTextureOnCreation, + {"d3d12_force_clear_copyable_depth_stencil_texture_on_creation", + "Always clearing copyable depth stencil textures when creating them instead of skipping the " + "initialization when the entire subresource is the copy destination as a workaround on Intel " + "D3D12 drivers.", "https://crbug.com/dawn/1487"}}, {Toggle::D3D12DontSetClearValueOnDepthTextureCreation, {"d3d12_dont_set_clear_value_on_depth_texture_creation",
diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index 22c1198..77f7d07 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h
@@ -72,7 +72,7 @@ D3D12SplitBufferTextureCopyForRowsPerImagePaddings, MetalRenderR8RG8UnormSmallMipToTempTexture, EnableBlobCache, - D3D12ForceInitializeCopyableDepthStencilTextureOnCreation, + D3D12ForceClearCopyableDepthStencilTextureOnCreation, D3D12DontSetClearValueOnDepthTextureCreation, D3D12AlwaysUseTypelessFormatsForCastableTexture,
diff --git a/src/dawn/native/d3d12/DeviceD3D12.cpp b/src/dawn/native/d3d12/DeviceD3D12.cpp index 1a43169..ef6baec 100644 --- a/src/dawn/native/d3d12/DeviceD3D12.cpp +++ b/src/dawn/native/d3d12/DeviceD3D12.cpp
@@ -660,7 +660,7 @@ // Currently this workaround is only needed on Intel GPUs. // See http://crbug.com/dawn/1487 for more information. if (gpu_info::IsIntel(vendorId)) { - SetToggle(Toggle::D3D12ForceInitializeCopyableDepthStencilTextureOnCreation, true); + SetToggle(Toggle::D3D12ForceClearCopyableDepthStencilTextureOnCreation, true); } // Currently this workaround is only needed on Intel Gen12 GPUs.
diff --git a/src/dawn/native/d3d12/TextureD3D12.cpp b/src/dawn/native/d3d12/TextureD3D12.cpp index 041a498..5f64920 100644 --- a/src/dawn/native/d3d12/TextureD3D12.cpp +++ b/src/dawn/native/d3d12/TextureD3D12.cpp
@@ -581,6 +581,14 @@ resourceDescriptor.Height = size.height; resourceDescriptor.DepthOrArraySize = size.depthOrArrayLayers; + Device* device = ToBackend(GetDevice()); + bool applyForceClearCopyableDepthStencilTextureOnCreationToggle = + device->IsToggleEnabled(Toggle::D3D12ForceClearCopyableDepthStencilTextureOnCreation) && + GetFormat().HasDepthOrStencil() && (GetInternalUsage() & wgpu::TextureUsage::CopyDst); + if (applyForceClearCopyableDepthStencilTextureOnCreationToggle) { + AddInternalUsage(wgpu::TextureUsage::RenderAttachment); + } + // This will need to be much more nuanced when WebGPU has // texture view compatibility rules. const bool needsTypelessFormat = @@ -601,17 +609,12 @@ mD3D12ResourceFlags = resourceDescriptor.Flags; DAWN_TRY_ASSIGN(mResourceAllocation, - ToBackend(GetDevice()) - ->AllocateMemory(D3D12_HEAP_TYPE_DEFAULT, resourceDescriptor, - D3D12_RESOURCE_STATE_COMMON)); + device->AllocateMemory(D3D12_HEAP_TYPE_DEFAULT, resourceDescriptor, + D3D12_RESOURCE_STATE_COMMON)); SetLabelImpl(); - Device* device = ToBackend(GetDevice()); - - if (device->IsToggleEnabled( - Toggle::D3D12ForceInitializeCopyableDepthStencilTextureOnCreation) && - GetFormat().HasDepthOrStencil() && (GetInternalUsage() & wgpu::TextureUsage::CopyDst)) { + if (applyForceClearCopyableDepthStencilTextureOnCreationToggle) { CommandRecordingContext* commandContext; DAWN_TRY_ASSIGN(commandContext, device->GetPendingCommandContext()); DAWN_TRY(ClearTexture(commandContext, GetAllSubresources(), TextureBase::ClearValue::Zero));
diff --git a/src/dawn/tests/end2end/CopyTests.cpp b/src/dawn/tests/end2end/CopyTests.cpp index 272a041..8b89e37 100644 --- a/src/dawn/tests/end2end/CopyTests.cpp +++ b/src/dawn/tests/end2end/CopyTests.cpp
@@ -2582,9 +2582,12 @@ return o; } +using AddRenderAttachmentUsage = bool; + DAWN_TEST_PARAM_STRUCT(CopyToDepthStencilTextureAfterDestroyingBigBufferTestsParams, TextureFormat, - InitializationMethod); + InitializationMethod, + AddRenderAttachmentUsage); } // anonymous namespace @@ -2636,8 +2639,10 @@ wgpu::TextureDescriptor textureDescriptor = {}; textureDescriptor.format = format; textureDescriptor.size = {1, 1, 1}; - textureDescriptor.usage = wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst | - wgpu::TextureUsage::RenderAttachment; + textureDescriptor.usage = wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::CopyDst; + if (GetParam().mAddRenderAttachmentUsage) { + textureDescriptor.usage |= wgpu::TextureUsage::RenderAttachment; + } wgpu::Texture texture = device.CreateTexture(&textureDescriptor); // Finally, upload valid data into the texture and validate its contents. @@ -2723,9 +2728,9 @@ DAWN_INSTANTIATE_TEST_P( CopyToDepthStencilTextureAfterDestroyingBigBufferTests, - {D3D12Backend(), - D3D12Backend({"d3d12_force_initialize_copyable_depth_stencil_texture_on_creation"}), + {D3D12Backend(), D3D12Backend({"d3d12_force_clear_copyable_depth_stencil_texture_on_creation"}), MetalBackend(), OpenGLBackend(), OpenGLESBackend(), VulkanBackend()}, {wgpu::TextureFormat::Depth16Unorm, wgpu::TextureFormat::Stencil8}, {InitializationMethod::CopyBufferToTexture, InitializationMethod::WriteTexture, - InitializationMethod::CopyTextureToTexture}); + InitializationMethod::CopyTextureToTexture}, + {true, false});