Make SkBitmap::CopyTo respect requested dst color type when bitmap is texture backed.

BUG=chromium:550559
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1576983002

Review URL: https://codereview.chromium.org/1576983002

7 files changed, 104 insertions, 13 deletions

diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h
index 4591ed82bf..9cdcd85a79 100644
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -225,7 +225,8 @@ public:
         return this->onGetYUV8Planes(sizes, planes, rowBytes, colorSpace);
     }
 
-    bool readPixels(SkBitmap* dst, const SkIRect* subset = NULL);
+    /** Populates dst with the pixels of this pixelRef, converting them to colorType. */
+    bool readPixels(SkBitmap* dst, SkColorType colorType, const SkIRect* subset = NULL);
 
     /**
      *  Makes a deep copy of this PixelRef, respecting the requested config.
@@ -299,7 +300,7 @@ protected:
      *
      *  The base class implementation returns false;
      */
-    virtual bool onReadPixels(SkBitmap* dst, const SkIRect* subsetOrNull);
+    virtual bool onReadPixels(SkBitmap* dst, SkColorType colorType, const SkIRect* subsetOrNull);
 
     // default impl returns NULL.
     virtual SkData* onRefEncodedData();
diff --git a/include/gpu/SkGrPixelRef.h b/include/gpu/SkGrPixelRef.h
index c06154db10..b4dbd9d73f 100644
--- a/include/gpu/SkGrPixelRef.h
+++ b/include/gpu/SkGrPixelRef.h
@@ -49,7 +49,7 @@ public:
 
 protected:
     // overrides from SkPixelRef
-    bool onReadPixels(SkBitmap* dst, const SkIRect* subset) override;
+    bool onReadPixels(SkBitmap* dst, SkColorType, const SkIRect* subset) override;
     SkPixelRef* deepCopy(SkColorType, SkColorProfileType,
                          const SkIRect* subset) override;
     void onNotifyPixelsChanged() override;
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index c66a5b2fd5..bdf1daafcc 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -828,7 +828,7 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc)
         SkIRect subset;
         subset.setXYWH(fPixelRefOrigin.fX, fPixelRefOrigin.fY,
                        fInfo.width(), fInfo.height());
-        if (fPixelRef->readPixels(&tmpSrc, &subset)) {
+        if (fPixelRef->readPixels(&tmpSrc, dstColorType, &subset)) {
             if (fPixelRef->info().alphaType() == kUnpremul_SkAlphaType) {
                 // FIXME: The only meaningful implementation of readPixels
                 // (GrPixelRef) assumes premultiplied pixels.
diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index 1b9130ac75..a7ca7b7181 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -286,7 +286,7 @@ void SkBitmapDevice::drawBitmapRect(const SkDraw& draw, const SkBitmap& bitmap,
         if(bitmap.pixelRef()->getTexture()) {
             // Accelerated source canvas, don't use extractSubset but readPixels to get the subset.
             // This way, the pixels are copied in CPU memory instead of GPU memory.
-            bitmap.pixelRef()->readPixels(&tmpBitmap, &srcIR);
+            bitmap.pixelRef()->readPixels(&tmpBitmap, kN32_SkColorType, &srcIR);
         } else {
             if (!bitmap.extractSubset(&tmpBitmap, srcIR)) {
                 return;
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index d7117617f6..0825760ccd 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -307,13 +307,13 @@ void SkPixelRef::restoreMutability() {
     fMutability = kMutable;
 }
 
-bool SkPixelRef::readPixels(SkBitmap* dst, const SkIRect* subset) {
-    return this->onReadPixels(dst, subset);
+bool SkPixelRef::readPixels(SkBitmap* dst, SkColorType ct, const SkIRect* subset) {
+    return this->onReadPixels(dst, ct, subset);
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-bool SkPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
+bool SkPixelRef::onReadPixels(SkBitmap* dst, SkColorType, const SkIRect* subset) {
     return false;
 }
 
diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp
index 6e014feded..58f516a19c 100644
--- a/src/gpu/SkGrPixelRef.cpp
+++ b/src/gpu/SkGrPixelRef.cpp
@@ -25,7 +25,7 @@ SkROLockPixelsPixelRef::~SkROLockPixelsPixelRef() {}
 bool SkROLockPixelsPixelRef::onNewLockPixels(LockRec* rec) {
     fBitmap.reset();
 //    SkDebugf("---------- calling readpixels in support of lockpixels\n");
-    if (!this->onReadPixels(&fBitmap, nullptr)) {
+    if (!this->onReadPixels(&fBitmap, this->info().colorType(), nullptr)) {
         SkDebugf("SkROLockPixelsPixelRef::onLockPixels failed!\n");
         return false;
     }
@@ -155,11 +155,20 @@ static bool tryAllocBitmapPixels(SkBitmap* bitmap) {
     }
 }
 
-bool SkGrPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
+bool SkGrPixelRef::onReadPixels(SkBitmap* dst, SkColorType colorType, const SkIRect* subset) {
     if (nullptr == fSurface || fSurface->wasDestroyed()) {
         return false;
     }
 
+    GrPixelConfig config;
+    if (kRGBA_8888_SkColorType == colorType) {
+        config = kRGBA_8888_GrPixelConfig;
+    } else if (kBGRA_8888_SkColorType == colorType) {
+        config = kBGRA_8888_GrPixelConfig;
+    } else {
+        return false;
+    }
+
     SkIRect bounds;
     if (subset) {
         bounds = *subset;
@@ -172,7 +181,9 @@ bool SkGrPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
         //Cache miss
 
         SkBitmap cachedBitmap;
-        cachedBitmap.setInfo(this->info().makeWH(bounds.width(), bounds.height()));
+        cachedBitmap.setInfo(SkImageInfo::Make(bounds.width(), bounds.height(), colorType,
+                                               this->info().alphaType(),
+                                               this->info().profileType()));
 
         // If we can't alloc the pixels, then fail
         if (!tryAllocBitmapPixels(&cachedBitmap)) {
@@ -183,8 +194,7 @@ bool SkGrPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
         void* buffer = cachedBitmap.getPixels();
         bool readPixelsOk = fSurface->readPixels(bounds.fLeft, bounds.fTop,
                                 bounds.width(), bounds.height(),
-                                kSkia8888_GrPixelConfig,
-                                buffer, cachedBitmap.rowBytes());
+                                config, buffer, cachedBitmap.rowBytes());
 
         if (!readPixelsOk) {
             return false;
diff --git a/tests/BitmapCopyTest.cpp b/tests/BitmapCopyTest.cpp
index 6db63fae45..8b2cb3ce54 100644
--- a/tests/BitmapCopyTest.cpp
+++ b/tests/BitmapCopyTest.cpp
@@ -632,3 +632,83 @@ DEF_TEST(BitmapReadPixels, reporter) {
     }
 }
 
+#if SK_SUPPORT_GPU
+
+#include "GrContext.h"
+#include "SkGr.h"
+#include "SkColorPriv.h"
+/** Tests calling copyTo on a texture backed bitmap. Tests that all BGRA_8888/RGBA_8888 combinations
+    of src and dst work. This test should be removed when SkGrPixelRef is removed. */
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(BitmapCopy_Texture, reporter, ctx) {
+    static const SkPMColor kData[] = {
+        0xFF112233, 0xAF224499,
+        0xEF004466, 0x80773311
+    };
+
+    uint32_t swizData[SK_ARRAY_COUNT(kData)];
+    for (size_t i = 0; i < SK_ARRAY_COUNT(kData); ++i) {
+        swizData[i] = SkSwizzle_RB(kData[i]);
+    }
+
+    static const GrPixelConfig kSrcConfigs[] = {
+        kRGBA_8888_GrPixelConfig,
+        kBGRA_8888_GrPixelConfig,
+    };
+
+    for (size_t srcC = 0; srcC < SK_ARRAY_COUNT(kSrcConfigs); ++srcC) {
+        for (int rt = 0; rt < 2; ++rt) {
+            GrSurfaceDesc desc;
+            desc.fConfig = kSrcConfigs[srcC];
+            desc.fFlags = rt ? kRenderTarget_GrSurfaceFlag : kNone_GrSurfaceFlags;
+            desc.fWidth = 2;
+            desc.fHeight = 2;
+            desc.fOrigin = kTopLeft_GrSurfaceOrigin;
+
+            const void* srcData = (kSkia8888_GrPixelConfig == desc.fConfig) ? kData : swizData;
+
+            SkAutoTUnref<GrTexture> texture(
+                ctx->textureProvider()->createTexture(desc, false, srcData, 0));
+
+            SkBitmap srcBmp;
+            GrWrapTextureInBitmap(texture, 2, 2, false, &srcBmp);
+            if (srcBmp.isNull()) {
+                ERRORF(reporter, "Could not wrap texture in bitmap.");
+                continue;
+            }
+            static const SkColorType kDstCTs[] = { kRGBA_8888_SkColorType, kBGRA_8888_SkColorType };
+            for (size_t dCT = 0; dCT < SK_ARRAY_COUNT(kDstCTs); ++dCT) {
+                SkBitmap dstBmp;
+                if (!srcBmp.copyTo(&dstBmp, kDstCTs[dCT])) {
+                    ERRORF(reporter, "CopyTo failed.");
+                }
+                if (dstBmp.colorType() != kDstCTs[dCT]) {
+                    ERRORF(reporter, "SkBitmap::CopyTo did not respect passed in color type.");
+                }
+                SkAutoLockPixels alp(dstBmp);
+                uint8_t* dstBmpPixels = static_cast<uint8_t*>(dstBmp.getPixels());
+                const uint32_t* refData;
+#if defined(SK_PMCOLOR_IS_RGBA)
+                refData = (kRGBA_8888_SkColorType == dstBmp.colorType()) ? kData : swizData;
+#elif defined(SK_PMCOLOR_IS_BGRA)
+                refData = (kBGRA_8888_SkColorType == dstBmp.colorType()) ? kData : swizData;
+#else 
+    #error "PM Color must be BGRA or RGBA to use GPU backend."
+#endif
+                bool foundError = false;
+                for (int y = 0; y < 2 && !foundError; ++y) {
+                    uint32_t* dstBmpRow = reinterpret_cast<uint32_t*>(dstBmpPixels);
+                    for (int x = 0; x < 2 && !foundError; ++x) {
+                        if (refData[2 * y + x] != dstBmpRow[x]) {
+                            ERRORF(reporter, "Expected pixel 0x%08x, found 0x%08x.",
+                                   refData[2 * y + x], dstBmpRow[x]);
+                            foundError = true;
+                        }
+                    }
+                    dstBmpPixels += dstBmp.rowBytes();
+                }
+            }
+        }
+    }
+}
+
+#endif