Skip to content

Commit

Permalink
D3D12: Hard-enable D3D12SplitBufferTextureCopyForRowsPerImagePaddings
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Kai Ninomiya <[email protected]>
Reviewed-by: Loko Kung <[email protected]>
  • Loading branch information
Jiawei-Shao authored and Dawn LUCI CQ committed Dec 16, 2024
1 parent ad54e95 commit afc9c13
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 32 deletions.
8 changes: 0 additions & 8 deletions src/dawn/native/Toggles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,14 +265,6 @@ static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{
"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 "
Expand Down
1 change: 0 additions & 1 deletion src/dawn/native/Toggles.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ enum class Toggle {
TimestampQuantization,
ClearBufferBeforeResolveQueries,
VulkanUseZeroInitializeWorkgroupMemoryExtension,
D3D12SplitBufferTextureCopyForRowsPerImagePaddings,
MetalRenderR8RG8UnormSmallMipToTempTexture,
DisableBlobCache,
D3D12ForceClearCopyableDepthStencilTextureOnCreation,
Expand Down
7 changes: 0 additions & 7 deletions src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -702,13 +702,6 @@ void PhysicalDevice::SetupBackendDeviceToggles(dawn::platform::Platform* platfor
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)) {
Expand Down
10 changes: 3 additions & 7 deletions src/dawn/native/d3d12/UtilsD3D12.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ uint64_t RequiredCopySizeByD3D12(const uint32_t bytesPerRow,
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).
Expand All @@ -79,10 +78,7 @@ bool NeedBufferSizeWorkaroundForBufferTextureCopyOnD3D12(const BufferCopy& buffe
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;
}
Expand Down
16 changes: 7 additions & 9 deletions src/dawn/tests/end2end/RequiredBufferSizeInCopyTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,15 +211,13 @@ TEST_P(RequiredBufferSizeInCopyTests, MinimumBufferSize) {
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

0 comments on commit afc9c13

Please sign in to comment.