Vulkan: Fix PBO read pixels with unaligned output pitch

Vulkan takes the output pitch in texels instead of bytes in VkBufferImageCopy::bufferRowLength. This means that it's impossible to use vkCmdCopyImageToBuffer when the pitch is not a multiple of texel size. This change makes sure the fallback path is taken. Bug: angleproject:5667 Change-Id: I3f2ef312bb8288de8ca3c6730d85a3c14858812e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2717006Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>

Vulkan: Fix PBO read pixels with unaligned output pitch
4a24786e · Shahbaz Youssefi · Commit Bot · 999da35e · 4a24786e
Commit 4a24786e authored Feb 23, 2021 by Shahbaz Youssefi Committed by Commit Bot Feb 24, 2021
Hide whitespace changes
Inline Side-by-side

Showing with 12 additions and 5 deletions

vk_helpers.cpp src/libANGLE/renderer/vulkan/vk_helpers.cpp +12 -5

No files found.
--- a/src/libANGLE/renderer/vulkan/vk_helpers.cpp
+++ b/src/libANGLE/renderer/vulkan/vk_helpers.cpp
@@ -683,10 +683,9 @@ bool CanCopyWithTransferForCopyImage(RendererVk *renderer,
 }

 bool CanCopyWithTransformForReadPixels(const PackPixelsParams &packPixelsParams,
-                                       const vk::Format *imageFormat)
+                                       const vk::Format *imageFormat,
+                                       const angle::Format *readFormat)
 {
-    const angle::Format *readFormat = &imageFormat->actualImageFormat();
-
    // Don't allow copies from emulated formats for simplicity.
    const bool isEmulatedFormat = imageFormat->hasEmulatedImageFormat();

@@ -697,7 +696,12 @@ bool CanCopyWithTransformForReadPixels(const PackPixelsParams &packPixelsParams,
    const bool needsTransformation =
        packPixelsParams.rotation != SurfaceRotation::Identity || packPixelsParams.reverseRowOrder;

-    return !isEmulatedFormat && isSameFormatCopy && !needsTransformation;
+    // Disallow copies when the output pitch cannot be correctly specified in Vulkan.
+    const bool isPitchMultipleOfTexelSize =
+        packPixelsParams.outputPitch % readFormat->pixelBytes == 0;
+
+    return !isEmulatedFormat && isSameFormatCopy && !needsTransformation &&
+           isPitchMultipleOfTexelSize;
 }

 void ReleaseBufferListToRenderer(RendererVk *renderer, BufferHelperPointerVector *buffers)
@@ -6354,7 +6358,8 @@ angle::Result ImageHelper::readPixels(ContextVk *contextVk,
    }

    // If PBO and if possible, copy directly on the GPU.
-    if (packPixelsParams.packBuffer && CanCopyWithTransformForReadPixels(packPixelsParams, mFormat))
+    if (packPixelsParams.packBuffer &&
+        CanCopyWithTransformForReadPixels(packPixelsParams, mFormat, readFormat))
    {
        BufferHelper &packBuffer = GetImpl(packPixelsParams.packBuffer)->getBuffer();

@@ -6365,6 +6370,8 @@ angle::Result ImageHelper::readPixels(ContextVk *contextVk,
        CommandBuffer *copyCommandBuffer;
        ANGLE_TRY(contextVk->getOutsideRenderPassCommandBuffer(copyAccess, &copyCommandBuffer));

+        ASSERT(packPixelsParams.outputPitch % readFormat->pixelBytes == 0);
+
        VkBufferImageCopy region = {};
        region.bufferImageHeight = srcExtent.height;
        region.bufferOffset      = packPixelsParams.offset;