Vulkan: Enhance ReadPixels to deal with pre-rotation

src/libANGLE/renderer/renderer_utils.cpp

@@ -205,11 +205,14 @@ void SetFloatUniformMatrixFast(unsigned int arrayElementOffset,
 
     memcpy(targetData, valueData, matrixSize * count);
 }
-
 }  // anonymous namespace
 
 PackPixelsParams::PackPixelsParams()
-    : destFormat(nullptr), outputPitch(0), packBuffer(nullptr), offset(0)
+    : destFormat(nullptr),
+      outputPitch(0),
+      packBuffer(nullptr),
+      offset(0),
+      orientation(SurfaceRotationType::Identity)
 {}
 
 PackPixelsParams::PackPixelsParams(const gl::Rectangle &areaIn,
@@ -223,7 +226,8 @@ PackPixelsParams::PackPixelsParams(const gl::Rectangle &areaIn,
       outputPitch(outputPitchIn),
       packBuffer(packBufferIn),
       reverseRowOrder(reverseRowOrderIn),
-      offset(offsetIn)
+      offset(offsetIn),
+      orientation(SurfaceRotationType::Identity)
 {}
 
 void PackPixels(const PackPixelsParams &params,
@@ -236,14 +240,73 @@ void PackPixels(const PackPixelsParams &params,
 
     const uint8_t *source = sourceIn;
     int inputPitch        = inputPitchIn;
-
-    if (params.reverseRowOrder)
-    {
-        source += inputPitch * (params.area.height - 1);
-        inputPitch = -inputPitch;
+    int destWidth         = params.area.width;
+    int destHeight        = params.area.height;
+    int xAxisPitch        = 0;
+    int yAxisPitch        = 0;
+    switch (params.orientation)
+    {
+        case SurfaceRotationType::Identity:
+            // The source image is not rotated (i.e. matches the device's orientation), and may or
+            // may not be y-flipped.  The image is row-major.  Each source row (one step along the
+            // y-axis for each step in the dest y-axis) is inputPitch past the previous row.  Along
+            // a row, each source pixel (one step along the x-axis for each step in the dest
+            // x-axis) is sourceFormat.pixelBytes past the previous pixel.
+            xAxisPitch = sourceFormat.pixelBytes;
+            if (params.reverseRowOrder)
+            {
+                // The source image is y-flipped, which means we start at the last row, and each
+                // source row is BEFORE the previous row.
+                source += inputPitchIn * (params.area.height - 1);
+                inputPitch = -inputPitch;
+                yAxisPitch = -inputPitchIn;
+            }
+            else
+            {
+                yAxisPitch = inputPitchIn;
+            }
+            break;
+        case SurfaceRotationType::Rotated90Degrees:
+            // The source image is rotated 90 degrees counter-clockwise.  Y-flip is always applied
+            // to rotated images.  The image is column-major.  Each source column (one step along
+            // the source x-axis for each step in the dest y-axis) is inputPitch past the previous
+            // column.  Along a column, each source pixel (one step along the y-axis for each step
+            // in the dest x-axis) is sourceFormat.pixelBytes past the previous pixel.
+            xAxisPitch = inputPitchIn;
+            yAxisPitch = sourceFormat.pixelBytes;
+            destWidth  = params.area.height;
+            destHeight = params.area.width;
+            break;
+        case SurfaceRotationType::Rotated180Degrees:
+            // The source image is rotated 180 degrees.  Y-flip is always applied to rotated
+            // images.  The image is row-major, but upside down.  Each source row (one step along
+            // the y-axis for each step in the dest y-axis) is inputPitch after the previous row.
+            // Along a row, each source pixel (one step along the x-axis for each step in the dest
+            // x-axis) is sourceFormat.pixelBytes BEFORE the previous pixel.
+            xAxisPitch = -static_cast<int>(sourceFormat.pixelBytes);
+            yAxisPitch = inputPitchIn;
+            source += sourceFormat.pixelBytes * (params.area.width - 1);
+            break;
+        case SurfaceRotationType::Rotated270Degrees:
+            // The source image is rotated 270 degrees counter-clockwise (or 90 degrees clockwise).
+            // Y-flip is always applied to rotated images.  The image is column-major, where each
+            // column (one step in the source x-axis for one step in the dest y-axis) is inputPitch
+            // BEFORE the previous column.  Along a column, each source pixel (one step along the
+            // y-axis for each step in the dest x-axis) is sourceFormat.pixelBytes BEFORE the
+            // previous pixel.  The first pixel is at the end of the source.
+            xAxisPitch = -inputPitchIn;
+            yAxisPitch = -static_cast<int>(sourceFormat.pixelBytes);
+            destWidth  = params.area.height;
+            destHeight = params.area.width;
+            source += inputPitch * (params.area.height - 1) +
+                      sourceFormat.pixelBytes * (params.area.width - 1);
+            break;
+        default:
+            UNREACHABLE();
+            break;
     }
 
-    if (sourceFormat == *params.destFormat)
+    if (params.orientation == SurfaceRotationType::Identity && sourceFormat == *params.destFormat)
     {
         // Direct copy possible
         for (int y = 0; y < params.area.height; ++y)
@@ -259,13 +322,13 @@ void PackPixels(const PackPixelsParams &params,
     if (fastCopyFunc)
     {
         // Fast copy is possible through some special function
-        for (int y = 0; y < params.area.height; ++y)
+        for (int y = 0; y < destHeight; ++y)
         {
-            for (int x = 0; x < params.area.width; ++x)
+            for (int x = 0; x < destWidth; ++x)
             {
                 uint8_t *dest =
                     destWithOffset + y * params.outputPitch + x * params.destFormat->pixelBytes;
-                const uint8_t *src = source + y * inputPitch + x * sourceFormat.pixelBytes;
+                const uint8_t *src = source + y * yAxisPitch + x * xAxisPitch;
 
                 fastCopyFunc(src, dest);
             }
@@ -286,13 +349,13 @@ void PackPixels(const PackPixelsParams &params,
     PixelReadFunction pixelReadFunction = sourceFormat.pixelReadFunction;
     ASSERT(pixelReadFunction != nullptr);
 
-    for (int y = 0; y < params.area.height; ++y)
+    for (int y = 0; y < destHeight; ++y)
     {
-        for (int x = 0; x < params.area.width; ++x)
+        for (int x = 0; x < destWidth; ++x)
         {
             uint8_t *dest =
                 destWithOffset + y * params.outputPitch + x * params.destFormat->pixelBytes;
-            const uint8_t *src = source + y * inputPitch + x * sourceFormat.pixelBytes;
+            const uint8_t *src = source + y * yAxisPitch + x * xAxisPitch;
 
             // readFunc and writeFunc will be using the same type of color, CopyTexImage
             // will not allow the copy otherwise.

src/libANGLE/renderer/renderer_utils.h

@@ -43,6 +43,23 @@ namespace rx
 {
 class ContextImpl;
 
+// The possible rotations of the surface/draw framebuffer, particularly for the Vulkan back-end on
+// Android.
+enum class SurfaceRotationType
+{
+    Identity,
+    Rotated90Degrees,
+    Rotated180Degrees,
+    Rotated270Degrees,
+    FlippedIdentity,
+    FlippedRotated90Degrees,
+    FlippedRotated180Degrees,
+    FlippedRotated270Degrees,
+
+    InvalidEnum,
+    EnumCount = InvalidEnum,
+};
+
 using MipGenerationFunction = void (*)(size_t sourceWidth,
                                        size_t sourceHeight,
                                        size_t sourceDepth,
@@ -94,6 +111,7 @@ struct PackPixelsParams
     gl::Buffer *packBuffer;
     bool reverseRowOrder;
     ptrdiff_t offset;
+    SurfaceRotationType orientation;
 };
 
 void PackPixels(const PackPixelsParams &params,

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -12,6 +12,7 @@
 
 #include "common/PackedEnums.h"
 #include "libANGLE/renderer/ContextImpl.h"
+#include "libANGLE/renderer/renderer_utils.h"
 #include "libANGLE/renderer/vulkan/OverlayVk.h"
 #include "libANGLE/renderer/vulkan/PersistentCommandPool.h"
 #include "libANGLE/renderer/vulkan/RendererVk.h"
@@ -29,23 +30,6 @@ class ProgramExecutableVk;
 class RendererVk;
 class WindowSurfaceVk;
 
-// The possible rotations of the surface/draw framebuffer, used for pre-rotating gl_Position
-// in the vertex shader.
-enum class SurfaceRotationType
-{
-    Identity,
-    Rotated90Degrees,
-    Rotated180Degrees,
-    Rotated270Degrees,
-    FlippedIdentity,
-    FlippedRotated90Degrees,
-    FlippedRotated180Degrees,
-    FlippedRotated270Degrees,
-
-    InvalidEnum,
-    EnumCount = InvalidEnum,
-};
-
 struct CommandBatch final : angle::NonCopyable
 {
     CommandBatch();

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -463,9 +463,42 @@ angle::Result FramebufferVk::readPixels(const gl::Context *context,
                                                    format, type, area, clippedArea, &params,
                                                    &outputSkipBytes));
 
-    if (contextVk->isViewportFlipEnabledForReadFBO())
+    bool flipY = contextVk->isViewportFlipEnabledForReadFBO();
+    switch (params.orientation = contextVk->getRotationReadFramebuffer())
+    {
+        case SurfaceRotationType::Identity:
+            // Do not rotate gl_Position (surface matches the device's orientation):
+            if (flipY)
+            {
+                params.area.y = fbRect.height - clippedArea.y - clippedArea.height;
+            }
+            break;
+        case SurfaceRotationType::Rotated90Degrees:
+            // Rotate gl_Position 90 degrees:
+            params.area.x = clippedArea.y;
+            params.area.y =
+                flipY ? clippedArea.x : fbRect.width - clippedArea.x - clippedArea.width;
+            std::swap(params.area.width, params.area.height);
+            break;
+        case SurfaceRotationType::Rotated180Degrees:
+            // Rotate gl_Position 180 degrees:
+            params.area.x = fbRect.width - clippedArea.x - clippedArea.width;
+            params.area.y =
+                flipY ? clippedArea.y : fbRect.height - clippedArea.y - clippedArea.height;
+            break;
+        case SurfaceRotationType::Rotated270Degrees:
+            // Rotate gl_Position 270 degrees:
+            params.area.x = fbRect.height - clippedArea.y - clippedArea.height;
+            params.area.y =
+                flipY ? fbRect.width - clippedArea.x - clippedArea.width : clippedArea.x;
+            std::swap(params.area.width, params.area.height);
+            break;
+        default:
+            UNREACHABLE();
+            break;
+    }
+    if (flipY)
     {
-        params.area.y          = fbRect.height - clippedArea.y - clippedArea.height;
         params.reverseRowOrder = !params.reverseRowOrder;
     }