Implement BufferCopyView.imageHeight Parameter
Implement BufferCopyView.imageHeight Parameter. Adds validation tests.
Adequate functional testing not possible without 3D texture support.
Bug: dawn:48
Change-Id: I00e4464eb451c948dee2890a11fb0984eff681a0
Reviewed-on: https://dawn-review.googlesource.com/c/2980
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp
index 2f12197..c9e6c25 100644
--- a/src/dawn_native/CommandBuffer.cpp
+++ b/src/dawn_native/CommandBuffer.cpp
@@ -88,11 +88,24 @@
return {};
}
+ MaybeError ValidateImageHeight(uint32_t imageHeight, uint32_t copyHeight) {
+ if (imageHeight < copyHeight) {
+ return DAWN_VALIDATION_ERROR("Image height must not be less than the copy height.");
+ }
+
+ return {};
+ }
+
MaybeError ComputeTextureCopyBufferSize(const Extent3D& copySize,
uint32_t rowPitch,
+ uint32_t imageHeight,
uint32_t* bufferSize) {
+ DAWN_TRY(ValidateImageHeight(imageHeight, copySize.height));
+
// TODO(cwallez@chromium.org): check for overflows
- *bufferSize = (rowPitch * (copySize.height - 1) + copySize.width) * copySize.depth;
+ uint32_t slicePitch = rowPitch * imageHeight;
+ uint32_t sliceSize = rowPitch * (copySize.height - 1) + copySize.width;
+ *bufferSize = (slicePitch * (copySize.depth - 1)) + sliceSize;
return {};
}
@@ -388,7 +401,9 @@
uint32_t bufferCopySize = 0;
DAWN_TRY(ValidateRowPitch(copy->destination.texture->GetFormat(),
copy->copySize, copy->source.rowPitch));
+
DAWN_TRY(ComputeTextureCopyBufferSize(copy->copySize, copy->source.rowPitch,
+ copy->source.imageHeight,
&bufferCopySize));
DAWN_TRY(ValidateCopySizeFitsInTexture(copy->destination, copy->copySize));
@@ -412,7 +427,8 @@
DAWN_TRY(ValidateRowPitch(copy->source.texture->GetFormat(), copy->copySize,
copy->destination.rowPitch));
DAWN_TRY(ComputeTextureCopyBufferSize(
- copy->copySize, copy->destination.rowPitch, &bufferCopySize));
+ copy->copySize, copy->destination.rowPitch, copy->destination.imageHeight,
+ &bufferCopySize));
DAWN_TRY(ValidateCopySizeFitsInTexture(copy->source, copy->copySize));
DAWN_TRY(ValidateCopySizeFitsInBuffer(copy->destination, bufferCopySize));
@@ -695,6 +711,11 @@
} else {
copy->source.rowPitch = source->rowPitch;
}
+ if (source->imageHeight == 0) {
+ copy->source.imageHeight = copySize->height;
+ } else {
+ copy->source.imageHeight = source->imageHeight;
+ }
}
void CommandBufferBuilder::CopyTextureToBuffer(const TextureCopyView* source,
@@ -729,6 +750,11 @@
} else {
copy->destination.rowPitch = destination->rowPitch;
}
+ if (destination->imageHeight == 0) {
+ copy->destination.imageHeight = copySize->height;
+ } else {
+ copy->destination.imageHeight = destination->imageHeight;
+ }
}
} // namespace dawn_native
diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
index 2044097..37bce73 100644
--- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp
+++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
@@ -331,7 +331,7 @@
auto copySplit = ComputeTextureCopySplit(
copy->destination.origin, copy->copySize,
static_cast<uint32_t>(TextureFormatPixelSize(texture->GetFormat())),
- copy->source.offset, copy->source.rowPitch);
+ copy->source.offset, copy->source.rowPitch, copy->source.imageHeight);
D3D12_TEXTURE_COPY_LOCATION textureLocation;
textureLocation.pResource = texture->GetD3D12Resource();
@@ -378,7 +378,8 @@
auto copySplit = ComputeTextureCopySplit(
copy->source.origin, copy->copySize,
static_cast<uint32_t>(TextureFormatPixelSize(texture->GetFormat())),
- copy->destination.offset, copy->destination.rowPitch);
+ copy->destination.offset, copy->destination.rowPitch,
+ copy->destination.imageHeight);
D3D12_TEXTURE_COPY_LOCATION textureLocation;
textureLocation.pResource = texture->GetD3D12Resource();
diff --git a/src/dawn_native/d3d12/TextureCopySplitter.cpp b/src/dawn_native/d3d12/TextureCopySplitter.cpp
index 7c58e89..5e58cbc 100644
--- a/src/dawn_native/d3d12/TextureCopySplitter.cpp
+++ b/src/dawn_native/d3d12/TextureCopySplitter.cpp
@@ -40,15 +40,10 @@
Extent3D copySize,
uint32_t texelSize,
uint32_t offset,
- uint32_t rowPitch) {
+ uint32_t rowPitch,
+ uint32_t imageHeight) {
TextureCopySplit copy;
- if (origin.z != 0 || copySize.depth > 1) {
- // TODO(enga@google.com): Handle 3D
- ASSERT(false);
- return copy;
- }
-
ASSERT(rowPitch % texelSize == 0);
uint32_t alignedOffset = offset & ~(D3D12_TEXTURE_DATA_PLACEMENT_ALIGNMENT - 1);
@@ -74,7 +69,7 @@
ASSERT(alignedOffset < offset);
Origin3D texelOffset;
- ComputeTexelOffsets(offset - alignedOffset, rowPitch, rowPitch * copySize.height, texelSize,
+ ComputeTexelOffsets(offset - alignedOffset, rowPitch, rowPitch * imageHeight, texelSize,
&texelOffset);
uint32_t rowPitchInTexels = rowPitch / texelSize;
@@ -111,7 +106,7 @@
copy.copies[0].bufferOffset = texelOffset;
copy.copies[0].bufferSize.width = copySize.width + texelOffset.x;
- copy.copies[0].bufferSize.height = copySize.height + texelOffset.y;
+ copy.copies[0].bufferSize.height = imageHeight + texelOffset.y;
copy.copies[0].bufferSize.depth = copySize.depth + texelOffset.z;
return copy;
@@ -162,7 +157,7 @@
copy.copies[0].bufferOffset = texelOffset;
copy.copies[0].bufferSize.width = rowPitchInTexels;
- copy.copies[0].bufferSize.height = copySize.height + texelOffset.y;
+ copy.copies[0].bufferSize.height = imageHeight + texelOffset.y;
copy.copies[0].bufferSize.depth = copySize.depth + texelOffset.z;
copy.copies[1].textureOffset.x = origin.x + copy.copies[0].copySize.width;
@@ -178,7 +173,7 @@
copy.copies[1].bufferOffset.y = texelOffset.y + 1;
copy.copies[1].bufferOffset.z = texelOffset.z;
copy.copies[1].bufferSize.width = copy.copies[1].copySize.width;
- copy.copies[1].bufferSize.height = copySize.height + texelOffset.y + 1;
+ copy.copies[1].bufferSize.height = imageHeight + texelOffset.y + 1;
copy.copies[1].bufferSize.depth = copySize.depth + texelOffset.z;
return copy;
diff --git a/src/dawn_native/d3d12/TextureCopySplitter.h b/src/dawn_native/d3d12/TextureCopySplitter.h
index f82da87..e468fc6 100644
--- a/src/dawn_native/d3d12/TextureCopySplitter.h
+++ b/src/dawn_native/d3d12/TextureCopySplitter.h
@@ -41,8 +41,8 @@
Extent3D copySize,
uint32_t texelSize,
uint32_t offset,
- uint32_t rowPitch);
-
+ uint32_t rowPitch,
+ uint32_t imageHeight);
}} // namespace dawn_native::d3d12
#endif // DAWNNATIVE_D3D12_TEXTURECOPYSPLITTER_H_
diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm
index 03066ed..7726fcc 100644
--- a/src/dawn_native/metal/CommandBufferMTL.mm
+++ b/src/dawn_native/metal/CommandBufferMTL.mm
@@ -265,7 +265,7 @@
[encoders.blit copyFromBuffer:buffer->GetMTLBuffer()
sourceOffset:src.offset
sourceBytesPerRow:src.rowPitch
- sourceBytesPerImage:(src.rowPitch * copySize.height)
+ sourceBytesPerImage:(src.rowPitch * src.imageHeight)
sourceSize:size
toTexture:texture->GetMTLTexture()
destinationSlice:dst.slice
@@ -300,7 +300,7 @@
toBuffer:buffer->GetMTLBuffer()
destinationOffset:dst.offset
destinationBytesPerRow:dst.rowPitch
- destinationBytesPerImage:dst.rowPitch * copySize.height];
+ destinationBytesPerImage:(dst.rowPitch * dst.imageHeight)];
} break;
default: { UNREACHABLE(); } break;
diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp
index 7ac7bdf..6602db8 100644
--- a/src/dawn_native/opengl/CommandBufferGL.cpp
+++ b/src/dawn_native/opengl/CommandBufferGL.cpp
@@ -333,6 +333,7 @@
glPixelStorei(GL_UNPACK_ROW_LENGTH,
src.rowPitch / TextureFormatPixelSize(texture->GetFormat()));
+ glPixelStorei(GL_UNPACK_IMAGE_HEIGHT, src.imageHeight);
switch (texture->GetDimension()) {
case dawn::TextureDimension::e2D:
if (texture->GetArrayLayers() > 1) {
@@ -352,6 +353,7 @@
UNREACHABLE();
}
glPixelStorei(GL_UNPACK_ROW_LENGTH, 0);
+ glPixelStorei(GL_UNPACK_IMAGE_HEIGHT, 0);
glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
} break;
@@ -393,11 +395,13 @@
glBindBuffer(GL_PIXEL_PACK_BUFFER, buffer->GetHandle());
glPixelStorei(GL_PACK_ROW_LENGTH,
dst.rowPitch / TextureFormatPixelSize(texture->GetFormat()));
+ glPixelStorei(GL_PACK_IMAGE_HEIGHT, dst.imageHeight);
ASSERT(copySize.depth == 1 && src.origin.z == 0);
void* offset = reinterpret_cast<void*>(static_cast<uintptr_t>(dst.offset));
glReadPixels(src.origin.x, src.origin.y, copySize.width, copySize.height,
format.format, format.type, offset);
glPixelStorei(GL_PACK_ROW_LENGTH, 0);
+ glPixelStorei(GL_PACK_IMAGE_HEIGHT, 0);
glBindBuffer(GL_PIXEL_PACK_BUFFER, 0);
glDeleteFramebuffers(1, &readFBO);
diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp
index 9ebdb70..ab165b0 100644
--- a/src/dawn_native/vulkan/CommandBufferVk.cpp
+++ b/src/dawn_native/vulkan/CommandBufferVk.cpp
@@ -50,7 +50,7 @@
// In Vulkan the row length is in texels while it is in bytes for Dawn
region.bufferRowLength =
bufferCopy.rowPitch / TextureFormatPixelSize(texture->GetFormat());
- region.bufferImageHeight = bufferCopy.rowPitch * copySize.height;
+ region.bufferImageHeight = bufferCopy.imageHeight;
region.imageSubresource.aspectMask = texture->GetVkAspectMask();
region.imageSubresource.mipLevel = textureCopy.level;
diff --git a/src/tests/unittests/d3d12/CopySplitTests.cpp b/src/tests/unittests/d3d12/CopySplitTests.cpp
index e3d6688..ff55415 100644
--- a/src/tests/unittests/d3d12/CopySplitTests.cpp
+++ b/src/tests/unittests/d3d12/CopySplitTests.cpp
@@ -37,6 +37,7 @@
struct BufferSpec {
uint32_t offset;
uint32_t rowPitch;
+ uint32_t imageHeight;
};
// Check that each copy region fits inside the buffer footprint
@@ -119,7 +120,7 @@
const auto& copy = copySplit.copies[i];
uint32_t rowPitchInTexels = bufferSpec.rowPitch / textureSpec.texelSize;
- uint32_t slicePitchInTexels = rowPitchInTexels * copy.copySize.height;
+ uint32_t slicePitchInTexels = rowPitchInTexels * bufferSpec.imageHeight;
uint32_t absoluteTexelOffset = copySplit.offset / textureSpec.texelSize + copy.bufferOffset.x + copy.bufferOffset.y * rowPitchInTexels + copy.bufferOffset.z * slicePitchInTexels;
ASSERT(absoluteTexelOffset >= bufferSpec.offset / textureSpec.texelSize);
@@ -153,7 +154,8 @@
}
std::ostream& operator<<(std::ostream& os, const BufferSpec& bufferSpec) {
- os << "BufferSpec(" << bufferSpec.offset << ", " << bufferSpec.rowPitch << ")";
+ os << "BufferSpec(" << bufferSpec.offset << ", " << bufferSpec.rowPitch << ", "
+ << bufferSpec.imageHeight << ")";
return os;
}
@@ -169,22 +171,25 @@
// Define base texture sizes and offsets to test with: some aligned, some unaligned
constexpr TextureSpec kBaseTextureSpecs[] = {
- { 0, 0, 0, 1, 1, 1, 4 },
- { 31, 16, 0, 1, 1, 1, 4 },
- { 64, 16, 0, 1, 1, 1, 4 },
+ {0, 0, 0, 1, 1, 1, 4},
+ {31, 16, 0, 1, 1, 1, 4},
+ {64, 16, 0, 1, 1, 1, 4},
+ {64, 16, 8, 1, 1, 1, 4},
- { 0, 0, 0, 1024, 1024, 1, 4 },
- { 256, 512, 0, 1024, 1024, 1, 4 },
- { 64, 48, 0, 1024, 1024, 1, 4 },
+ {0, 0, 0, 1024, 1024, 1, 4},
+ {256, 512, 0, 1024, 1024, 1, 4},
+ {64, 48, 0, 1024, 1024, 1, 4},
+ {64, 48, 16, 1024, 1024, 1024, 4},
- { 0, 0, 0, 257, 31, 1, 4 },
- { 0, 0, 0, 17, 93, 1, 4 },
- { 59, 13, 0, 257, 31, 1, 4 },
- { 17, 73, 0, 17, 93, 1, 4 },
+ {0, 0, 0, 257, 31, 1, 4},
+ {0, 0, 0, 17, 93, 1, 4},
+ {59, 13, 0, 257, 31, 1, 4},
+ {17, 73, 0, 17, 93, 1, 4},
+ {17, 73, 59, 17, 93, 99, 4},
};
// Define base buffer sizes to work with: some offsets aligned, some unaligned. rowPitch is the minimum required
- std::array<BufferSpec, 10> BaseBufferSpecs(const TextureSpec& textureSpec) {
+ std::array<BufferSpec, 13> BaseBufferSpecs(const TextureSpec& textureSpec) {
uint32_t rowPitch = Align(textureSpec.texelSize * textureSpec.width, kTextureRowPitchAlignment);
auto alignNonPow2 = [](uint32_t value, uint32_t size) -> uint32_t {
@@ -192,18 +197,21 @@
};
return {
- BufferSpec{alignNonPow2(0, textureSpec.texelSize), rowPitch},
- BufferSpec{alignNonPow2(512, textureSpec.texelSize), rowPitch},
- BufferSpec{alignNonPow2(1024, textureSpec.texelSize), rowPitch},
+ BufferSpec{alignNonPow2(0, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(512, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(1024, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(1024, textureSpec.texelSize), rowPitch, textureSpec.height * 2},
- BufferSpec{alignNonPow2(32, textureSpec.texelSize), rowPitch},
- BufferSpec{alignNonPow2(64, textureSpec.texelSize), rowPitch},
+ BufferSpec{alignNonPow2(32, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(64, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(64, textureSpec.texelSize), rowPitch, textureSpec.height * 2},
- BufferSpec{alignNonPow2(31, textureSpec.texelSize), rowPitch},
- BufferSpec{alignNonPow2(257, textureSpec.texelSize), rowPitch},
- BufferSpec{alignNonPow2(511, textureSpec.texelSize), rowPitch},
- BufferSpec{alignNonPow2(513, textureSpec.texelSize), rowPitch},
- BufferSpec{alignNonPow2(1023, textureSpec.texelSize), rowPitch},
+ BufferSpec{alignNonPow2(31, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(257, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(511, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(513, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(1023, textureSpec.texelSize), rowPitch, textureSpec.height},
+ BufferSpec{alignNonPow2(1023, textureSpec.texelSize), rowPitch, textureSpec.height * 2},
};
}
@@ -223,7 +231,7 @@
TextureCopySplit copySplit = ComputeTextureCopySplit(
{textureSpec.x, textureSpec.y, textureSpec.z},
{textureSpec.width, textureSpec.height, textureSpec.depth}, textureSpec.texelSize,
- bufferSpec.offset, bufferSpec.rowPitch);
+ bufferSpec.offset, bufferSpec.rowPitch, bufferSpec.imageHeight);
ValidateCopySplit(textureSpec, bufferSpec, copySplit);
return copySplit;
}
@@ -370,3 +378,23 @@
}
}
}
+
+TEST_F(CopySplitTest, ImageHeight) {
+ for (TextureSpec textureSpec : kBaseTextureSpecs) {
+ for (BufferSpec bufferSpec : BaseBufferSpecs(textureSpec)) {
+ uint32_t baseImageHeight = bufferSpec.imageHeight;
+ for (uint32_t i = 0; i < 5; ++i) {
+ bufferSpec.imageHeight = baseImageHeight + i * 256;
+
+ TextureCopySplit copySplit = DoTest(textureSpec, bufferSpec);
+ if (HasFatalFailure()) {
+ std::ostringstream message;
+ message << "Failed generating splits: " << textureSpec << ", " << bufferSpec
+ << std::endl
+ << copySplit << std::endl;
+ FAIL() << message.str();
+ }
+ }
+ }
+ }
+}
diff --git a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
index 636df88..6f41450 100644
--- a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
+++ b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
@@ -340,6 +340,29 @@
dawn::TextureAspect::Color, {65, 1, 1});
}
+TEST_F(CopyCommandTest_B2T, ImageHeightConstraint) {
+ uint32_t bufferSize = BufferSizeForTextureCopy(5, 5, 1);
+ dawn::Buffer source = CreateBuffer(bufferSize, dawn::BufferUsageBit::TransferSrc);
+ dawn::Texture destination = Create2DTexture(16, 16, 1, 1, dawn::TextureFormat::R8G8B8A8Unorm,
+ dawn::TextureUsageBit::TransferDst);
+
+ // Image height is zero (Valid)
+ TestB2TCopy(utils::Expectation::Success, source, 0, 256, 0, destination, 0, 0, {0, 0, 0},
+ dawn::TextureAspect::Color, {4, 4, 1});
+
+ // Image height is equal to copy height (Valid)
+ TestB2TCopy(utils::Expectation::Success, source, 0, 256, 0, destination, 0, 0, {0, 0, 0},
+ dawn::TextureAspect::Color, {4, 4, 1});
+
+ // Image height is larger than copy height (Valid)
+ TestB2TCopy(utils::Expectation::Success, source, 0, 256, 4, destination, 0, 0, {0, 0, 0},
+ dawn::TextureAspect::Color, {4, 4, 1});
+
+ // Image height is less than copy height (Invalid)
+ TestB2TCopy(utils::Expectation::Failure, source, 0, 256, 3, destination, 0, 0, {0, 0, 0},
+ dawn::TextureAspect::Color, {4, 4, 1});
+}
+
// Test B2T copies with incorrect buffer offset usage
TEST_F(CopyCommandTest_B2T, IncorrectBufferOffset) {
uint32_t bufferSize = BufferSizeForTextureCopy(4, 4, 1);
@@ -523,6 +546,29 @@
destination, 0, 256, 0, {65, 1, 1});
}
+TEST_F(CopyCommandTest_T2B, ImageHeightConstraint) {
+ uint32_t bufferSize = BufferSizeForTextureCopy(5, 5, 1);
+ dawn::Texture source = Create2DTexture(16, 16, 1, 1, dawn::TextureFormat::R8G8B8A8Unorm,
+ dawn::TextureUsageBit::TransferSrc);
+ dawn::Buffer destination = CreateBuffer(bufferSize, dawn::BufferUsageBit::TransferDst);
+
+ // Image height is zero (Valid)
+ TestT2BCopy(utils::Expectation::Success, source, 0, 0, {0, 0, 0}, dawn::TextureAspect::Color,
+ destination, 0, 256, 0, {4, 4, 1});
+
+ // Image height is equal to copy height (Valid)
+ TestT2BCopy(utils::Expectation::Success, source, 0, 0, {0, 0, 0}, dawn::TextureAspect::Color,
+ destination, 0, 256, 4, {4, 4, 1});
+
+ // Image height exceeds copy height (Valid)
+ TestT2BCopy(utils::Expectation::Success, source, 0, 0, {0, 0, 0}, dawn::TextureAspect::Color,
+ destination, 0, 256, 5, {4, 4, 1});
+
+ // Image height is less than copy height (Invalid)
+ TestT2BCopy(utils::Expectation::Failure, source, 0, 0, {0, 0, 0}, dawn::TextureAspect::Color,
+ destination, 0, 256, 3, {4, 4, 1});
+}
+
// Test T2B copies with incorrect buffer offset usage
TEST_F(CopyCommandTest_T2B, IncorrectBufferOffset) {
uint32_t bufferSize = BufferSizeForTextureCopy(128, 16, 1);
