Don't use depth offset in TextureCopySplitter

Remove usage of the depth offset in TextureCopySplitter. Because there
is no slice pitch alignment requirement, it is simpler to express the
subresource footprint with just width and height. Fixes incorrect copies
when using WriteTexture on a texture with 64 or less width.

Bug: dawn:573
Change-Id: I51cb73b522a443b8b42d9ba290c69541211be4bb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/39200
Reviewed-by: Rafael Cintron <rafael.cintron@microsoft.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Brandon Jones <brandon1.jones@intel.com>
This commit is contained in:
Brandon Jones 2021-03-12 17:58:47 +00:00 committed by Commit Bot service account
parent a57308e60b
commit c53ea046a6
4 changed files with 81 additions and 22 deletions

View File

@ -23,17 +23,13 @@ namespace dawn_native { namespace d3d12 {
namespace {
Origin3D ComputeTexelOffsets(const TexelBlockInfo& blockInfo,
uint32_t offset,
uint32_t bytesPerRow,
uint32_t slicePitch) {
uint32_t bytesPerRow) {
ASSERT(bytesPerRow != 0);
ASSERT(slicePitch != 0);
uint32_t byteOffsetX = offset % bytesPerRow;
offset -= byteOffsetX;
uint32_t byteOffsetY = offset % slicePitch;
uint32_t byteOffsetZ = offset - byteOffsetY;
uint32_t byteOffsetY = offset - byteOffsetX;
return {byteOffsetX / blockInfo.byteSize * blockInfo.width,
byteOffsetY / bytesPerRow * blockInfo.height, byteOffsetZ / slicePitch};
byteOffsetY / bytesPerRow * blockInfo.height, 0};
}
} // namespace
@ -47,10 +43,15 @@ namespace dawn_native { namespace d3d12 {
ASSERT(bytesPerRow % blockInfo.byteSize == 0);
// The copies must be 512-aligned. To do this, we calculate the first 512-aligned address
// preceding our data.
uint64_t alignedOffset =
offset & ~static_cast<uint64_t>(D3D12_TEXTURE_DATA_PLACEMENT_ALIGNMENT - 1);
copy.offset = alignedOffset;
// If the provided offset to the data was already 512-aligned, we can simply copy the data
// without further translation.
if (offset == alignedOffset) {
copy.count = 1;
@ -63,17 +64,36 @@ namespace dawn_native { namespace d3d12 {
copy.copies[0].bufferOffset.z = 0;
copy.copies[0].bufferSize = copySize;
// Return early. There is only one copy needed because the offset is already 512-byte
// aligned
return copy;
}
ASSERT(alignedOffset < offset);
ASSERT(offset - alignedOffset < D3D12_TEXTURE_DATA_PLACEMENT_ALIGNMENT);
uint32_t slicePitch = bytesPerRow * rowsPerImage;
// We must reinterpret our aligned offset into X and Y offsets with respect to the row
// pitch.
//
// You can visualize the data in the buffer like this:
// |-----------------------++++++++++++++++++++++++++++++++|
// ^ 512-aligned address ^ Aligned offset ^ End of copy data
//
// Now when you consider the row pitch, you can visualize the data like this:
// |~~~~~~~~~~~~~~~~|
// |~~~~~+++++++++++|
// |++++++++++++++++|
// |+++++~~~~~~~~~~~|
// |<---row pitch-->|
//
// The X and Y offsets calculated in ComputeTexelOffsets can be visualized like this:
// |YYYYYYYYYYYYYYYY|
// |XXXXXX++++++++++|
// |++++++++++++++++|
// |++++++~~~~~~~~~~|
// |<---row pitch-->|
Origin3D texelOffset = ComputeTexelOffsets(
blockInfo, static_cast<uint32_t>(offset - alignedOffset), bytesPerRow, slicePitch);
blockInfo, static_cast<uint32_t>(offset - alignedOffset), bytesPerRow);
ASSERT(texelOffset.z == 0);
uint32_t copyBytesPerRowPitch = copySize.width / blockInfo.width * blockInfo.byteSize;
uint32_t byteOffsetInRowPitch = texelOffset.x / blockInfo.width * blockInfo.byteSize;
@ -111,7 +131,7 @@ namespace dawn_native { namespace d3d12 {
copy.copies[0].bufferOffset = texelOffset;
copy.copies[0].bufferSize.width = copySize.width + texelOffset.x;
copy.copies[0].bufferSize.height = rowsPerImageInTexels + texelOffset.y;
copy.copies[0].bufferSize.depth = copySize.depth + texelOffset.z;
copy.copies[0].bufferSize.depth = copySize.depth;
return copy;
}
@ -163,7 +183,7 @@ namespace dawn_native { namespace d3d12 {
copy.copies[0].bufferOffset = texelOffset;
copy.copies[0].bufferSize.width = texelsPerRow;
copy.copies[0].bufferSize.height = rowsPerImageInTexels + texelOffset.y;
copy.copies[0].bufferSize.depth = copySize.depth + texelOffset.z;
copy.copies[0].bufferSize.depth = copySize.depth;
copy.copies[1].textureOffset.x = origin.x + copy.copies[0].copySize.width;
copy.copies[1].textureOffset.y = origin.y;
@ -176,10 +196,10 @@ namespace dawn_native { namespace d3d12 {
copy.copies[1].bufferOffset.x = 0;
copy.copies[1].bufferOffset.y = texelOffset.y + blockInfo.height;
copy.copies[1].bufferOffset.z = texelOffset.z;
copy.copies[1].bufferOffset.z = 0;
copy.copies[1].bufferSize.width = copy.copies[1].copySize.width;
copy.copies[1].bufferSize.height = rowsPerImageInTexels + texelOffset.y + blockInfo.height;
copy.copies[1].bufferSize.depth = copySize.depth + texelOffset.z;
copy.copies[1].bufferSize.depth = copySize.depth;
return copy;
}

View File

@ -1565,10 +1565,6 @@ TEST_P(CopyTests_T2T, MultipleMipSrcSingleMipDst) {
// A regression test for a bug on D3D12 backend that causes crash when doing texture-to-texture
// copy one row with the texture format Depth32Float.
TEST_P(CopyTests_T2B, CopyOneRowWithDepth32Float) {
// TODO(jiawei.shao@intel.com): enable this test when the bug on D3D12 is fixed. See dawn:693
// for more details.
DAWN_SKIP_TEST_IF(IsD3D12());
constexpr wgpu::TextureFormat kFormat = wgpu::TextureFormat::Depth32Float;
constexpr uint32_t kPixelsPerRow = 4u;

View File

@ -322,6 +322,32 @@ class QueueWriteTextureTests : public DawnTest {
dataOffset += bytesPerImage;
}
}
void DoSimpleWriteTextureTest(uint32_t width, uint32_t height) {
constexpr wgpu::TextureFormat kFormat = wgpu::TextureFormat::RGBA8Unorm;
constexpr uint32_t kPixelSize = 4;
std::vector<uint32_t> data(width * height);
for (size_t i = 0; i < data.size(); i++) {
data[i] = 0xFFFFFFFF;
}
wgpu::TextureDescriptor descriptor = {};
descriptor.size = {width, height, 1};
descriptor.format = kFormat;
descriptor.usage = wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::CopySrc;
wgpu::Texture texture = device.CreateTexture(&descriptor);
wgpu::ImageCopyTexture imageCopyTexture =
utils::CreateImageCopyTexture(texture, 0, {0, 0, 0});
wgpu::TextureDataLayout textureDataLayout =
utils::CreateTextureDataLayout(0, width * kPixelSize);
wgpu::Extent3D copyExtent = {width, height, 1};
device.GetQueue().WriteTexture(&imageCopyTexture, data.data(), width * height * kPixelSize,
&textureDataLayout, &copyExtent);
EXPECT_TEXTURE_RGBA8_EQ(data.data(), texture, 0, 0, width, height, 0, 0);
}
};
// Test writing the whole texture for varying texture sizes.
@ -613,6 +639,19 @@ TEST_P(QueueWriteTextureTests, UnalignedDynamicUploader) {
DoTest(textureSpec, MinimumDataSpec(size), size);
}
// This tests for a bug that occurred within the D3D12 CopyTextureSplitter, which incorrectly copied
// data when the internal offset was larger than 256, but less than 512 and the copy size was 64
// width or less with a height of 1.
TEST_P(QueueWriteTextureTests, WriteTo64x1TextureFromUnalignedDynamicUploader) {
// First, WriteTexture with 96 pixels, or 384 bytes to create an offset in the dynamic uploader.
DoSimpleWriteTextureTest(96, 1);
// Now test writing to a 64x1 texture. Because a 64x1 texture's row pitch is equal to its slice
// pitch, the texture copy offset could be calculated incorrectly inside the internal D3D12
// TextureCopySplitter.
DoSimpleWriteTextureTest(64, 1);
}
DAWN_INSTANTIATE_TEST(QueueWriteTextureTests,
D3D12Backend(),
MetalBackend(),

View File

@ -141,8 +141,9 @@ namespace {
uint32_t absoluteTexelOffset =
copySplit.offset / textureSpec.texelBlockSizeInBytes * texelsPerBlock +
copy.bufferOffset.x / textureSpec.blockWidth * texelsPerBlock +
copy.bufferOffset.y / textureSpec.blockHeight * bytesPerRowInTexels +
copy.bufferOffset.z * slicePitchInTexels;
copy.bufferOffset.y / textureSpec.blockHeight * bytesPerRowInTexels;
ASSERT_EQ(copy.bufferOffset.z, 0u);
ASSERT(absoluteTexelOffset >=
bufferSpec.offset / textureSpec.texelBlockSizeInBytes * texelsPerBlock);
@ -203,6 +204,7 @@ namespace {
// Define base texture sizes and offsets to test with: some aligned, some unaligned
constexpr TextureSpec kBaseTextureSpecs[] = {
{0, 0, 0, 1, 1, 1, 4},
{0, 0, 0, 64, 1, 1, 4},
{31, 16, 0, 1, 1, 1, 4},
{64, 16, 0, 1, 1, 1, 4},
{64, 16, 8, 1, 1, 1, 4},
@ -237,7 +239,7 @@ namespace {
// Define base buffer sizes to work with: some offsets aligned, some unaligned. bytesPerRow is
// the minimum required
std::array<BufferSpec, 13> BaseBufferSpecs(const TextureSpec& textureSpec) {
std::array<BufferSpec, 14> BaseBufferSpecs(const TextureSpec& textureSpec) {
uint32_t bytesPerRow = Align(textureSpec.texelBlockSizeInBytes * textureSpec.width,
kTextureBytesPerRowAlignment);
@ -266,6 +268,8 @@ namespace {
textureSpec.height},
BufferSpec{alignNonPow2(257, textureSpec.texelBlockSizeInBytes), bytesPerRow,
textureSpec.height},
BufferSpec{alignNonPow2(384, textureSpec.texelBlockSizeInBytes), bytesPerRow,
textureSpec.height},
BufferSpec{alignNonPow2(511, textureSpec.texelBlockSizeInBytes), bytesPerRow,
textureSpec.height},
BufferSpec{alignNonPow2(513, textureSpec.texelBlockSizeInBytes), bytesPerRow,