Override 32BPP format in SkCanvas::readPixels
authorbsalomon@google.com <bsalomon@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Thu, 3 Nov 2011 20:29:47 +0000 (20:29 +0000)
committerbsalomon@google.com <bsalomon@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Thu, 3 Nov 2011 20:29:47 +0000 (20:29 +0000)
Review URL: http://codereview.appspot.com/5330073/

git-svn-id: http://skia.googlecode.com/svn/trunk@2600 2bbb7eff-a529-9590-31e7-b0007b416f81

include/core/SkCanvas.h
include/core/SkDevice.h
include/device/xps/SkXPSDevice.h
include/gpu/SkGpuDevice.h
include/pdf/SkPDFDevice.h
src/core/SkCanvas.cpp
src/core/SkDevice.cpp
src/gpu/SkGpuDevice.cpp
tests/ReadPixelsTest.cpp

index 00fdfbe..f7f918e 100644 (file)
@@ -106,36 +106,75 @@ public:
     ///////////////////////////////////////////////////////////////////////////
 
     /**
+     * This enum can be used with readPixels to perform a readback into an 8888
+     * format other than Skia's native format (SkPMColor). There are three byte
+     * orders supported: native, BGRA, and RGBA. Each has a premultiplied and
+     * unpremultiplied variant. Unlike the other 8888 configs, the native 8888
+     * config is defined by shift values rather than byte order. The conversion
+     * from the native config to a byte order must consider endianness.
+     */
+    enum Config8888 {
+        /**
+         * Skia's native order specified by:
+         *      SK_A32_SHIFT, SK_R32_SHIFT, SK_G32_SHIFT, and SK_B32_SHIFT
+         *
+         * kNative_Premul_Config8888 is equivalent to SkPMColor
+         * kNative_Unpremul_Config8888 has the same component order as SkPMColor
+         * but is not premultiplied.
+         */
+        kNative_Premul_Config8888,
+        kNative_Unpremul_Config8888,
+        /**
+         * low byte to high byte: B, G, R, A.
+         */
+        kBGRA_Premul_Config8888,
+        kBGRA_Unpremul_Config8888,
+        /**
+         * low byte to high byte: R, G, B, A.
+         */
+        kRGBA_Premul_Config8888,
+        kRGBA_Unpremul_Config8888,
+    };
+
+    /**
      *  On success (returns true), copy the canvas pixels into the bitmap.
      *  On failure, the bitmap parameter is left unchanged and false is
      *  returned.
      *
-     *  If the canvas is backed by a non-raster device (e.g. PDF) then
-     *  readPixels will fail.
+     *  The canvas' pixels are converted to the bitmap's config. The only
+     *  supported config is kARGB_8888_Config, though this is likely to be
+     *  relaxed in  the future. The meaning of config kARGB_8888_Config is
+     *  modified by the enum param config8888. The default value interprets
+     *  kARGB_8888_Config as SkPMColor
      *
      *  If the bitmap has pixels already allocated, the canvas pixels will be
      *  written there. If not, bitmap->allocPixels() will be called 
      *  automatically. If the bitmap is backed by a texture readPixels will
      *  fail.
      *
-     *  The canvas' pixels are converted to the bitmap's config. The only
-     *  supported config is kARGB_8888_Config, though this may be relaxed in
-     *  future.
-     *
      *  The actual pixels written is the intersection of the canvas' bounds, and
      *  the rectangle formed by the bitmap's width,height and the specified x,y.
      *  If bitmap pixels extend outside of that intersection, they will not be
      *  modified.
      *
-     *  Example that reads the entire canvas into a bitmap:
-     *  SkISize size = canvas->getDeviceSize();
-     *  bitmap->setConfig(SkBitmap::kARGB_8888_Config, size.fWidth,
-     *                                                 size.fHeight);
-     *  if (canvas->readPixels(bitmap, 0, 0)) {
-     *     // use the pixels
-     *  }
+     *  Other failure conditions:
+     *    * If the canvas is backed by a non-raster device (e.g. PDF) then
+     *       readPixels will fail.
+     *    * If bitmap is texture-backed then readPixels will fail. (This may be
+     *       relaxed in the future.)
+     *
+     *  Example that reads the entire canvas into a bitmap using the native
+     *  SkPMColor:
+     *    SkISize size = canvas->getDeviceSize();
+     *    bitmap->setConfig(SkBitmap::kARGB_8888_Config, size.fWidth,
+     *                                                   size.fHeight);
+     *    if (canvas->readPixels(bitmap, 0, 0)) {
+     *       // use the pixels
+     *    }
      */
-    bool readPixels(SkBitmap* bitmap, int x, int y);
+    bool readPixels(SkBitmap* bitmap,
+                    int x, int y,
+                    Config8888 config8888 = kNative_Premul_Config8888);
 
     /**
      * DEPRECATED: This will be removed as soon as webkit is no longer relying
index 4dadfc4..488a004 100644 (file)
@@ -107,26 +107,8 @@ public:
     const SkBitmap& accessBitmap(bool changePixels);
 
     /**
-     *  Copy the pixels from the device into bitmap. Returns true on success.
-     *  If false is returned, then the bitmap parameter is left unchanged. The
-     *  rectangle read is defined by x, y and the bitmap's width and height.
-     *
-     *  If the bitmap has pixels allocated the canvas will write directly to
-     *  into that memory (if the call succeeds).
-     *
-     *  The read is clipped to the device bounds. If bitmap pixels were
-     *  preallocated then pixels outside the clip are left unmodified. If the
-     *  call allocates bitmap pixels then pixels outside the clip will be
-     *  uninitialized.
-     *
-     *  Currently bitmap must have kARGB_8888_Config or readPixels will fail.
-     *  This will likely be relaxed in the future.
-     *
-     *  The bitmap parameter is not modified if the call fails.
-     */
-    bool readPixels(SkBitmap* bitmap, int x, int y);
-
-    /**
+     *  DEPRECATED: This will be made protected once WebKit stops using it.
+     *              Instead use Canvas' writePixels method.
      *  Similar to draw sprite, this method will copy the pixels in bitmap onto
      *  the device, with the top/left corner specified by (x, y). The pixel
      *  values in the device are completely replaced: there is no blending.
@@ -254,6 +236,37 @@ protected:
     virtual void drawDevice(const SkDraw&, SkDevice*, int x, int y,
                             const SkPaint&);
 
+    /**
+     *  On success (returns true), copy the device pixels into the bitmap.
+     *  On failure, the bitmap parameter is left unchanged and false is
+     *  returned.
+     *
+     *  The device's pixels are converted to the bitmap's config. The only
+     *  supported config is kARGB_8888_Config, though this is likely to be
+     *  relaxed in  the future. The meaning of config kARGB_8888_Config is
+     *  modified by the enum param config8888. The default value interprets
+     *  kARGB_8888_Config as SkPMColor
+     *
+     *  If the bitmap has pixels already allocated, the device pixels will be
+     *  written there. If not, bitmap->allocPixels() will be called 
+     *  automatically. If the bitmap is backed by a texture readPixels will
+     *  fail.
+     *
+     *  The actual pixels written is the intersection of the device's bounds,
+     *  and the rectangle formed by the bitmap's width,height and the specified
+     *  x,y. If bitmap pixels extend outside of that intersection, they will not
+     *  be modified.
+     *
+     *  Other failure conditions:
+     *    * If the device is not a raster device (e.g. PDF) then readPixels will
+     *      fail.
+     *    * If bitmap is texture-backed then readPixels will fail. (This may be
+     *      relaxed in the future.)
+     */
+    bool readPixels(SkBitmap* bitmap,
+                    int x, int y,
+                    SkCanvas::Config8888 config8888);
+
     ///////////////////////////////////////////////////////////////////////////
 
     /** Update as needed the pixel value in the bitmap, so that the caller can access
@@ -276,8 +289,9 @@ protected:
      *  3. The rectangle (x, y, x + bitmap->width(), y + bitmap->height()) is
      *     contained in the device bounds.
      */
-    virtual bool onReadPixels(const SkBitmap& bitmap, int x, int y);
-
+    virtual bool onReadPixels(const SkBitmap& bitmap,
+                              int x, int y,
+                              SkCanvas::Config8888 config8888);
 
     /** Called when this device is installed into a Canvas. Balanaced by a call
         to unlockPixels() when the device is removed from a Canvas.
@@ -285,6 +299,10 @@ protected:
     virtual void lockPixels();
     virtual void unlockPixels();
 
+    // This is equal kBGRA_Premul_Config8888 or kRGBA_Premul_Config8888 if 
+    // either is identical to kNative_Premul_Config8888. Otherwise, -1.
+    static const SkCanvas::Config8888 kPMColorAlias;
+
 private:
     friend class SkCanvas;
     friend struct DeviceCM; //for setMatrixClip
index bac7f38..2ba20f0 100644 (file)
@@ -143,7 +143,8 @@ protected:
 
     virtual bool onReadPixels(const SkBitmap& bitmap,
                               int x,
-                              int y) SK_OVERRIDE {
+                              int y,
+                              SkCanvas::Config8888) SK_OVERRIDE {
         return false;
     }
 
index d394d12..631cc06 100644 (file)
@@ -142,7 +142,8 @@ protected:
     
     // overrides from SkDevice
     virtual bool onReadPixels(const SkBitmap& bitmap,
-                              int x, int y) SK_OVERRIDE;
+                              int x, int y,
+                              SkCanvas::Config8888 config8888) SK_OVERRIDE;
 
 
 private:
index 1e4db86..b74e98f 100644 (file)
@@ -159,7 +159,8 @@ public:
     
 protected:
     virtual bool onReadPixels(const SkBitmap& bitmap,
-                              int x, int y) SK_OVERRIDE {
+                              int x, int y,
+                              SkCanvas::Config8888) SK_OVERRIDE {
         return false;
     }
 
index f70795d..1640a94 100644 (file)
@@ -550,12 +550,14 @@ SkDevice* SkCanvas::setBitmapDevice(const SkBitmap& bitmap) {
     return device;
 }
 
-bool SkCanvas::readPixels(SkBitmap* bitmap, int x, int y) {
+bool SkCanvas::readPixels(SkBitmap* bitmap,
+                          int x, int y,
+                          Config8888 config8888) {
     SkDevice* device = this->getDevice();
     if (!device) {
         return false;
     }
-    return device->readPixels(bitmap, x, y);
+    return device->readPixels(bitmap, x, y, config8888);
 }
 
 bool SkCanvas::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) {
index 4e2cd0a..bf25ff1 100644 (file)
@@ -102,7 +102,8 @@ void SkDevice::setMatrixClip(const SkMatrix& matrix, const SkRegion& region,
 
 ///////////////////////////////////////////////////////////////////////////////
 
-bool SkDevice::readPixels(SkBitmap* bitmap, int x, int y) {
+bool SkDevice::readPixels(SkBitmap* bitmap, int x, int y,
+                          SkCanvas::Config8888 config8888) {
     if (SkBitmap::kARGB_8888_Config != bitmap->config() ||
         NULL != bitmap->getTexture()) {
         return false;
@@ -135,14 +136,149 @@ bool SkDevice::readPixels(SkBitmap* bitmap, int x, int y) {
     SkBitmap bmpSubset;
     bmp->extractSubset(&bmpSubset, subrect);
 
-    bool result = this->onReadPixels(bmpSubset, srcRect.fLeft, srcRect.fTop);
+    bool result = this->onReadPixels(bmpSubset,
+                                     srcRect.fLeft,
+                                     srcRect.fTop,
+                                     config8888);
     if (result && bmp == &tmp) {
         tmp.swap(*bitmap);
     }
     return result;
 }
 
-bool SkDevice::onReadPixels(const SkBitmap& bitmap, int x, int y) {
+#ifdef SK_CPU_LENDIAN
+    #if   24 == SK_A32_SHIFT && 16 == SK_R32_SHIFT && \
+           8 == SK_G32_SHIFT &&  0 == SK_B32_SHIFT
+        const SkCanvas::Config8888 SkDevice::kPMColorAlias =
+            SkCanvas::kBGRA_Premul_Config8888;
+    #elif 24 == SK_A32_SHIFT &&  0 == SK_R32_SHIFT && \
+           8 == SK_G32_SHIFT && 16 == SK_B32_SHIFT
+        const SkCanvas::Config8888 SkDevice::kPMColorAlias =
+            SkCanvas::kRGBA_Premul_Config8888;
+    #else
+        const SkCanvas::Config8888 SkDevice::kPMColorAlias =
+            (SkCanvas::Config8888) -1;
+    #endif
+    static const int NATIVE_A_IDX = SK_A32_SHIFT / 8;
+    static const int NATIVE_R_IDX = SK_R32_SHIFT / 8;
+    static const int NATIVE_G_IDX = SK_G32_SHIFT / 8;
+    static const int NATIVE_B_IDX = SK_B32_SHIFT / 8;
+#else
+    #if    0 == SK_A32_SHIFT &&   8 == SK_R32_SHIFT && \
+          16 == SK_G32_SHIFT &&  24 == SK_B32_SHIFT
+        const SkCanvas::Config8888 SkDevice::kPMColorAlias =
+            SkCanvas::kBGRA_Premul_Config8888;
+    #elif  0 == SK_A32_SHIFT &&  24 == SK_R32_SHIFT && \
+          16 == SK_G32_SHIFT &&   8 == SK_B32_SHIFT
+        const SkCanvas::Config8888 SkDevice::kPMColorAlias =
+            SkCanvas::kRGBA_Premul_Config8888;
+    #else
+        const SkCanvas::Config8888 SkDevice::kPMColorAlias =
+            (SkCanvas::Config8888) -1;
+    #endif
+    static const int NATIVE_A_IDX = 3 - (SK_A32_SHIFT / 8);
+    static const int NATIVE_R_IDX = 3 - (SK_R32_SHIFT / 8);
+    static const int NATIVE_G_IDX = 3 - (SK_G32_SHIFT / 8);
+    static const int NATIVE_B_IDX = 3 - (SK_B32_SHIFT / 8);
+#endif
+
+#include <SkColorPriv.h>
+
+namespace {
+
+template <int A_IDX, int R_IDX, int G_IDX, int B_IDX>
+inline uint32_t pack_config8888(uint32_t a, uint32_t r,
+                                uint32_t g, uint32_t b) {
+#ifdef SK_CPU_LENDIAN
+    return (a << (A_IDX * 8)) | (r << (R_IDX * 8)) |
+           (g << (G_IDX * 8)) | (b << (B_IDX * 8));
+#else
+    return (a << ((3-A_IDX) * 8)) | (r << ((3-R_IDX) * 8)) |
+           (g << ((3-G_IDX) * 8)) | (b << ((3-B_IDX) * 8));
+#endif
+}
+
+template <bool UNPM, int A_IDX, int R_IDX, int G_IDX, int B_IDX>
+inline void bitmap_copy_to_config8888(const SkBitmap& srcBmp,
+                                        uint32_t* dstPixels,
+                                        size_t dstRowBytes) {
+    SkASSERT(SkBitmap::kARGB_8888_Config == srcBmp.config());
+    SkAutoLockPixels alp(srcBmp);
+    int w = srcBmp.width();
+    int h = srcBmp.height();
+    size_t srcRowBytes = srcBmp.rowBytes();
+
+    intptr_t src = reinterpret_cast<intptr_t>(srcBmp.getPixels());
+    intptr_t dst = reinterpret_cast<intptr_t>(dstPixels);
+
+    for (int y = 0; y < h; ++y) {
+        const SkPMColor* srcRow = reinterpret_cast<SkPMColor*>(src);
+        uint32_t* dstRow  = reinterpret_cast<uint32_t*>(dst);
+        for (int x = 0; x < w; ++x) {
+            SkPMColor pmcolor = srcRow[x];
+            if (UNPM) {
+                U8CPU a, r, g, b;
+                a = SkGetPackedA32(pmcolor);
+                if (a) {
+                    // We're doing the explicit divide to match WebKit layout
+                    // test expectations. We can modify and rebaseline if there
+                    // it can be shown that there is a more performant way to
+                    // unpremul.
+                    r = SkGetPackedR32(pmcolor) * 0xff / a;
+                    g = SkGetPackedG32(pmcolor) * 0xff / a;
+                    b = SkGetPackedB32(pmcolor) * 0xff / a;
+                    dstRow[x] = pack_config8888<A_IDX, R_IDX,
+                                                G_IDX, B_IDX>(a, r, g, b);
+                } else {
+                    dstRow[x] = 0;
+                }
+            } else {
+                dstRow[x] = pack_config8888<A_IDX, R_IDX,
+                                            G_IDX, B_IDX>(
+                                                   SkGetPackedA32(pmcolor),
+                                                   SkGetPackedR32(pmcolor),
+                                                   SkGetPackedG32(pmcolor),
+                                                   SkGetPackedB32(pmcolor));
+            }
+        }
+        dst += dstRowBytes;
+        src += srcRowBytes;
+    }
+}
+
+inline void bitmap_copy_to_native(const SkBitmap& srcBmp,
+                                  uint32_t* dstPixels,
+                                  size_t dstRowBytes) {
+    SkASSERT(SkBitmap::kARGB_8888_Config == srcBmp.config());
+
+    SkAutoLockPixels alp(srcBmp);
+
+    int w = srcBmp.width();
+    int h = srcBmp.height();
+    size_t srcRowBytes = srcBmp.rowBytes();
+
+    size_t tightRowBytes = w * 4;
+
+    char* src = reinterpret_cast<char*>(srcBmp.getPixels());
+    char* dst = reinterpret_cast<char*>(dstPixels);
+
+    if (tightRowBytes == srcRowBytes &&
+        tightRowBytes == dstRowBytes) {
+        memcpy(dst, src, tightRowBytes * h);
+    } else {
+        for (int y = 0; y < h; ++y) {
+            memcpy(dst, src, tightRowBytes);
+            dst += dstRowBytes;
+            src += srcRowBytes;
+        }
+    }
+}
+
+}
+
+bool SkDevice::onReadPixels(const SkBitmap& bitmap,
+                            int x, int y,
+                            SkCanvas::Config8888 config8888) {
     SkASSERT(SkBitmap::kARGB_8888_Config == bitmap.config());
     SkASSERT(!bitmap.isNull());
     SkASSERT(SkIRect::MakeWH(this->width(), this->height()).contains(SkIRect::MakeXYWH(x, y, bitmap.width(), bitmap.height())));
@@ -157,15 +293,53 @@ bool SkDevice::onReadPixels(const SkBitmap& bitmap, int x, int y) {
     }
     if (SkBitmap::kARGB_8888_Config != subset.config()) {
         // It'd be preferable to do this directly to bitmap.
-        // We'd need a SkBitmap::copyPixelsTo that takes a config
-        // or make copyTo lazily allocate.
         subset.copyTo(&subset, SkBitmap::kARGB_8888_Config); 
     }
     SkAutoLockPixels alp(bitmap);
-    return subset.copyPixelsTo(bitmap.getPixels(),
-                               bitmap.getSize(),
-                               bitmap.rowBytes(),
-                               true);
+    uint32_t* bmpPixels = reinterpret_cast<uint32_t*>(bitmap.getPixels());
+    if ((SkCanvas::kNative_Premul_Config8888 == config8888 ||
+         kPMColorAlias == config8888)) {
+        bitmap_copy_to_native(subset, bmpPixels, bitmap.rowBytes());
+    } else {
+        switch (config8888) {
+            case SkCanvas::kNative_Premul_Config8888:
+                bitmap_copy_to_config8888<false,
+                                          NATIVE_A_IDX, NATIVE_R_IDX,
+                                          NATIVE_G_IDX, NATIVE_B_IDX>(
+                                                subset,
+                                                bmpPixels,
+                                                bitmap.rowBytes());
+                break;
+            case SkCanvas::kNative_Unpremul_Config8888:
+                bitmap_copy_to_config8888<true,
+                                          NATIVE_A_IDX, NATIVE_R_IDX,
+                                          NATIVE_G_IDX, NATIVE_B_IDX>(
+                                                subset,
+                                                bmpPixels,
+                                                bitmap.rowBytes());
+                break;
+            case SkCanvas::kBGRA_Premul_Config8888:
+                bitmap_copy_to_config8888<false, 3, 2, 1, 0> (
+                                        subset, bmpPixels, bitmap.rowBytes());
+                break;
+            case SkCanvas::kBGRA_Unpremul_Config8888:
+                bitmap_copy_to_config8888<true, 3, 2, 1, 0> (
+                                        subset, bmpPixels, bitmap.rowBytes());
+                break;
+            case SkCanvas::kRGBA_Premul_Config8888:
+                bitmap_copy_to_config8888<false, 3, 0, 1, 2> (
+                                        subset, bmpPixels, bitmap.rowBytes());
+                break;
+            case SkCanvas::kRGBA_Unpremul_Config8888:
+                bitmap_copy_to_config8888<true, 3, 0, 1, 2> (
+                                        subset, bmpPixels, bitmap.rowBytes());
+                break;
+            default:
+                SkASSERT(false && "unexpected Config8888");
+                break;
+        }
+    }
+    return true;
 }
 
 void SkDevice::writePixels(const SkBitmap& bitmap, int x, int y) {
index f145939..5283dc0 100644 (file)
@@ -256,7 +256,13 @@ void SkGpuDevice::makeRenderTargetCurrent() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-bool SkGpuDevice::onReadPixels(const SkBitmap& bitmap, int x, int y) {
+bool SkGpuDevice::onReadPixels(const SkBitmap& bitmap,
+                               int x, int y,
+                               SkCanvas::Config8888 config8888) {
+    // support for non-native configs coming soon
+    if (config8888 != SkCanvas::kNative_Premul_Config8888) {
+        return false;
+    }
     SkASSERT(SkBitmap::kARGB_8888_Config == bitmap.config());
     SkASSERT(!bitmap.isNull());
     SkASSERT(SkIRect::MakeWH(this->width(), this->height()).contains(SkIRect::MakeXYWH(x, y, bitmap.width(), bitmap.height())));
index c39e03b..e2b21c4 100644 (file)
@@ -21,18 +21,81 @@ namespace {
 SkPMColor getCanvasColor(int x, int y) {
     SkASSERT(x >= 0 && x < DEV_W);
     SkASSERT(y >= 0 && y < DEV_H);
-    return SkPackARGB32(0xff, x, y, 0x0);
+
+    U8CPU r = x;
+    U8CPU g = y;
+    U8CPU b = 0xc;
+
+    U8CPU a = 0xff;
+    switch (x % 5) {
+        case 0:
+            a = 0xff;
+            break;
+        case 1:
+            a = 0x80;
+            break;
+        case 2:
+            a = 0xCC;
+            break;
+        case 4:
+            a = 0x01;
+            break;
+        case 3:
+            a = 0x00;
+            break;
+    }
+    return SkPremultiplyARGBInline(a, r, g, b);
 }
     
 SkPMColor getBitmapColor(int x, int y, int w, int h) {
     int n = y * w + x;
-    
+
     U8CPU b = n & 0xff;
     U8CPU g = (n >> 8) & 0xff;
     U8CPU r = (n >> 16) & 0xff;
     return SkPackARGB32(0xff, r, g , b);
 }
 
+SkPMColor convertConfig8888ToPMColor(SkCanvas::Config8888 config8888,
+                                     uint32_t color) {
+    const uint8_t* c = reinterpret_cast<uint8_t*>(&color);
+    U8CPU a,r,g,b;
+    bool mul = false;
+    switch (config8888) {
+        case SkCanvas::kNative_Premul_Config8888:
+            return color;
+        case SkCanvas::kNative_Unpremul_Config8888:
+            mul = true;
+            a = SkGetPackedA32(color);
+            r = SkGetPackedR32(color);
+            g = SkGetPackedG32(color);
+            b = SkGetPackedB32(color);
+            break;
+        case SkCanvas::kBGRA_Unpremul_Config8888:
+            mul = true; // fallthru
+        case SkCanvas::kBGRA_Premul_Config8888:
+            a = static_cast<U8CPU>(c[3]);
+            r = static_cast<U8CPU>(c[2]);
+            g = static_cast<U8CPU>(c[1]);
+            b = static_cast<U8CPU>(c[0]);
+            break;
+        case SkCanvas::kRGBA_Unpremul_Config8888:
+            mul = true; // fallthru
+        case SkCanvas::kRGBA_Premul_Config8888:
+            a = static_cast<U8CPU>(c[3]);
+            r = static_cast<U8CPU>(c[0]);
+            g = static_cast<U8CPU>(c[1]);
+            b = static_cast<U8CPU>(c[2]);
+            break;
+    }
+    if (mul) {
+        r = SkMulDiv255Ceiling(r, a);
+        g = SkMulDiv255Ceiling(g, a);
+        b = SkMulDiv255Ceiling(b, a);
+    }
+    return SkPackARGB32(a, r, g, b);
+}
+
 void fillCanvas(SkCanvas* canvas) {
     static SkBitmap bmp;
     if (bmp.isNull()) {
@@ -77,9 +140,12 @@ void fillBitmap(SkBitmap* bitmap) {
 bool checkRead(skiatest::Reporter* reporter,
                const SkBitmap& bitmap,
                int x, int y,
-               bool preFilledBmp) {
+               bool checkCanvasPixels,
+               bool checkBitmapPixels,
+               SkCanvas::Config8888 config8888) {
     SkASSERT(SkBitmap::kARGB_8888_Config == bitmap.config());
     SkASSERT(!bitmap.isNull());
+    SkASSERT(checkCanvasPixels || checkBitmapPixels);
     
     int bw = bitmap.width();
     int bh = bitmap.height();
@@ -100,11 +166,15 @@ bool checkRead(skiatest::Reporter* reporter,
             SkPMColor pixel = *reinterpret_cast<SkPMColor*>(pixels + by * bitmap.rowBytes() + bx * bitmap.bytesPerPixel());
 
             if (clippedSrcRect.contains(devx, devy)) {
-                REPORTER_ASSERT(reporter, getCanvasColor(devx, devy) == pixel);
-                if (getCanvasColor(devx, devy) != pixel) {
-                    return false;
+                if (checkCanvasPixels) {
+                    SkPMColor canvasPixel = getCanvasColor(devx, devy);
+                    pixel = convertConfig8888ToPMColor(config8888, pixel);
+                    REPORTER_ASSERT(reporter, canvasPixel == pixel);
+                    if (getCanvasColor(devx, devy) != pixel) {
+                        return false;
+                    }
                 }
-            } else if (preFilledBmp) {
+            } else if (checkBitmapPixels) {
                 REPORTER_ASSERT(reporter, getBitmapColor(bx, by, bw, bh) == pixel);
                 if (getBitmapColor(bx, by, bw, bh) != pixel) {
                     return false;
@@ -207,51 +277,79 @@ void ReadPixelsTest(skiatest::Reporter* reporter, GrContext* context) {
     for (int dtype = 0; dtype < 2; ++dtype) {
 
         if (0 == dtype) {
-            canvas.setDevice(new SkDevice(SkBitmap::kARGB_8888_Config, DEV_W, DEV_H, false))->unref();
+            canvas.setDevice(new SkDevice(SkBitmap::kARGB_8888_Config,
+                                          DEV_W,
+                                          DEV_H,
+                                          false))->unref();
         } else {
 #if SK_SCALAR_IS_FIXED
             // GPU device known not to work in the fixed pt build.
             continue;
 #endif
-            canvas.setDevice(new SkGpuDevice(context, SkBitmap::kARGB_8888_Config, DEV_W, DEV_H))->unref();
+            canvas.setDevice(new SkGpuDevice(context,
+                                             SkBitmap::kARGB_8888_Config,
+                                             DEV_W,
+                                             DEV_H))->unref();
         }
         fillCanvas(&canvas);
 
+        static const SkCanvas::Config8888 gReadConfigs[] = {
+            SkCanvas::kNative_Premul_Config8888,
+            SkCanvas::kNative_Unpremul_Config8888,
+            SkCanvas::kBGRA_Premul_Config8888,
+            SkCanvas::kBGRA_Unpremul_Config8888,
+            SkCanvas::kRGBA_Premul_Config8888,
+            SkCanvas::kRGBA_Unpremul_Config8888,
+        };
         for (int rect = 0; rect < SK_ARRAY_COUNT(testRects); ++rect) {
-            SkBitmap bmp;
-            for (BitmapInit bmi = kFirstBitmapInit; bmi < kBitmapInitCnt; bmi = nextBMI(bmi)) {
+            const SkIRect& srcRect = testRects[rect];
+            for (BitmapInit bmi = kFirstBitmapInit;
+                 bmi < kBitmapInitCnt;
+                 bmi = nextBMI(bmi)) {
+                for (int c = 0; c < SK_ARRAY_COUNT(gReadConfigs); ++c) {
+                    SkCanvas::Config8888 config8888 = gReadConfigs[c];
+                    SkBitmap bmp;
+                    init_bitmap(&bmp, srcRect, bmi);
 
-                const SkIRect& srcRect = testRects[rect];
+                    // if the bitmap has pixels allocated before the readPixels,
+                    // note that and fill them with pattern
+                    bool startsWithPixels = !bmp.isNull();
+                    if (startsWithPixels) {
+                        fillBitmap(&bmp);
+                    }
 
-                init_bitmap(&bmp, srcRect, bmi);
+                    bool success =
+                        canvas.readPixels(&bmp, srcRect.fLeft,
+                                          srcRect.fTop, config8888);
 
-                // if the bitmap has pixels allocated before the readPixels, note
-                // that and fill them with pattern
-                bool startsWithPixels = !bmp.isNull();
-                if (startsWithPixels) {
-                    fillBitmap(&bmp);
-                }
-                
-                bool success = canvas.readPixels(&bmp, srcRect.fLeft, srcRect.fTop);
-                
-                // determine whether we expected the read to succeed.
-                REPORTER_ASSERT(reporter, success == SkIRect::Intersects(srcRect, DEV_RECT));
+                    // non-native not implemented on GPU yet
+                    bool expectSuccess =
+                        SkIRect::Intersects(srcRect, DEV_RECT) &&
+                        !(1 == dtype &&
+                          config8888 != SkCanvas::kNative_Premul_Config8888);
 
-                if (success || startsWithPixels) {
-                    checkRead(reporter, bmp, srcRect.fLeft, srcRect.fTop, startsWithPixels);
-                } else {
-                    // if we had no pixels beforehand and the readPixels failed then
-                    // our bitmap should still not have any pixels
-                    REPORTER_ASSERT(reporter, bmp.isNull());
-                }
+                    // determine whether we expected the read to succeed.
+                    REPORTER_ASSERT(reporter, success == expectSuccess);
 
-                // check the old webkit version of readPixels that clips the bitmap size
+                    if (success || startsWithPixels) {
+                        checkRead(reporter, bmp, srcRect.fLeft, srcRect.fTop,
+                                  success, startsWithPixels, config8888);
+                    } else {
+                        // if we had no pixels beforehand and the readPixels
+                        // failed then our bitmap should still not have pixels
+                        REPORTER_ASSERT(reporter, bmp.isNull());
+                    }
+                }
+                // check the old webkit version of readPixels that clips the
+                // bitmap size
                 SkBitmap wkbmp;
-                success = canvas.readPixels(srcRect, &wkbmp);
+                bool success = canvas.readPixels(srcRect, &wkbmp);
                 SkIRect clippedRect = DEV_RECT;
                 if (clippedRect.intersect(srcRect)) {
                     REPORTER_ASSERT(reporter, success);
-                    checkRead(reporter, wkbmp, clippedRect.fLeft, clippedRect.fTop, false);
+                    checkRead(reporter, wkbmp, clippedRect.fLeft,
+                              clippedRect.fTop, true, false,
+                              SkCanvas::kNative_Premul_Config8888);
                 } else {
                     REPORTER_ASSERT(reporter, !success);
                 }