From ee0516e3989ff6a7047083c7f317210245375b79 Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Mon, 8 Jun 2020 09:53:11 +0000 Subject: [PATCH] D3D12: Properly compute rowsPerImage for lazy texture clear This code was incorrectly passing 0 and dividing by zero for non-renderable, unaligned-size textures. Bug: dawn:451 Change-Id: Ic79183a7ccee712b9a9ee002056f090c8e9fae15 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22665 Reviewed-by: Jiawei Shao Reviewed-by: Corentin Wallez Commit-Queue: Corentin Wallez --- src/dawn_native/d3d12/TextureCopySplitter.cpp | 2 ++ src/dawn_native/d3d12/TextureD3D12.cpp | 7 ++-- src/tests/end2end/TextureZeroInitTests.cpp | 36 +++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/dawn_native/d3d12/TextureCopySplitter.cpp b/src/dawn_native/d3d12/TextureCopySplitter.cpp index cfa1d1eec4..f2a8388220 100644 --- a/src/dawn_native/d3d12/TextureCopySplitter.cpp +++ b/src/dawn_native/d3d12/TextureCopySplitter.cpp @@ -25,6 +25,8 @@ namespace dawn_native { namespace d3d12 { uint32_t offset, uint32_t bytesPerRow, uint32_t slicePitch) { + ASSERT(bytesPerRow != 0); + ASSERT(slicePitch != 0); uint32_t byteOffsetX = offset % bytesPerRow; offset -= byteOffsetX; uint32_t byteOffsetY = offset % slicePitch; diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp index d9cf9b7638..77966b48d7 100644 --- a/src/dawn_native/d3d12/TextureD3D12.cpp +++ b/src/dawn_native/d3d12/TextureD3D12.cpp @@ -823,8 +823,11 @@ namespace dawn_native { namespace d3d12 { for (uint32_t level = baseMipLevel; level < baseMipLevel + levelCount; ++level) { // compute d3d12 texture copy locations for texture and buffer Extent3D copySize = GetMipLevelVirtualSize(level); - TextureCopySplit copySplit = ComputeTextureCopySplit( - {0, 0, 0}, copySize, GetFormat(), uploadHandle.startOffset, bytesPerRow, 0); + + uint32_t rowsPerImage = GetSize().height; + TextureCopySplit copySplit = + ComputeTextureCopySplit({0, 0, 0}, copySize, GetFormat(), + uploadHandle.startOffset, bytesPerRow, rowsPerImage); for (uint32_t layer = baseArrayLayer; layer < baseArrayLayer + layerCount; ++layer) { diff --git a/src/tests/end2end/TextureZeroInitTests.cpp b/src/tests/end2end/TextureZeroInitTests.cpp index 40962f544f..82155cf38c 100644 --- a/src/tests/end2end/TextureZeroInitTests.cpp +++ b/src/tests/end2end/TextureZeroInitTests.cpp @@ -1034,6 +1034,42 @@ TEST_P(TextureZeroInitTest, PreservesInitializedArrayLayer) { EXPECT_EQ(true, dawn_native::IsTextureSubresourceInitialized(sampleTexture.Get(), 0, 1, 0, 2)); } +// This is a regression test for crbug.com/dawn/451 where the lazy texture +// init path on D3D12 had a divide-by-zero exception in the copy split logic. +TEST_P(TextureZeroInitTest, CopyTextureToBufferNonRenderableUnaligned) { + wgpu::TextureDescriptor descriptor; + descriptor.size.width = kUnalignedSize; + descriptor.size.height = kUnalignedSize; + descriptor.size.depth = 1; + descriptor.format = wgpu::TextureFormat::R8Snorm; + descriptor.usage = wgpu::TextureUsage::CopySrc; + wgpu::Texture texture = device.CreateTexture(&descriptor); + + { + uint32_t bytesPerRow = Align(kUnalignedSize, kTextureBytesPerRowAlignment); + + wgpu::BufferDescriptor bufferDesc; + bufferDesc.size = kUnalignedSize * bytesPerRow; + bufferDesc.usage = wgpu::BufferUsage::CopyDst; + wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc); + + wgpu::TextureCopyView textureCopyView = + utils::CreateTextureCopyView(texture, 0, 0, {0, 0, 0}); + wgpu::BufferCopyView bufferCopyView = + utils::CreateBufferCopyView(buffer, 0, bytesPerRow, 0); + wgpu::Extent3D copySize = {kUnalignedSize, kUnalignedSize, 1}; + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.CopyTextureToBuffer(&textureCopyView, &bufferCopyView, ©Size); + + wgpu::CommandBuffer commands = encoder.Finish(); + EXPECT_LAZY_CLEAR(1u, queue.Submit(1, &commands)); + } + + // Expect texture subresource initialized to be true + EXPECT_EQ(true, dawn_native::IsTextureSubresourceInitialized(texture.Get(), 0, 1, 0, 1)); +} + DAWN_INSTANTIATE_TEST( TextureZeroInitTest, D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}),