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>

src/dawn_native/CommandBuffer.cpp

@@ -88,11 +88,24 @@ namespace dawn_native {
             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 @@ namespace dawn_native {
                     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 @@ namespace dawn_native {
                     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 @@ namespace dawn_native {
         } 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 @@ namespace dawn_native {
         } else {
             copy->destination.rowPitch = destination->rowPitch;
         }
+        if (destination->imageHeight == 0) {
+            copy->destination.imageHeight = copySize->height;
+        } else {
+            copy->destination.imageHeight = destination->imageHeight;
+        }
     }
 
 }  // namespace dawn_native

src/dawn_native/d3d12/CommandBufferD3D12.cpp

@@ -331,7 +331,7 @@ namespace dawn_native { namespace d3d12 {
                     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 @@ namespace dawn_native { namespace d3d12 {
                     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();

src/dawn_native/d3d12/TextureCopySplitter.cpp

@@ -40,15 +40,10 @@ namespace dawn_native { namespace d3d12 {
                                              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 @@ namespace dawn_native { namespace d3d12 {
         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 @@ namespace dawn_native { namespace d3d12 {
 
             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 @@ namespace dawn_native { namespace d3d12 {
 
         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 @@ namespace dawn_native { namespace d3d12 {
         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;

src/dawn_native/d3d12/TextureCopySplitter.h

@@ -41,8 +41,8 @@ namespace dawn_native { namespace d3d12 {
                                              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_

src/dawn_native/metal/CommandBufferMTL.mm

@@ -265,7 +265,7 @@ namespace dawn_native { namespace metal {
                     [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 @@ namespace dawn_native { namespace metal {
                                           toBuffer:buffer->GetMTLBuffer()
                                  destinationOffset:dst.offset
                             destinationBytesPerRow:dst.rowPitch
-                          destinationBytesPerImage:dst.rowPitch * copySize.height];
+                          destinationBytesPerImage:(dst.rowPitch * dst.imageHeight)];
                 } break;
 
                 default: { UNREACHABLE(); } break;

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -333,6 +333,7 @@ namespace dawn_native { namespace opengl {
 
                     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 @@ namespace dawn_native { namespace opengl {
                             UNREACHABLE();
                     }
                     glPixelStorei(GL_UNPACK_ROW_LENGTH, 0);
+                    glPixelStorei(GL_UNPACK_IMAGE_HEIGHT, 0);
 
                     glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
                 } break;
@@ -393,11 +395,13 @@ namespace dawn_native { namespace opengl {
                     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);

src/dawn_native/vulkan/CommandBufferVk.cpp

@@ -50,7 +50,7 @@ namespace dawn_native { namespace vulkan {
             // 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;

src/tests/unittests/d3d12/CopySplitTests.cpp

@@ -37,6 +37,7 @@ namespace {
     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 @@ namespace {
             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 @@ namespace {
     }
 
     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;
     }
 
@@ -172,19 +174,22 @@ namespace {
         {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},
+        {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},
+        {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 @@ namespace {
         };
 
         return {
-             BufferSpec{alignNonPow2(0, textureSpec.texelSize), rowPitch},
+            BufferSpec{alignNonPow2(0, textureSpec.texelSize), rowPitch, textureSpec.height},
-             BufferSpec{alignNonPow2(512, textureSpec.texelSize), rowPitch},
+            BufferSpec{alignNonPow2(512, textureSpec.texelSize), rowPitch, textureSpec.height},
-             BufferSpec{alignNonPow2(1024, textureSpec.texelSize), rowPitch},
+            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(32, textureSpec.texelSize), rowPitch, textureSpec.height},
-             BufferSpec{alignNonPow2(64, textureSpec.texelSize), rowPitch},
+            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(31, textureSpec.texelSize), rowPitch, textureSpec.height},
-             BufferSpec{alignNonPow2(257, textureSpec.texelSize), rowPitch},
+            BufferSpec{alignNonPow2(257, textureSpec.texelSize), rowPitch, textureSpec.height},
-             BufferSpec{alignNonPow2(511, textureSpec.texelSize), rowPitch},
+            BufferSpec{alignNonPow2(511, textureSpec.texelSize), rowPitch, textureSpec.height},
-             BufferSpec{alignNonPow2(513, textureSpec.texelSize), rowPitch},
+            BufferSpec{alignNonPow2(513, textureSpec.texelSize), rowPitch, textureSpec.height},
-             BufferSpec{alignNonPow2(1023, textureSpec.texelSize), rowPitch},
+            BufferSpec{alignNonPow2(1023, textureSpec.texelSize), rowPitch, textureSpec.height},
+            BufferSpec{alignNonPow2(1023, textureSpec.texelSize), rowPitch, textureSpec.height * 2},
         };
     }
 
@@ -223,7 +231,7 @@ class CopySplitTest : public testing::Test {
             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, RowPitch) {
         }
     }
 }
+
+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();
+                }
+            }
+        }
+    }
+}

src/tests/unittests/validation/CopyCommandsValidationTests.cpp

@@ -340,6 +340,29 @@ TEST_F(CopyCommandTest_B2T, IncorrectRowPitch) {
                 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 @@ TEST_F(CopyCommandTest_T2B, IncorrectRowPitch) {
                 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);