Make raster pipeline support all pixel conversions

BUG=skia:

CQ_INCLUDE_TRYBOTS=skia.primary:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD

Change-Id: Idc76999d0f5591a567b3976cb9db829c350e4be2
Reviewed-on: https://skia-review.googlesource.com/8304
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Matt Sarett <msarett@google.com>

9 files changed, 289 insertions, 166 deletions

diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index f54488d0a5..33575499e6 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -726,8 +726,9 @@ bool SkBitmap::writePixels(const SkPixmap& src, int dstX, int dstY) {
 
     void* dstPixels = this->getAddr(rec.fX, rec.fY);
     const SkImageInfo dstInfo = fInfo.makeWH(rec.fInfo.width(), rec.fInfo.height());
-    return SkPixelInfo::CopyPixels(dstInfo, dstPixels, this->rowBytes(),
-                                   rec.fInfo, rec.fPixels, rec.fRowBytes, src.ctable());
+    SkPixelInfo::CopyPixels(dstInfo, dstPixels, this->rowBytes(), rec.fInfo, rec.fPixels,
+                            rec.fRowBytes, src.ctable());
+    return true;
 }
 
 bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc) const {
diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp
index c1cd16aeb6..616ba192a7 100644
--- a/src/core/SkConfig8888.cpp
+++ b/src/core/SkConfig8888.cpp
@@ -25,6 +25,10 @@ static inline bool can_memcpy(const SkImageInfo& dstInfo, const SkImageInfo& src
         return false;
     }
 
+    if (kAlpha_8_SkColorType == dstInfo.colorType()) {
+        return true;
+    }
+
     if (dstInfo.alphaType() != srcInfo.alphaType() &&
         kOpaque_SkAlphaType != dstInfo.alphaType() &&
         kOpaque_SkAlphaType != srcInfo.alphaType())
@@ -85,7 +89,7 @@ void swizzle_and_multiply(const SkImageInfo& dstInfo, void* dstPixels, size_t ds
 }
 
 // Default: Use the pipeline.
-static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_t dstRB,
+static void copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_t dstRB,
                                  const SkImageInfo& srcInfo, const void* srcRow, size_t srcRB,
                                  bool isColorAware) {
     SkRasterPipeline pipeline;
@@ -106,8 +110,12 @@ static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_
         case kGray_8_SkColorType:
             pipeline.append(SkRasterPipeline::load_g8, &srcRow);
             break;
+        case kARGB_4444_SkColorType:
+            pipeline.append(SkRasterPipeline::load_4444, &srcRow);
+            break;
         default:
-            return false;
+            SkASSERT(false);
+            break;
     }
 
     if (isColorAware && srcInfo.gammaCloseToSRGB()) {
@@ -146,8 +154,15 @@ static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_
         case kRGBA_F16_SkColorType:
             pipeline.append(SkRasterPipeline::store_f16, &dstRow);
             break;
+        case kAlpha_8_SkColorType:
+            pipeline.append(SkRasterPipeline::store_a8, &dstRow);
+            break;
+        case kARGB_4444_SkColorType:
+            pipeline.append(SkRasterPipeline::store_4444, &dstRow);
+            break;
         default:
-            return false;
+            SkASSERT(false);
+            break;
     }
 
     auto p = pipeline.compile();
@@ -159,67 +174,9 @@ static bool copy_pipeline_pixels(const SkImageInfo& dstInfo, void* dstRow, size_
         dstRow = SkTAddOffset<void>(dstRow, dstRB);
         srcRow = SkTAddOffset<const void>(srcRow, srcRB);
     }
-    return true;
-}
-
-static bool extract_alpha(void* dst, size_t dstRB, const void* src, size_t srcRB,
-                          const SkImageInfo& srcInfo, SkColorTable* ctable) {
-    uint8_t* SK_RESTRICT dst8 = (uint8_t*)dst;
-
-    const int w = srcInfo.width();
-    const int h = srcInfo.height();
-    if (srcInfo.isOpaque()) {
-        // src is opaque, so just fill alpha with 0xFF
-        for (int y = 0; y < h; ++y) {
-           memset(dst8, 0xFF, w);
-           dst8 += dstRB;
-        }
-        return true;
-    }
-    switch (srcInfo.colorType()) {
-        case kN32_SkColorType: {
-            const SkPMColor* SK_RESTRICT src32 = (const SkPMColor*)src;
-            for (int y = 0; y < h; ++y) {
-                for (int x = 0; x < w; ++x) {
-                    dst8[x] = SkGetPackedA32(src32[x]);
-                }
-                dst8 += dstRB;
-                src32 = (const SkPMColor*)((const char*)src32 + srcRB);
-            }
-            break;
-        }
-        case kARGB_4444_SkColorType: {
-            const SkPMColor16* SK_RESTRICT src16 = (const SkPMColor16*)src;
-            for (int y = 0; y < h; ++y) {
-                for (int x = 0; x < w; ++x) {
-                    dst8[x] = SkPacked4444ToA32(src16[x]);
-                }
-                dst8 += dstRB;
-                src16 = (const SkPMColor16*)((const char*)src16 + srcRB);
-            }
-            break;
-        }
-        case kIndex_8_SkColorType: {
-            if (nullptr == ctable) {
-                return false;
-            }
-            const SkPMColor* SK_RESTRICT table = ctable->readColors();
-            const uint8_t* SK_RESTRICT src8 = (const uint8_t*)src;
-            for (int y = 0; y < h; ++y) {
-                for (int x = 0; x < w; ++x) {
-                    dst8[x] = SkGetPackedA32(table[src8[x]]);
-                }
-                dst8 += dstRB;
-                src8 += srcRB;
-            }
-            break;
-        }
-        default:
-            return false;
-    }
-    return true;
 }
 
+// Fast Path 3: Color space xform.
 static inline bool optimized_color_xform(const SkImageInfo& dstInfo, const SkImageInfo& srcInfo) {
     if (kUnpremul_SkAlphaType == dstInfo.alphaType() && kPremul_SkAlphaType == srcInfo.alphaType())
     {
@@ -287,19 +244,61 @@ static inline void apply_color_xform(const SkImageInfo& dstInfo, void* dstPixels
     }
 }
 
-bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
+// Fast Path 4: Index 8 sources.
+template <typename T>
+void do_index8(const SkImageInfo& dstInfo, T* dstPixels, size_t dstRB,
+               const SkImageInfo& srcInfo, const uint8_t* srcPixels, size_t srcRB,
+               SkColorTable* ctable) {
+    T dstCTable[256];
+    int count = ctable->count();
+    SkImageInfo srcInfo8888 = srcInfo.makeColorType(kN32_SkColorType).makeWH(count, 1);
+    SkImageInfo dstInfoCT = dstInfo.makeWH(count, 1);
+    size_t rowBytes = count * sizeof(T);
+    SkPixelInfo::CopyPixels(dstInfoCT, dstCTable, rowBytes, srcInfo8888, ctable->readColors(),
+                            rowBytes, nullptr);
+
+    for (int y = 0; y < dstInfo.height(); y++) {
+        for (int x = 0; x < dstInfo.width(); x++) {
+            dstPixels[x] = dstCTable[srcPixels[x]];
+        }
+        dstPixels = SkTAddOffset<T>(dstPixels, dstRB);
+        srcPixels = SkTAddOffset<const uint8_t>(srcPixels, srcRB);
+    }
+}
+
+void xform_from_index8(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
+                       const SkImageInfo& srcInfo, const uint8_t* srcPixels, size_t srcRB,
+                       SkColorTable* ctable) {
+    switch (dstInfo.colorType()) {
+        case kAlpha_8_SkColorType:
+            do_index8(dstInfo, (uint8_t*) dstPixels, dstRB, srcInfo, srcPixels, srcRB, ctable);
+            break;
+        case kRGB_565_SkColorType:
+        case kARGB_4444_SkColorType:
+            do_index8(dstInfo, (uint16_t*) dstPixels, dstRB, srcInfo, srcPixels, srcRB, ctable);
+            break;
+        case kRGBA_8888_SkColorType:
+        case kBGRA_8888_SkColorType:
+            do_index8(dstInfo, (uint32_t*) dstPixels, dstRB, srcInfo, srcPixels, srcRB, ctable);
+            break;
+        case kRGBA_F16_SkColorType:
+            do_index8(dstInfo, (uint64_t*) dstPixels, dstRB, srcInfo, srcPixels, srcRB, ctable);
+            break;
+        default:
+            SkASSERT(false);
+    }
+}
+
+void SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
                              const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRB,
                              SkColorTable* ctable) {
     SkASSERT(dstInfo.dimensions() == srcInfo.dimensions());
     SkASSERT(SkImageInfoValidConversion(dstInfo, srcInfo));
 
-    const int width = srcInfo.width();
-    const int height = srcInfo.height();
-
     // Fast Path 1: The memcpy() case.
     if (can_memcpy(dstInfo, srcInfo)) {
         SkRectMemcpy(dstPixels, dstRB, srcPixels, srcRB, dstInfo.minRowBytes(), dstInfo.height());
-        return true;
+        return;
     }
 
     const bool isColorAware = dstInfo.colorSpace();
@@ -308,88 +307,23 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t
     // Fast Path 2: Simple swizzles and premuls.
     if (4 == srcInfo.bytesPerPixel() && 4 == dstInfo.bytesPerPixel() && !isColorAware) {
         swizzle_and_multiply(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, srcRB);
-        return true;
+        return;
     }
 
+    // Fast Path 3: Color space xform.
     if (isColorAware && optimized_color_xform(dstInfo, srcInfo)) {
         apply_color_xform(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, srcRB);
-        return true;
-    }
-
-    /*
-     *  Begin section where we try to change colorTypes along the way. Not all combinations
-     *  are supported.
-     */
-
-    if (kAlpha_8_SkColorType == dstInfo.colorType() &&
-        extract_alpha(dstPixels, dstRB, srcPixels, srcRB, srcInfo, ctable)) {
-        return true;
+        return;
     }
 
-    //  Try the pipeline
-    //
-    if (copy_pipeline_pixels(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, srcRB, isColorAware)) {
-        return true;
+    // Fast Path 4: Index 8 sources.
+    if (kIndex_8_SkColorType == srcInfo.colorType()) {
+        SkASSERT(ctable);
+        xform_from_index8(dstInfo, dstPixels, dstRB, srcInfo, (const uint8_t*) srcPixels, srcRB,
+                          ctable);
+        return;
     }
 
-    // Can no longer draw directly into 4444, but we can manually whack it for a few combinations
-    if (kARGB_4444_SkColorType == dstInfo.colorType() &&
-        (kN32_SkColorType == srcInfo.colorType() || kIndex_8_SkColorType == srcInfo.colorType())) {
-        if (srcInfo.alphaType() == kUnpremul_SkAlphaType) {
-            // Our method for converting to 4444 assumes premultiplied.
-            return false;
-        }
-
-        const SkPMColor* table = nullptr;
-        if (kIndex_8_SkColorType == srcInfo.colorType()) {
-            SkASSERT(ctable);
-            table = ctable->readColors();
-        }
-
-        for (int y = 0; y < height; ++y) {
-            DITHER_4444_SCAN(y);
-            SkPMColor16* SK_RESTRICT dstRow = (SkPMColor16*)dstPixels;
-            if (table) {
-                const uint8_t* SK_RESTRICT srcRow = (const uint8_t*)srcPixels;
-                for (int x = 0; x < width; ++x) {
-                    dstRow[x] = SkDitherARGB32To4444(table[srcRow[x]], DITHER_VALUE(x));
-                }
-            } else {
-                const SkPMColor* SK_RESTRICT srcRow = (const SkPMColor*)srcPixels;
-                for (int x = 0; x < width; ++x) {
-                    dstRow[x] = SkDitherARGB32To4444(srcRow[x], DITHER_VALUE(x));
-                }
-            }
-            dstPixels = (char*)dstPixels + dstRB;
-            srcPixels = (const char*)srcPixels + srcRB;
-        }
-        return true;
-    }
-
-    if (dstInfo.alphaType() == kUnpremul_SkAlphaType) {
-        // We do not support drawing to unpremultiplied bitmaps.
-        return false;
-    }
-
-    // Final fall-back, draw with a canvas
-    //
-    // Always clear the dest in case one of the blitters accesses it
-    // TODO: switch the allocation of tmpDst to call sk_calloc_throw
-    {
-        SkBitmap bm;
-        if (!bm.installPixels(srcInfo, const_cast<void*>(srcPixels), srcRB, ctable, nullptr, nullptr)) {
-            return false;
-        }
-        std::unique_ptr<SkCanvas> canvas = SkCanvas::MakeRasterDirect(dstInfo, dstPixels, dstRB);
-        if (!canvas) {
-            return false;
-        }
-
-        SkPaint  paint;
-        paint.setDither(true);
-
-        canvas->clear(0);
-        canvas->drawBitmap(bm, 0, 0, &paint);
-        return true;
-    }
+    // Default: Use the pipeline.
+    copy_pipeline_pixels(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, srcRB, isColorAware);
 }
diff --git a/src/core/SkConfig8888.h b/src/core/SkConfig8888.h
index 9c1ff4ab07..ae42242ad5 100644
--- a/src/core/SkConfig8888.h
+++ b/src/core/SkConfig8888.h
@@ -13,14 +13,10 @@
 
 class SkColorTable;
 
-struct SkPixelInfo {
-    SkColorType fColorType;
-    SkAlphaType fAlphaType;
-    size_t      fRowBytes;
-
-    static bool CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
-                           const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes,
-                           SkColorTable* srcCTable = nullptr);
+namespace SkPixelInfo {
+    void CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
+                    const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes,
+                    SkColorTable* srcCTable = nullptr);
 };
 
 static inline void SkRectMemcpy(void* dst, size_t dstRB, const void* src, size_t srcRB,
diff --git a/src/core/SkImageInfoPriv.h b/src/core/SkImageInfoPriv.h
index 40209f8946..855e506bb2 100644
--- a/src/core/SkImageInfoPriv.h
+++ b/src/core/SkImageInfoPriv.h
@@ -19,6 +19,11 @@ static inline bool SkImageInfoIsValid(const SkImageInfo& info) {
         return false;
     }
 
+    const int kMaxDimension = SK_MaxS32 >> 2;
+    if (info.width() > kMaxDimension || info.height() > kMaxDimension) {
+        return false;
+    }
+
     if (kUnknown_SkColorType == info.colorType() || kUnknown_SkAlphaType == info.alphaType()) {
         return false;
     }
@@ -44,8 +49,8 @@ static inline bool SkImageInfoIsValid(const SkImageInfo& info) {
 /**
  *  Returns true if Skia has defined a pixel conversion from the |src| to the |dst|.
  *  Returns false otherwise.  Some discussion of false cases:
- *      We will not convert to kIndex8 when the |src| is not kIndex8 in the same color space
- *      (color tables are immutable).
+ *      We will not convert to kIndex8 unless it exactly matches the src, since color tables
+ *      are immutable.
  *      We do not convert to kGray8 when the |src| is not kGray8 in the same color space.
  *      We may add this feature - it just requires some work to convert to luminance while
  *      handling color spaces correctly.  Currently no one is asking for this.
@@ -64,16 +69,30 @@ static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkIm
         return false;
     }
 
-    if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType() &&
-        dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace()))
-    {
-        return false;
+    if (kIndex_8_SkColorType == dst.colorType()) {
+        if (kIndex_8_SkColorType != src.colorType()) {
+            return false;
+        }
+
+        if ((kPremul_SkAlphaType == dst.alphaType() && kUnpremul_SkAlphaType == src.alphaType()) ||
+            (kUnpremul_SkAlphaType == dst.alphaType() && kPremul_SkAlphaType == src.alphaType()))
+        {
+            return false;
+        }
+
+        if (dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace())) {
+            return false;
+        }
     }
 
-    if (kGray_8_SkColorType == dst.colorType() && kGray_8_SkColorType != src.colorType() &&
-        dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace()))
-    {
-        return false;
+    if (kGray_8_SkColorType == dst.colorType()) {
+        if (kGray_8_SkColorType != src.colorType()) {
+            return false;
+        }
+
+        if (dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace())) {
+            return false;
+        }
     }
 
     if (kAlpha_8_SkColorType != dst.colorType() && kAlpha_8_SkColorType == src.colorType()) {
diff --git a/src/core/SkPixmap.cpp b/src/core/SkPixmap.cpp
index c0889cfd84..8b7171f96f 100644
--- a/src/core/SkPixmap.cpp
+++ b/src/core/SkPixmap.cpp
@@ -97,8 +97,9 @@ const {
 
     const void* srcPixels = this->addr(rec.fX, rec.fY);
     const SkImageInfo srcInfo = fInfo.makeWH(rec.fInfo.width(), rec.fInfo.height());
-    return SkPixelInfo::CopyPixels(rec.fInfo, rec.fPixels, rec.fRowBytes,
-                                   srcInfo, srcPixels, this->rowBytes(), this->ctable());
+    SkPixelInfo::CopyPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, srcInfo, srcPixels,
+                            this->rowBytes(), this->ctable());
+    return true;
 }
 
 static uint16_t pack_8888_to_4444(unsigned a, unsigned r, unsigned g, unsigned b) {
diff --git a/src/core/SkRasterPipeline.h b/src/core/SkRasterPipeline.h
index e507b28e7c..9afe2fdd29 100644
--- a/src/core/SkRasterPipeline.h
+++ b/src/core/SkRasterPipeline.h
@@ -68,6 +68,7 @@
     M(load_a8)   M(store_a8)                                     \
     M(load_g8)                                                   \
     M(load_565)  M(store_565)                                    \
+    M(load_4444) M(store_4444)                                   \
     M(load_f16)  M(store_f16)                                    \
     M(load_8888) M(store_8888)                                   \
     M(load_u16_be) M(load_rgb_u16_be) M(store_u16_be)            \
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index 0364fc6b2a..01b92a7dac 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -293,7 +293,9 @@ sk_sp<SkImage> SkImage::MakeRasterData(const SkImageInfo& info, sk_sp<SkData> da
 sk_sp<SkImage> SkImage::MakeFromRaster(const SkPixmap& pmap, RasterReleaseProc proc,
                                        ReleaseContext ctx) {
     size_t size;
-    if (!SkImage_Raster::ValidArgs(pmap.info(), pmap.rowBytes(), false, &size) || !pmap.addr()) {
+    if (!SkImage_Raster::ValidArgs(pmap.info(), pmap.rowBytes(), pmap.ctable(), &size) ||
+        !pmap.addr())
+    {
         return nullptr;
     }
 
diff --git a/src/opts/SkRasterPipeline_opts.h b/src/opts/SkRasterPipeline_opts.h
index 4f01fd5b70..a81516877a 100644
--- a/src/opts/SkRasterPipeline_opts.h
+++ b/src/opts/SkRasterPipeline_opts.h
@@ -538,6 +538,17 @@ STAGE_CTX(store_565, uint16_t**) {
                                    | SkNf_round(b, SK_B16_MASK) << SK_B16_SHIFT), ptr);
 }
 
+STAGE_CTX(load_4444, const uint16_t**) {
+    auto ptr = *ctx + x;
+    from_4444(load(tail, ptr), &r,&g,&b,&a);
+}
+STAGE_CTX(store_4444, uint16_t**) {
+    auto ptr = *ctx + x;
+    store(tail, SkNx_cast<uint16_t>( SkNf_round(r, 0xF) << SK_R4444_SHIFT
+                                   | SkNf_round(g, 0xF) << SK_G4444_SHIFT
+                                   | SkNf_round(b, 0xF) << SK_B4444_SHIFT
+                                   | SkNf_round(a, 0xF) << SK_A4444_SHIFT), ptr);
+}
 
 STAGE_CTX(load_f16, const uint64_t**) {
     auto ptr = *ctx + x;
diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp
index ee762d19a2..aa98e919df 100644
--- a/tests/ReadPixelsTest.cpp
+++ b/tests/ReadPixelsTest.cpp
@@ -7,6 +7,9 @@
 
 #include "SkCanvas.h"
 #include "SkColorPriv.h"
+#include "SkColorSpace_Base.h"
+#include "SkHalf.h"
+#include "SkImageInfoPriv.h"
 #include "SkMathPriv.h"
 #include "SkSurface.h"
 #include "Test.h"
@@ -484,3 +487,158 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Texture, reporter, ctxInfo) {
     }
 }
 #endif
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
+static const uint32_t kNumPixels = 5;
+
+// The five reference pixels are: red, green, blue, white, black.
+// Five is an interesting number to test because we'll exercise a full 4-wide SIMD vector
+// plus a tail pixel.
+static const uint32_t rgba[kNumPixels] = {
+        0xFF0000FF, 0xFF00FF00, 0xFFFF0000, 0xFFFFFFFF, 0xFF000000
+};
+static const uint32_t bgra[kNumPixels] = {
+        0xFFFF0000, 0xFF00FF00, 0xFF0000FF, 0xFFFFFFFF, 0xFF000000
+};
+static const uint16_t rgb565[kNumPixels] = {
+        SK_R16_MASK_IN_PLACE, SK_G16_MASK_IN_PLACE, SK_B16_MASK_IN_PLACE, 0xFFFF, 0x0
+};
+
+static const uint16_t rgba4444[kNumPixels] = { 0xF00F, 0x0F0F, 0x00FF, 0xFFFF, 0x000F };
+
+static const uint64_t kRed      = (uint64_t) SK_Half1 <<  0;
+static const uint64_t kGreen    = (uint64_t) SK_Half1 << 16;
+static const uint64_t kBlue     = (uint64_t) SK_Half1 << 32;
+static const uint64_t kAlpha    = (uint64_t) SK_Half1 << 48;
+static const uint64_t f16[kNumPixels] = {
+        kAlpha | kRed, kAlpha | kGreen, kAlpha | kBlue, kAlpha | kBlue | kGreen | kRed, kAlpha
+};
+
+#ifdef SK_PMCOLOR_IS_RGBA
+static const SkPMColor index8colors[kNumPixels] = {
+        0xFF0000FF, 0xFF00FF00, 0xFFFF0000, 0xFFFFFFFF, 0xFF000000
+};
+#else
+static const SkPMColor index8colors[kNumPixels] = {
+        0xFFFF0000, 0xFF00FF00, 0xFF0000FF, 0xFFFFFFFF, 0xFF000000
+};
+#endif
+static const uint8_t index8[kNumPixels] = { 0, 1, 2, 3, 4 };
+static const uint8_t alpha8[kNumPixels] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
+static const uint8_t gray8[kNumPixels] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };
+
+static const void* five_reference_pixels(SkColorType colorType) {
+    switch (colorType) {
+        case kUnknown_SkColorType:
+            return nullptr;
+        case kAlpha_8_SkColorType:
+            return alpha8;
+        case kRGB_565_SkColorType:
+            return rgb565;
+        case kARGB_4444_SkColorType:
+            return rgba4444;
+        case kRGBA_8888_SkColorType:
+            return rgba;
+        case kBGRA_8888_SkColorType:
+            return bgra;
+        case kIndex_8_SkColorType:
+            return index8;
+        case kGray_8_SkColorType:
+            return gray8;
+        case kRGBA_F16_SkColorType:
+            return f16;
+    }
+
+    SkASSERT(false);
+    return nullptr;
+}
+
+static void test_conversion(skiatest::Reporter* r, const SkImageInfo& dstInfo,
+                            const SkImageInfo& srcInfo) {
+    if (!SkImageInfoIsValid(srcInfo)) {
+        return;
+    }
+
+    sk_sp<SkColorTable> srcColorTable = (kIndex_8_SkColorType == srcInfo.colorType())
+            ? sk_make_sp<SkColorTable>(index8colors, 5)
+            : nullptr;
+    sk_sp<SkColorTable> dstColorTable = (kIndex_8_SkColorType == dstInfo.colorType())
+            ? sk_make_sp<SkColorTable>(index8colors, 5)
+            : nullptr;
+
+    const void* srcPixels = five_reference_pixels(srcInfo.colorType());
+    SkPixmap srcPixmap(srcInfo, srcPixels, srcInfo.minRowBytes(), srcColorTable.get());
+    sk_sp<SkImage> src = SkImage::MakeFromRaster(srcPixmap, nullptr, nullptr);
+    REPORTER_ASSERT(r, src);
+
+    // Enough space for 5 pixels when color type is F16, more than enough space in other cases.
+    uint64_t dstPixels[kNumPixels];
+    SkPixmap dstPixmap(dstInfo, dstPixels, dstInfo.minRowBytes(), dstColorTable.get());
+    bool success = src->readPixels(dstPixmap, 0, 0);
+    REPORTER_ASSERT(r, success == SkImageInfoValidConversion(dstInfo, srcInfo));
+
+    if (success) {
+        if (kGray_8_SkColorType == srcInfo.colorType() &&
+            kGray_8_SkColorType != dstInfo.colorType())
+        {
+            // This conversion is legal, but we won't get the "reference" pixels since we cannot
+            // represent colors in kGray8.
+            return;
+        }
+
+        REPORTER_ASSERT(r, 0 == memcmp(dstPixels, five_reference_pixels(dstInfo.colorType()),
+                                       kNumPixels * SkColorTypeBytesPerPixel(dstInfo.colorType())));
+
+    }
+}
+
+DEF_TEST(ReadPixels_ValidConversion, reporter) {
+    const SkColorType kColorTypes[] = {
+            kUnknown_SkColorType,
+            kAlpha_8_SkColorType,
+            kRGB_565_SkColorType,
+            kARGB_4444_SkColorType,
+            kRGBA_8888_SkColorType,
+            kBGRA_8888_SkColorType,
+            kIndex_8_SkColorType,
+            kGray_8_SkColorType,
+            kRGBA_F16_SkColorType,
+    };
+
+    const SkAlphaType kAlphaTypes[] = {
+            kUnknown_SkAlphaType,
+            kOpaque_SkAlphaType,
+            kPremul_SkAlphaType,
+            kUnpremul_SkAlphaType,
+    };
+
+    const sk_sp<SkColorSpace> kColorSpaces[] = {
+            nullptr,
+            SkColorSpace::MakeSRGB(),
+    };
+
+    for (SkColorType dstCT : kColorTypes) {
+        for (SkAlphaType dstAT: kAlphaTypes) {
+            for (sk_sp<SkColorSpace> dstCS : kColorSpaces) {
+                for (SkColorType srcCT : kColorTypes) {
+                    for (SkAlphaType srcAT: kAlphaTypes) {
+                        for (sk_sp<SkColorSpace> srcCS : kColorSpaces) {
+                            if (kRGBA_F16_SkColorType == dstCT && dstCS) {
+                                dstCS = as_CSB(dstCS)->makeLinearGamma();
+                            }
+
+                            if (kRGBA_F16_SkColorType == srcCT && srcCS) {
+                                srcCS = as_CSB(srcCS)->makeLinearGamma();
+                            }
+
+                            test_conversion(reporter,
+                                            SkImageInfo::Make(kNumPixels, 1, dstCT, dstAT, dstCS),
+                                            SkImageInfo::Make(kNumPixels, 1, srcCT, srcAT, srcCS));
+                        }
+                    }
+                }
+            }
+        }
+    }
+}