D3D12: Hard-enable `D3D12SplitBufferTextureCopyForRowsPerImagePaddings` The toggle `D3D12SplitBufferTextureCopyForRowsPerImagePaddings` doesn't need to be defined as the code path it controlled before should always be executed. It is because: 1. In D3D12, the concept of `bufferLocation` specifies how to treat a region of a buffer as a texture. In buffer-texture copies `rowsPerImage` is used as the height of the `bufferLocation`, so if the buffer is not large enough to hold a complete `rowsPerImage` D3D12 debug layer will generate an error that the buffer is not large enough to hold a complete `bufferLocation`, which fully meets the concept in D3D12 and cannot say it a `bug` of D3D12 runtime. 2. Although `UnrestrictedBufferTextureCopyPitchSupported` has been used in Dawn we still need to split a buffer-texture copy into two in some certain situations because of (1). Bug: 42240278 Change-Id: I83981d92057b79661aa39ede66a611329b7cf8a4 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/218915 Commit-Queue: Jiawei Shao <jiawei.shao@intel.com> Reviewed-by: Kai Ninomiya <kainino@chromium.org> Reviewed-by: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index c5d1cba..8248c17 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp
@@ -265,14 +265,6 @@ "Initialize workgroup memory with OpConstantNull on Vulkan when the Vulkan extension " "VK_KHR_zero_initialize_workgroup_memory is supported.", "https://crbug.com/dawn/1302", ToggleStage::Device}}, - {Toggle::D3D12SplitBufferTextureCopyForRowsPerImagePaddings, - {"d3d12_split_buffer_texture_copy_for_rows_per_image_paddings", - "D3D12 requires more buffer storage than it should when rowsPerImage is greater than " - "copyHeight, which means there are pure padding row(s) on each image. In this situation, " - "the buffer used for B2T/T2B copy might be big enough according to WebGPU's spec but it " - "doesn't meet D3D12's requirement, then we need to workaround it via split the copy " - "operation into two copies, in order to make B2T/T2B copy being done correctly on D3D12.", - "https://crbug.com/dawn/1289", ToggleStage::Device}}, {Toggle::MetalRenderR8RG8UnormSmallMipToTempTexture, {"metal_render_r8_rg8_unorm_small_mip_to_temp_texture", "Metal Intel devices have issues with r8unorm and rg8unorm textures where rendering to small "
diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index f096d35..0993ef9 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h
@@ -83,7 +83,6 @@ TimestampQuantization, ClearBufferBeforeResolveQueries, VulkanUseZeroInitializeWorkgroupMemoryExtension, - D3D12SplitBufferTextureCopyForRowsPerImagePaddings, MetalRenderR8RG8UnormSmallMipToTempTexture, DisableBlobCache, D3D12ForceClearCopyableDepthStencilTextureOnCreation,
diff --git a/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp b/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp index 10b6e0a..58789ae 100644 --- a/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp +++ b/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp
@@ -702,13 +702,6 @@ deviceToggles->Default(Toggle::D3D12DontSetClearValueOnDepthTextureCreation, true); } - // Currently this workaround is needed on any D3D12 backend for some particular situations. - // But we may need to limit it if D3D12 runtime fixes the bug on its new release. See - // https://crbug.com/dawn/1289 for more information. - // TODO(dawn:1289): Unset this toggle when we skip the split on the buffer-texture copy - // on the platforms where UnrestrictedBufferTextureCopyPitchSupported is true. - deviceToggles->Default(Toggle::D3D12SplitBufferTextureCopyForRowsPerImagePaddings, true); - // This workaround is only needed on Intel Gen12LP with driver prior to 30.0.101.1692. // See http://crbug.com/dawn/949 for more information. if (gpu_info::IsIntelGen12LP(vendorId, deviceId)) {
diff --git a/src/dawn/native/d3d12/UtilsD3D12.cpp b/src/dawn/native/d3d12/UtilsD3D12.cpp index f79591a..946aa26 100644 --- a/src/dawn/native/d3d12/UtilsD3D12.cpp +++ b/src/dawn/native/d3d12/UtilsD3D12.cpp
@@ -68,9 +68,8 @@ return requiredCopySizeByD3D12; } -// This function is used to access whether we need a workaround for D3D12's wrong algorithm -// of calculating required buffer size for B2T/T2B copy. The workaround is needed only when -// - The corresponding toggle is enabled. +// This function is used to access whether we need a workaround for D3D12's algorithm of +// calculating required buffer size for B2T/T2B copy. The workaround is needed only when // - It is a 3D texture (so the format is uncompressed). // - There are multiple depth images to be copied (copySize.depthOrArrayLayers > 1). // - It has rowsPerImage paddings (rowsPerImage > copySize.height). @@ -79,10 +78,7 @@ const TextureCopy& textureCopy, const Extent3D& copySize) { TextureBase* texture = textureCopy.texture.Get(); - Device* device = ToBackend(texture->GetDevice()); - - if (!device->IsToggleEnabled(Toggle::D3D12SplitBufferTextureCopyForRowsPerImagePaddings) || - texture->GetDimension() != wgpu::TextureDimension::e3D || + if (texture->GetDimension() != wgpu::TextureDimension::e3D || copySize.depthOrArrayLayers <= 1 || bufferCopy.rowsPerImage <= copySize.height) { return false; }
diff --git a/src/dawn/tests/end2end/RequiredBufferSizeInCopyTests.cpp b/src/dawn/tests/end2end/RequiredBufferSizeInCopyTests.cpp index 1704a89..35b9e18 100644 --- a/src/dawn/tests/end2end/RequiredBufferSizeInCopyTests.cpp +++ b/src/dawn/tests/end2end/RequiredBufferSizeInCopyTests.cpp
@@ -211,15 +211,13 @@ DoTest(size, copySize, rowsPerImage); } -DAWN_INSTANTIATE_TEST_P( - RequiredBufferSizeInCopyTests, - {D3D11Backend(), D3D12Backend(), - D3D12Backend({"d3d12_split_buffer_texture_copy_for_rows_per_image_paddings"}), MetalBackend(), - OpenGLBackend(), OpenGLESBackend(), VulkanBackend()}, - {Type::T2BCopy, Type::B2TCopy}, - {wgpu::TextureDimension::e3D, wgpu::TextureDimension::e2D}, - {2u, 1u}, - {1u, 0u}); +DAWN_INSTANTIATE_TEST_P(RequiredBufferSizeInCopyTests, + {D3D11Backend(), D3D12Backend(), MetalBackend(), OpenGLBackend(), + OpenGLESBackend(), VulkanBackend()}, + {Type::T2BCopy, Type::B2TCopy}, + {wgpu::TextureDimension::e3D, wgpu::TextureDimension::e2D}, + {2u, 1u}, + {1u, 0u}); } // anonymous namespace } // namespace dawn