Fix Vulkan readPixels

Fixed bug with setting up tempReadTexture from previous CL.

Also, previously we were not correctly handling the rowBytes during the read.

BUG=skia:5461
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2102633003

Review-Url: https://codereview.chromium.org/2102633003

2 files changed, 42 insertions, 27 deletions

diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index bfda30a033..ca6ccde67a 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -414,8 +414,8 @@ bool GrVkGpu::uploadTexDataLinear(GrVkTexture* tex,
         if (trimRowBytes == rowBytes && trimRowBytes == layout.rowPitch) {
             memcpy(mapPtr, data, trimRowBytes * height);
         } else {
-            SkRectMemcpy(mapPtr, static_cast<size_t>(layout.rowPitch), data, rowBytes,
-                            trimRowBytes, height);
+            SkRectMemcpy(mapPtr, static_cast<size_t>(layout.rowPitch), data, rowBytes, trimRowBytes,
+                         height);
         }
     }
 
@@ -1356,6 +1356,24 @@ void GrVkGpu::onGetMultisampleSpecs(GrRenderTarget* rt, const GrStencilSettings&
 bool GrVkGpu::onGetReadPixelsInfo(GrSurface* srcSurface, int width, int height, size_t rowBytes,
                                   GrPixelConfig readConfig, DrawPreference* drawPreference,
                                   ReadPixelTempDrawInfo* tempDrawInfo) {
+    // These settings we will always want if a temp draw is performed.
+    tempDrawInfo->fTempSurfaceDesc.fFlags = kRenderTarget_GrSurfaceFlag;
+    tempDrawInfo->fTempSurfaceDesc.fWidth = width;
+    tempDrawInfo->fTempSurfaceDesc.fHeight = height;
+    tempDrawInfo->fTempSurfaceDesc.fSampleCnt = 0;
+    tempDrawInfo->fTempSurfaceDesc.fOrigin = kTopLeft_GrSurfaceOrigin; // no CPU y-flip for TL.
+    tempDrawInfo->fUseExactScratch = false;
+
+    // For now assume no swizzling, we may change that below.
+    tempDrawInfo->fSwizzle = GrSwizzle::RGBA();
+
+    // Depends on why we need/want a temp draw. Start off assuming no change, the surface we read
+    // from will be srcConfig and we will read readConfig pixels from it.
+    // Not that if we require a draw and return a non-renderable format for the temp surface the
+    // base class will fail for us.
+    tempDrawInfo->fTempSurfaceDesc.fConfig = srcSurface->config();
+    tempDrawInfo->fReadConfig = readConfig;
+
     if (srcSurface->config() == readConfig) {
         return true;
     }
@@ -1408,7 +1426,7 @@ bool GrVkGpu::onReadPixels(GrSurface* surface,
     VkBufferImageCopy region;
     memset(&region, 0, sizeof(VkBufferImageCopy));
     region.bufferOffset = 0;
-    region.bufferRowLength = 0; // Forces RowLength to be imageExtent.width
+    region.bufferRowLength = 0; // Forces RowLength to be width. We handle the rowBytes below.
     region.bufferImageHeight = 0; // Forces height to be tightly packed. Only useful for 3d images.
     region.imageSubresource = { VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1 };
     region.imageOffset = offset;
@@ -1435,29 +1453,26 @@ bool GrVkGpu::onReadPixels(GrSurface* surface,
 
     void* mappedMemory = transferBuffer->map();
 
-    memcpy(buffer, mappedMemory, rowBytes*height);
-
-    transferBuffer->unmap();
-    transferBuffer->unref();
-
+    size_t tightRowBytes = GrBytesPerPixel(config) * width;
     if (flipY) {
-        SkAutoSMalloc<32 * sizeof(GrColor)> scratch;
-        size_t tightRowBytes = GrBytesPerPixel(config) * width;
-        scratch.reset(tightRowBytes);
-        void* tmpRow = scratch.get();
-        // flip y in-place by rows
-        const int halfY = height >> 1;
-        char* top = reinterpret_cast<char*>(buffer);
-        char* bottom = top + (height - 1) * rowBytes;
-        for (int y = 0; y < halfY; y++) {
-            memcpy(tmpRow, top, tightRowBytes);
-            memcpy(top, bottom, tightRowBytes);
-            memcpy(bottom, tmpRow, tightRowBytes);
-            top += rowBytes;
-            bottom -= rowBytes;
+        const char* srcRow = reinterpret_cast<const char*>(mappedMemory);
+        char* dstRow = reinterpret_cast<char*>(buffer)+(height - 1) * rowBytes;
+        for (int y = 0; y < height; y++) {
+            memcpy(dstRow, srcRow, tightRowBytes);
+            srcRow += tightRowBytes;
+            dstRow -= rowBytes;
+        }
+    } else {
+        if (tightRowBytes == rowBytes) {
+            memcpy(buffer, mappedMemory, rowBytes*height);
+        } else {
+            SkRectMemcpy(buffer, rowBytes, mappedMemory, tightRowBytes, tightRowBytes, height);
         }
     }
 
+    transferBuffer->unmap();
+    transferBuffer->unref();
+
     return true;
 }
 
diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp
index a4dbae9eb7..20305cc3eb 100644
--- a/tests/ReadPixelsTest.cpp
+++ b/tests/ReadPixelsTest.cpp
@@ -200,8 +200,8 @@ static bool check_read(skiatest::Reporter* reporter,
                     bool didPremul;
                     SkPMColor pmPixel = convert_to_pmcolor(ct, at, pixel, &didPremul);
                     if (!check_read_pixel(pmPixel, canvasPixel, didPremul)) {
-                        ERRORF(reporter, "Expected readback pixel value 0x%08x, got 0x%08x. "
-                               "Readback was unpremul: %d", canvasPixel, pmPixel, didPremul);
+                        ERRORF(reporter, "Expected readback pixel (%d, %d) value 0x%08x, got 0x%08x. "
+                               "Readback was unpremul: %d", bx, by, canvasPixel, pmPixel, didPremul);
                         return false;
                     }
                 }
@@ -387,7 +387,7 @@ DEF_TEST(ReadPixels, reporter) {
     test_readpixels(reporter, surface, kLastAligned_BitmapInit);
 }
 #if SK_SUPPORT_GPU
-DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ReadPixels_Gpu, reporter, ctxInfo) {
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Gpu, reporter, ctxInfo) {
     for (auto& origin : {kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin}) {
         GrSurfaceDesc desc;
         desc.fFlags = kRenderTarget_GrSurfaceFlag;
@@ -442,7 +442,7 @@ static void test_readpixels_texture(skiatest::Reporter* reporter, GrTexture* tex
         }
     }
 }
-DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ReadPixels_Texture, reporter, ctxInfo) {
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Texture, reporter, ctxInfo) {
     // On the GPU we will also try reading back from a non-renderable texture.
     for (auto& origin : {kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin}) {
         SkAutoTUnref<GrTexture> texture;
@@ -577,7 +577,7 @@ static void dump_to_file(const char name[], SkData* data) {
  *
  *  https://bug.skia.org/4351
  */
-DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ReadPixels_Subset_Gpu, reporter, ctxInfo) {
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Subset_Gpu, reporter, ctxInfo) {
     SkBitmap bitmap;
     make_ringed_bitmap(&bitmap, 6, 6);
     const SkIRect subset = SkIRect::MakeLTRB(2, 2, 4, 4);