add new readPixels with direct memory parameters

BUG=skia:
R=scroggo@google.com, bsalomon@google.com, robertphillips@google.com, fmalita@google.com

Author: reed@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@13840 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/bench/PremulAndUnpremulAlphaOpsBench.cpp b/bench/PremulAndUnpremulAlphaOpsBench.cpp
index 4afa43f..8eb9028 100644
--- a/bench/PremulAndUnpremulAlphaOpsBench.cpp
+++ b/bench/PremulAndUnpremulAlphaOpsBench.cpp
@@ -12,12 +12,16 @@
 #include "sk_tool_utils.h"
 
 class PremulAndUnpremulAlphaOpsBench : public SkBenchmark {
+    enum {
+        W = 256,
+        H = 256,
+    };
+    SkBitmap fBmp1, fBmp2;
+    
 public:
-    PremulAndUnpremulAlphaOpsBench(SkCanvas::Config8888 config) {
-        fUnPremulConfig = config;
-        fName.printf("premul_and_unpremul_alpha_%s",
-                     (config ==  SkCanvas::kRGBA_Unpremul_Config8888) ?
-                     "RGBA8888" : "Native8888");
+    PremulAndUnpremulAlphaOpsBench(SkColorType ct) {
+        fColorType = ct;
+        fName.printf("premul_and_unpremul_alpha_%s", sk_tool_utils::colortype_name(ct));
     }
 
 protected:
@@ -25,47 +29,38 @@
         return fName.c_str();
     }
 
+    virtual void onPreDraw() {
+        SkImageInfo info = SkImageInfo::Make(W, H, fColorType, kPremul_SkAlphaType);
+        fBmp1.allocPixels(info);   // used in writePixels
+
+        for (int h = 0; h < H; ++h) {
+            for (int w = 0; w < W; ++w) {
+                // SkColor places A in the right slot for either RGBA or BGRA
+                *fBmp1.getAddr32(w, h) = SkColorSetARGB(h & 0xFF, w & 0xFF, w & 0xFF, w & 0xFF);
+            }
+        }
+        
+        fBmp2.allocPixels(info);    // used in readPixels()
+    }
+
     virtual void onDraw(const int loops, SkCanvas* canvas) SK_OVERRIDE {
         canvas->clear(SK_ColorBLACK);
-        SkISize size = canvas->getDeviceSize();
-
-        SkBitmap bmp1;
-        bmp1.setConfig(SkBitmap::kARGB_8888_Config, size.width(),
-                       size.height());
-        bmp1.allocPixels();
-        SkAutoLockPixels alp(bmp1);
-        uint32_t* pixels = reinterpret_cast<uint32_t*>(bmp1.getPixels());
-        for (int h = 0; h < size.height(); ++h) {
-            for (int w = 0; w < size.width(); ++w)
-                pixels[h * size.width() + w] = SkPackConfig8888(fUnPremulConfig,
-                    h & 0xFF, w & 0xFF, w & 0xFF, w & 0xFF);
-        }
-
-        SkBitmap bmp2;
-        bmp2.setConfig(SkBitmap::kARGB_8888_Config, size.width(),
-                       size.height());
-
-        SkColorType ct;
-        SkAlphaType at;
-        sk_tool_utils::config8888_to_imagetypes(fUnPremulConfig, &ct, &at);
-        if (bmp1.isOpaque()) {
-            at = kOpaque_SkAlphaType;
-        }
 
         for (int loop = 0; loop < loops; ++loop) {
             // Unpremul -> Premul
-            sk_tool_utils::write_pixels(canvas, bmp1, 0, 0, ct, at);
+            canvas->writePixels(fBmp1.info(), fBmp1.getPixels(), fBmp1.rowBytes(), 0, 0);
             // Premul -> Unpremul
-            canvas->readPixels(&bmp2, 0, 0, fUnPremulConfig);
+            canvas->readPixels(fBmp2.info(), fBmp2.getPixels(), fBmp2.rowBytes(), 0, 0);
         }
     }
 
 private:
-    SkCanvas::Config8888 fUnPremulConfig;
+    SkColorType fColorType;
     SkString fName;
+
     typedef SkBenchmark INHERITED;
 };
 
 
-DEF_BENCH(return new PremulAndUnpremulAlphaOpsBench(SkCanvas::kRGBA_Unpremul_Config8888));
-DEF_BENCH(return new PremulAndUnpremulAlphaOpsBench(SkCanvas::kNative_Unpremul_Config8888));
+DEF_BENCH(return new PremulAndUnpremulAlphaOpsBench(kRGBA_8888_SkColorType));
+DEF_BENCH(return new PremulAndUnpremulAlphaOpsBench(kBGRA_8888_SkColorType));
diff --git a/gyp/skia_for_chromium_defines.gypi b/gyp/skia_for_chromium_defines.gypi
index 9275298..40c574f 100644
--- a/gyp/skia_for_chromium_defines.gypi
+++ b/gyp/skia_for_chromium_defines.gypi
@@ -15,6 +15,7 @@
     'skia_for_chromium_defines': [
       'SK_SUPPORT_LEGACY_COMPATIBLEDEVICE_CONFIG=1',
       'SK_SUPPORT_LEGACY_PUBLICEFFECTCONSTRUCTORS=1',
+      'SK_SUPPORT_LEGACY_READPIXELSCONFIG',
       'SK_SUPPORT_LEGACY_GETCLIPTYPE',
       'SK_SUPPORT_LEGACY_GETTOTALCLIP',
       'SK_SUPPORT_LEGACY_GETTOPDEVICE',
diff --git a/include/core/SkBitmapDevice.h b/include/core/SkBitmapDevice.h
index 85b70f4..076fb8a 100644
--- a/include/core/SkBitmapDevice.h
+++ b/include/core/SkBitmapDevice.h
@@ -195,7 +195,10 @@
      *  3. The rectangle (x, y, x + bitmap->width(), y + bitmap->height()) is
      *     contained in the device bounds.
      */
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
     virtual bool onReadPixels(const SkBitmap&, int x, int y, SkCanvas::Config8888) SK_OVERRIDE;
+#endif
+    virtual bool onReadPixels(const SkImageInfo&, void*, size_t, int x, int y) SK_OVERRIDE;
     virtual bool onWritePixels(const SkImageInfo&, const void*, size_t, int, int) SK_OVERRIDE;
     virtual void* onAccessPixels(SkImageInfo* info, size_t* rowBytes) SK_OVERRIDE;
 
diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h
index ca40107..8e5f3ac 100644
--- a/include/core/SkCanvas.h
+++ b/include/core/SkCanvas.h
@@ -18,6 +18,8 @@
 #include "SkRegion.h"
 #include "SkXfermode.h"
 
+//#define SK_SUPPORT_LEGACY_READPIXELSCONFIG
+
 // if not defined, we always assume ClipToLayer for saveLayer()
 //#define SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG
 
@@ -264,6 +266,7 @@
         kRGBA_Unpremul_Config8888
     };
 
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
     /**
      *  On success (returns true), copy the canvas pixels into the bitmap.
      *  On failure, the bitmap parameter is left unchanged and false is
@@ -300,15 +303,42 @@
      *       // use the pixels
      *    }
      */
-    bool readPixels(SkBitmap* bitmap,
-                    int x, int y,
-                    Config8888 config8888 = kNative_Premul_Config8888);
+    bool readPixels(SkBitmap* bitmap, int x, int y, Config8888 config8888);
+#endif
 
     /**
-     * DEPRECATED: This will be removed as soon as webkit is no longer relying
-     * on it. The bitmap is resized to the intersection of srcRect and the
-     * canvas bounds. New pixels are always allocated on success. Bitmap is
-     * unmodified on failure.
+     *  Copy the pixels from the base-layer into the specified buffer (pixels + rowBytes),
+     *  converting them into the requested format (SkImageInfo). The base-layer pixels are read
+     *  starting at the specified (x,y) location in the coordinate system of the base-layer.
+     *
+     *  The specified ImageInfo and (x,y) offset specifies a source rectangle
+     *
+     *      srcR(x, y, info.width(), info.height());
+     *
+     *  SrcR is intersected with the bounds of the base-layer. If this intersection is not empty,
+     *  then we have two sets of pixels (of equal size), the "src" specified by base-layer at (x,y)
+     *  and the "dst" by info+pixels+rowBytes. Replace the dst pixels with the corresponding src
+     *  pixels, performing any colortype/alphatype transformations needed (in the case where the
+     *  src and dst have different colortypes or alphatypes).
+     *
+     *  This call can fail, returning false, for several reasons:
+     *  - If the requested colortype/alphatype cannot be converted from the base-layer's types.
+     *  - If this canvas is not backed by pixels (e.g. picture or PDF)
+     */
+    bool readPixels(const SkImageInfo&, void* pixels, size_t rowBytes, int x, int y);
+
+    /**
+     *  Helper for calling readPixels(info, ...). This call will check if bitmap has been allocated.
+     *  If not, it will attempt to call allocPixels(). If this fails, it will return false. If not,
+     *  it calls through to readPixels(info, ...) and returns its result.
+     */
+    bool readPixels(SkBitmap* bitmap, int x, int y);
+
+    /**
+     *  Helper for allocating pixels and then calling readPixels(info, ...). The bitmap is resized
+     *  to the intersection of srcRect and the base-layer bounds. On success, pixels will be
+     *  allocated in bitmap and true returned. On failure, false is returned and bitmap will be
+     *  set to empty.
      */
     bool readPixels(const SkIRect& srcRect, SkBitmap* bitmap);
 
diff --git a/include/core/SkDevice.h b/include/core/SkDevice.h
index 7bb2579..abd0a63 100644
--- a/include/core/SkDevice.h
+++ b/include/core/SkDevice.h
@@ -287,6 +287,7 @@
     virtual void drawDevice(const SkDraw&, SkBaseDevice*, int x, int y,
                             const SkPaint&) = 0;
 
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
     /**
      *  On success (returns true), copy the device pixels into the bitmap.
      *  On failure, the bitmap parameter is left unchanged and false is
@@ -317,6 +318,8 @@
     bool readPixels(SkBitmap* bitmap,
                     int x, int y,
                     SkCanvas::Config8888 config8888);
+#endif
+    bool readPixels(const SkImageInfo&, void* dst, size_t rowBytes, int x, int y);
 
     ///////////////////////////////////////////////////////////////////////////
 
@@ -378,9 +381,17 @@
      *  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,
-                              SkCanvas::Config8888 config8888);
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
+    virtual bool onReadPixels(const SkBitmap& bitmap, int x, int y, SkCanvas::Config8888);
+#endif
+
+    /**
+     *  The caller is responsible for "pre-clipping" the dst. The impl can assume that the dst
+     *  image at the specified x,y offset will fit within the device's bounds.
+     *
+     *  This is explicitly asserted in readPixels(), the public way to call this.
+     */
+    virtual bool onReadPixels(const SkImageInfo&, void*, size_t, int x, int y);
 
     /**
      *  The caller is responsible for "pre-clipping" the src. The impl can assume that the src
diff --git a/include/gpu/SkGpuDevice.h b/include/gpu/SkGpuDevice.h
index 8549595..4ea67fe 100644
--- a/include/gpu/SkGpuDevice.h
+++ b/include/gpu/SkGpuDevice.h
@@ -145,7 +145,10 @@
 
 
 protected:
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
     virtual bool onReadPixels(const SkBitmap&, int x, int y, SkCanvas::Config8888) SK_OVERRIDE;
+#endif
+    virtual bool onReadPixels(const SkImageInfo&, void*, size_t, int, int) SK_OVERRIDE;
     virtual bool onWritePixels(const SkImageInfo&, const void*, size_t, int, int) SK_OVERRIDE;
 
     /**  PRIVATE / EXPERIMENTAL -- do not call */
diff --git a/include/pdf/SkPDFDevice.h b/include/pdf/SkPDFDevice.h
index daec7a3..a51684e 100644
--- a/include/pdf/SkPDFDevice.h
+++ b/include/pdf/SkPDFDevice.h
@@ -208,9 +208,10 @@
     }
 
 protected:
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
     virtual bool onReadPixels(const SkBitmap& bitmap, int x, int y,
                               SkCanvas::Config8888) SK_OVERRIDE;
-
+#endif
     virtual bool allowImageFilter(const SkImageFilter*) SK_OVERRIDE;
 
 private:
diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index 476a124..868306c 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -173,6 +173,7 @@
     return true;
 }
 
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
 bool SkBitmapDevice::onReadPixels(const SkBitmap& bitmap,
                                   int x, int y,
                                   SkCanvas::Config8888 config8888) {
@@ -198,6 +199,7 @@
     SkCopyBitmapToConfig8888(bmpPixels, bitmap.rowBytes(), config8888, subset);
     return true;
 }
+#endif
 
 void* SkBitmapDevice::onAccessPixels(SkImageInfo* info, size_t* rowBytes) {
     if (fBitmap.getPixels()) {
@@ -246,8 +248,8 @@
 
 // TODO: make this guy real, and not rely on legacy config8888 utility
 #include "SkConfig8888.h"
-static bool write_pixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
-                         const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes) {
+static bool copy_pixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
+                        const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRowBytes) {
     if (srcInfo.dimensions() != dstInfo.dimensions()) {
         return false;
     }
@@ -295,13 +297,39 @@
     void* dstPixels = fBitmap.getAddr(x, y);
     size_t dstRowBytes = fBitmap.rowBytes();
 
-    if (write_pixels(dstInfo, dstPixels, dstRowBytes, srcInfo, srcPixels, srcRowBytes)) {
+    if (copy_pixels(dstInfo, dstPixels, dstRowBytes, srcInfo, srcPixels, srcRowBytes)) {
         fBitmap.notifyPixelsChanged();
         return true;
     }
     return false;
 }
 
+bool SkBitmapDevice::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
+                                  int x, int y) {
+    // since we don't stop creating un-pixeled devices yet, check for no pixels here
+    if (NULL == fBitmap.getPixels()) {
+        return false;
+    }
+    
+    SkImageInfo srcInfo = fBitmap.info();
+
+    // perhaps can relax these in the future
+    if (4 != dstInfo.bytesPerPixel()) {
+        return false;
+    }
+    if (4 != srcInfo.bytesPerPixel()) {
+        return false;
+    }
+
+    srcInfo.fWidth = dstInfo.width();
+    srcInfo.fHeight = dstInfo.height();
+    
+    const void* srcPixels = fBitmap.getAddr(x, y);
+    const size_t srcRowBytes = fBitmap.rowBytes();
+
+    return copy_pixels(dstInfo, dstPixels, dstRowBytes, srcInfo, srcPixels, srcRowBytes);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkBitmapDevice::drawPaint(const SkDraw& draw, const SkPaint& paint) {
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 17e03c2..ef6a82d 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -667,6 +667,7 @@
     return device;
 }
 
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
 bool SkCanvas::readPixels(SkBitmap* bitmap,
                           int x, int y,
                           Config8888 config8888) {
@@ -676,28 +677,95 @@
     }
     return device->readPixels(bitmap, x, y, config8888);
 }
+#endif
+
+bool SkCanvas::readPixels(SkBitmap* bitmap, int x, int y) {
+    if (kUnknown_SkColorType == bitmap->colorType() || bitmap->getTexture()) {
+        return false;
+    }
+
+    bool weAllocated = false;
+    if (NULL == bitmap->pixelRef()) {
+        if (!bitmap->allocPixels()) {
+            return false;
+        }
+        weAllocated = true;
+    }
+
+    SkBitmap bm(*bitmap);
+    bm.lockPixels();
+    if (bm.getPixels() && this->readPixels(bm.info(), bm.getPixels(), bm.rowBytes(), x, y)) {
+        return true;
+    }
+
+    if (weAllocated) {
+        bitmap->setPixelRef(NULL);
+    }
+    return false;
+}
 
 bool SkCanvas::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) {
+    SkIRect r = srcRect;
+    const SkISize size = this->getBaseLayerSize();
+    if (!r.intersect(0, 0, size.width(), size.height())) {
+        bitmap->reset();
+        return false;
+    }
+
+    if (!bitmap->allocN32Pixels(r.width(), r.height())) {
+        // bitmap will already be reset.
+        return false;
+    }
+    if (!this->readPixels(bitmap->info(), bitmap->getPixels(), bitmap->rowBytes(), r.x(), r.y())) {
+        bitmap->reset();
+        return false;
+    }
+    return true;
+}
+
+bool SkCanvas::readPixels(const SkImageInfo& origInfo, void* dstP, size_t rowBytes, int x, int y) {
+    switch (origInfo.colorType()) {
+        case kUnknown_SkColorType:
+        case kIndex_8_SkColorType:
+            return false;
+        default:
+            break;
+    }
+    if (NULL == dstP || rowBytes < origInfo.minRowBytes()) {
+        return false;
+    }
+    if (0 == origInfo.width() || 0 == origInfo.height()) {
+        return false;
+    }
+
     SkBaseDevice* device = this->getDevice();
     if (!device) {
         return false;
     }
 
-    SkIRect bounds;
-    bounds.set(0, 0, device->width(), device->height());
-    if (!bounds.intersect(srcRect)) {
+    const SkISize size = this->getBaseLayerSize();
+    SkIRect srcR = SkIRect::MakeXYWH(x, y, origInfo.width(), origInfo.height());
+    if (!srcR.intersect(0, 0, size.width(), size.height())) {
         return false;
     }
-
-    SkBitmap tmp;
-    tmp.setConfig(SkBitmap::kARGB_8888_Config, bounds.width(),
-                                               bounds.height());
-    if (this->readPixels(&tmp, bounds.fLeft, bounds.fTop)) {
-        bitmap->swap(tmp);
-        return true;
-    } else {
-        return false;
+    
+    SkImageInfo info = origInfo;
+    // the intersect may have shrunk info's logical size
+    info.fWidth = srcR.width();
+    info.fHeight = srcR.height();
+    
+    // if x or y are negative, then we have to adjust pixels
+    if (x > 0) {
+        x = 0;
     }
+    if (y > 0) {
+        y = 0;
+    }
+    // here x,y are either 0 or negative
+    dstP = ((char*)dstP - y * rowBytes - x * info.bytesPerPixel());
+    
+    // The device can assert that the requested area is always contained in its bounds
+    return device->readPixels(info, dstP, rowBytes, srcR.x(), srcR.y());
 }
 
 bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
@@ -1063,12 +1131,9 @@
     fAddr = canvas->peekPixels(&fInfo, &fRowBytes);
     if (NULL == fAddr) {
         fInfo = canvas->imageInfo();
-        if (kUnknown_SkColorType == fInfo.colorType() ||
-            !fBitmap.allocPixels(fInfo))
-        {
+        if (kUnknown_SkColorType == fInfo.colorType() || !fBitmap.allocPixels(fInfo)) {
             return; // failure, fAddr is NULL
         }
-        fBitmap.lockPixels();
         if (!canvas->readPixels(&fBitmap, 0, 0)) {
             return; // failure, fAddr is NULL
         }
diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
index 5b6ecc0..61a7ab6 100644
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -112,6 +112,7 @@
     return bitmap;
 }
 
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
 bool SkBaseDevice::readPixels(SkBitmap* bitmap, int x, int y,
                               SkCanvas::Config8888 config8888) {
     if (SkBitmap::kARGB_8888_Config != bitmap->config() ||
@@ -154,6 +155,10 @@
     }
     return result;
 }
+bool SkBaseDevice::onReadPixels(const SkBitmap&, int x, int y, SkCanvas::Config8888) {
+    return false;
+}
+#endif
 
 SkSurface* SkBaseDevice::newSurface(const SkImageInfo&) { return NULL; }
 
@@ -171,6 +176,20 @@
     this->drawPath(draw, path, paint, preMatrix, pathIsMutable);
 }
 
+bool SkBaseDevice::readPixels(const SkImageInfo& info, void* dstP, size_t rowBytes, int x, int y) {
+#ifdef SK_DEBUG
+    SkASSERT(info.width() > 0 && info.height() > 0);
+    SkASSERT(dstP);
+    SkASSERT(rowBytes >= info.minRowBytes());
+    SkASSERT(x >= 0 && y >= 0);
+    
+    const SkImageInfo& srcInfo = this->imageInfo();
+    SkASSERT(x + info.width() <= srcInfo.width());
+    SkASSERT(y + info.height() <= srcInfo.height());
+#endif
+    return this->onReadPixels(info, dstP, rowBytes, x, y);
+}
+
 bool SkBaseDevice::writePixels(const SkImageInfo& info, const void* pixels, size_t rowBytes,
                                int x, int y) {
 #ifdef SK_DEBUG
@@ -190,7 +209,7 @@
     return false;
 }
 
-bool SkBaseDevice::onReadPixels(const SkBitmap&, int x, int y, SkCanvas::Config8888) {
+bool SkBaseDevice::onReadPixels(const SkImageInfo&, void*, size_t, int x, int y) {
     return false;
 }
 
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 26392ca..ac4c02a 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -343,6 +343,7 @@
 
 ///////////////////////////////////////////////////////////////////////////////
 
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
 namespace {
 GrPixelConfig config8888_to_grconfig_and_flags(SkCanvas::Config8888 config8888, uint32_t* flags) {
     switch (config8888) {
@@ -379,7 +380,7 @@
     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())));
-
+    
     SkAutoLockPixels alp(bitmap);
     GrPixelConfig config;
     uint32_t flags;
@@ -393,6 +394,25 @@
                                             bitmap.rowBytes(),
                                             flags);
 }
+#endif
+
+bool SkGpuDevice::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
+                               int x, int y) {
+    DO_DEFERRED_CLEAR();
+
+    // TODO: teach fRenderTarget to take ImageInfo directly to specify the src pixels
+    GrPixelConfig config = SkImageInfo2GrPixelConfig(dstInfo.colorType(), dstInfo.alphaType());
+    if (kUnknown_GrPixelConfig == config) {
+        return false;
+    }
+
+    uint32_t flags = 0;
+    if (kUnpremul_SkAlphaType == dstInfo.alphaType()) {
+        flags = GrContext::kUnpremul_PixelOpsFlag;
+    }
+    return fContext->readRenderTargetPixels(fRenderTarget, x, y, dstInfo.width(), dstInfo.height(),
+                                            config, dstPixels, dstRowBytes, flags);
+}
 
 bool SkGpuDevice::onWritePixels(const SkImageInfo& info, const void* pixels, size_t rowBytes,
                                 int x, int y) {
diff --git a/src/pdf/SkPDFDevice.cpp b/src/pdf/SkPDFDevice.cpp
index efd2b02..66ddccf 100644
--- a/src/pdf/SkPDFDevice.cpp
+++ b/src/pdf/SkPDFDevice.cpp
@@ -2310,10 +2310,12 @@
                                 &content.entry()->fContent);
 }
 
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
 bool SkPDFDevice::onReadPixels(const SkBitmap& bitmap, int x, int y,
                                SkCanvas::Config8888) {
     return false;
 }
+#endif
 
 bool SkPDFDevice::allowImageFilter(const SkImageFilter*) {
     return false;
diff --git a/src/utils/SkDeferredCanvas.cpp b/src/utils/SkDeferredCanvas.cpp
index 9051874..2e3f5bb 100644
--- a/src/utils/SkDeferredCanvas.cpp
+++ b/src/utils/SkDeferredCanvas.cpp
@@ -171,9 +171,12 @@
 
 protected:
     virtual const SkBitmap& onAccessBitmap() SK_OVERRIDE;
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
     virtual bool onReadPixels(const SkBitmap& bitmap,
                                 int x, int y,
                                 SkCanvas::Config8888 config8888) SK_OVERRIDE;
+#endif
+    virtual bool onReadPixels(const SkImageInfo&, void*, size_t, int x, int y) SK_OVERRIDE;
     virtual bool onWritePixels(const SkImageInfo&, const void*, size_t, int x, int y) SK_OVERRIDE;
 
     // The following methods are no-ops on a deferred device
@@ -506,12 +509,20 @@
     return this->immediateDevice()->newSurface(info);
 }
 
+#ifdef SK_SUPPORT_LEGACY_READPIXELSCONFIG
 bool SkDeferredDevice::onReadPixels(
     const SkBitmap& bitmap, int x, int y, SkCanvas::Config8888 config8888) {
     this->flushPendingCommands(kNormal_PlaybackMode);
     return fImmediateCanvas->readPixels(const_cast<SkBitmap*>(&bitmap),
                                                    x, y, config8888);
 }
+#endif
+
+bool SkDeferredDevice::onReadPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
+                                    int x, int y) {
+    this->flushPendingCommands(kNormal_PlaybackMode);
+    return fImmediateCanvas->readPixels(info, pixels, rowBytes, x, y);
+}
 
 class AutoImmediateDrawIfNeeded {
 public:
diff --git a/src/utils/SkGatherPixelRefsAndRects.h b/src/utils/SkGatherPixelRefsAndRects.h
index 1c9d6fc..894b8f0 100644
--- a/src/utils/SkGatherPixelRefsAndRects.h
+++ b/src/utils/SkGatherPixelRefsAndRects.h
@@ -291,12 +291,6 @@
     virtual const SkBitmap& onAccessBitmap() SK_OVERRIDE {
         return fEmptyBitmap;
     }
-    virtual bool onReadPixels(const SkBitmap& bitmap,
-                              int x, int y,
-                              SkCanvas::Config8888 config8888) SK_OVERRIDE {
-        NotSupported();
-        return false;
-    }
     virtual void lockPixels() SK_OVERRIDE { NothingToDo(); }
     virtual void unlockPixels() SK_OVERRIDE { NothingToDo(); }
     virtual bool allowImageFilter(const SkImageFilter*) SK_OVERRIDE { return false; }
diff --git a/src/utils/SkPictureUtils.cpp b/src/utils/SkPictureUtils.cpp
index 78d70ca..fc1611d 100644
--- a/src/utils/SkPictureUtils.cpp
+++ b/src/utils/SkPictureUtils.cpp
@@ -157,13 +157,6 @@
     }
 
 protected:
-    virtual bool onReadPixels(const SkBitmap& bitmap,
-                              int x, int y,
-                              SkCanvas::Config8888 config8888) SK_OVERRIDE {
-        not_supported();
-        return false;
-    }
-
     virtual void replaceBitmapBackendForRasterSurface(const SkBitmap&) SK_OVERRIDE {
         not_supported();
     }
diff --git a/tests/PremulAlphaRoundTripTest.cpp b/tests/PremulAlphaRoundTripTest.cpp
index 1a42a9c..af041ce 100644
--- a/tests/PremulAlphaRoundTripTest.cpp
+++ b/tests/PremulAlphaRoundTripTest.cpp
@@ -39,12 +39,11 @@
 typedef uint32_t (*PackUnpremulProc)(SkColor);
 
 const struct {
-    SkColorType             fColorType;
-    PackUnpremulProc        fPackProc;
-    SkCanvas::Config8888    fConfig8888;
+    SkColorType         fColorType;
+    PackUnpremulProc    fPackProc;
 } gUnpremul[] = {
-    { kRGBA_8888_SkColorType, pack_unpremul_rgba, SkCanvas::kRGBA_Unpremul_Config8888 },
-    { kBGRA_8888_SkColorType, pack_unpremul_bgra, SkCanvas::kBGRA_Unpremul_Config8888 },
+    { kRGBA_8888_SkColorType, pack_unpremul_rgba },
+    { kBGRA_8888_SkColorType, pack_unpremul_bgra },
 };
 
 static void fillCanvas(SkCanvas* canvas, SkColorType colorType, PackUnpremulProc proc) {
@@ -99,21 +98,23 @@
             }
             SkCanvas canvas(device);
 
-            SkBitmap readBmp1;
-            readBmp1.allocN32Pixels(256, 256);
-            SkBitmap readBmp2;
-            readBmp2.allocN32Pixels(256, 256);
-
             for (size_t upmaIdx = 0; upmaIdx < SK_ARRAY_COUNT(gUnpremul); ++upmaIdx) {
                 fillCanvas(&canvas, gUnpremul[upmaIdx].fColorType, gUnpremul[upmaIdx].fPackProc);
 
+                const SkImageInfo info = SkImageInfo::Make(256, 256, gUnpremul[upmaIdx].fColorType,
+                                                           kUnpremul_SkAlphaType);
+                SkBitmap readBmp1;
+                readBmp1.allocPixels(info);
+                SkBitmap readBmp2;
+                readBmp2.allocPixels(info);
+
                 readBmp1.eraseColor(0);
                 readBmp2.eraseColor(0);
 
-                canvas.readPixels(&readBmp1, 0, 0, gUnpremul[upmaIdx].fConfig8888);
+                canvas.readPixels(&readBmp1, 0, 0);
                 sk_tool_utils::write_pixels(&canvas, readBmp1, 0, 0, gUnpremul[upmaIdx].fColorType,
                                             kUnpremul_SkAlphaType);
-                canvas.readPixels(&readBmp2, 0, 0, gUnpremul[upmaIdx].fConfig8888);
+                canvas.readPixels(&readBmp2, 0, 0);
 
                 bool success = true;
                 for (int y = 0; y < 256 && success; ++y) {
diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp
index d14e989..0ed062e 100644
--- a/tests/ReadPixelsTest.cpp
+++ b/tests/ReadPixelsTest.cpp
@@ -60,43 +60,31 @@
     return SkPackARGB32(0xff, r, g , b);
 }
 
-static SkPMColor convertConfig8888ToPMColor(SkCanvas::Config8888 config8888,
-                                            uint32_t color,
-                                            bool* premul) {
-    const uint8_t* c = reinterpret_cast<uint8_t*>(&color);
+static SkPMColor convertToPMColor(SkColorType ct, SkAlphaType at, const uint32_t* addr,
+                                  bool* doUnpremul) {
+    *doUnpremul = (kUnpremul_SkAlphaType == at);
+
+    const uint8_t* c = reinterpret_cast<const uint8_t*>(addr);
     U8CPU a,r,g,b;
-    *premul = false;
-    switch (config8888) {
-        case SkCanvas::kNative_Premul_Config8888:
-            return color;
-        case SkCanvas::kNative_Unpremul_Config8888:
-            *premul = true;
-            a = SkGetPackedA32(color);
-            r = SkGetPackedR32(color);
-            g = SkGetPackedG32(color);
-            b = SkGetPackedB32(color);
-            break;
-        case SkCanvas::kBGRA_Unpremul_Config8888:
-            *premul = 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]);
+    switch (ct) {
+        case kBGRA_8888_SkColorType:
             b = static_cast<U8CPU>(c[0]);
-            break;
-        case SkCanvas::kRGBA_Unpremul_Config8888:
-            *premul = true; // fallthru
-        case SkCanvas::kRGBA_Premul_Config8888:
+            g = static_cast<U8CPU>(c[1]);
+            r = static_cast<U8CPU>(c[2]);
             a = static_cast<U8CPU>(c[3]);
+            break;
+        case kRGBA_8888_SkColorType:
             r = static_cast<U8CPU>(c[0]);
             g = static_cast<U8CPU>(c[1]);
             b = static_cast<U8CPU>(c[2]);
+            a = static_cast<U8CPU>(c[3]);
             break;
         default:
-            SkDEBUGFAIL("Unexpected Config8888");
+            SkDEBUGFAIL("Unexpected colortype");
             return 0;
     }
-    if (*premul) {
+
+    if (*doUnpremul) {
         r = SkMulDiv255Ceiling(r, a);
         g = SkMulDiv255Ceiling(g, a);
         b = SkMulDiv255Ceiling(b, a);
@@ -168,12 +156,14 @@
                       const SkBitmap& bitmap,
                       int x, int y,
                       bool checkCanvasPixels,
-                      bool checkBitmapPixels,
-                      SkCanvas::Config8888 config8888) {
-    SkASSERT(SkBitmap::kARGB_8888_Config == bitmap.config());
+                      bool checkBitmapPixels) {
+    SkASSERT(4 == bitmap.bytesPerPixel());
     SkASSERT(!bitmap.isNull());
     SkASSERT(checkCanvasPixels || checkBitmapPixels);
 
+    const SkColorType ct = bitmap.colorType();
+    const SkAlphaType at = bitmap.alphaType();
+
     int bw = bitmap.width();
     int bh = bitmap.height();
 
@@ -183,19 +173,18 @@
         clippedSrcRect.setEmpty();
     }
     SkAutoLockPixels alp(bitmap);
-    intptr_t pixels = reinterpret_cast<intptr_t>(bitmap.getPixels());
     for (int by = 0; by < bh; ++by) {
         for (int bx = 0; bx < bw; ++bx) {
             int devx = bx + srcRect.fLeft;
             int devy = by + srcRect.fTop;
 
-            uint32_t pixel = *reinterpret_cast<SkPMColor*>(pixels + by * bitmap.rowBytes() + bx * bitmap.bytesPerPixel());
+            const uint32_t* pixel = bitmap.getAddr32(bx, by);
 
             if (clippedSrcRect.contains(devx, devy)) {
                 if (checkCanvasPixels) {
                     SkPMColor canvasPixel = getCanvasColor(devx, devy);
                     bool didPremul;
-                    SkPMColor pmPixel = convertConfig8888ToPMColor(config8888, pixel, &didPremul);
+                    SkPMColor pmPixel = convertToPMColor(ct, at, pixel, &didPremul);
                     bool check;
                     REPORTER_ASSERT(reporter, check = checkPixel(pmPixel, canvasPixel, didPremul));
                     if (!check) {
@@ -203,8 +192,8 @@
                     }
                 }
             } else if (checkBitmapPixels) {
-                REPORTER_ASSERT(reporter, getBitmapColor(bx, by, bw) == pixel);
-                if (getBitmapColor(bx, by, bw) != pixel) {
+                REPORTER_ASSERT(reporter, getBitmapColor(bx, by, bw) == *pixel);
+                if (getBitmapColor(bx, by, bw) != *pixel) {
                     return false;
                 }
             }
@@ -228,8 +217,9 @@
     return static_cast<BitmapInit>(++x);
 }
 
-static void init_bitmap(SkBitmap* bitmap, const SkIRect& rect, BitmapInit init) {
-    SkImageInfo info = SkImageInfo::MakeN32Premul(rect.width(), rect.height());
+static void init_bitmap(SkBitmap* bitmap, const SkIRect& rect, BitmapInit init, SkColorType ct,
+                        SkAlphaType at) {
+    SkImageInfo info = SkImageInfo::Make(rect.width(), rect.height(), ct, at);
     size_t rowBytes = 0;
     bool alloc = true;
     switch (init) {
@@ -340,25 +330,22 @@
             SkCanvas canvas(device);
             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,
+            static const struct {
+                SkColorType fColorType;
+                SkAlphaType fAlphaType;
+            } gReadConfigs[] = {
+                { kRGBA_8888_SkColorType,   kPremul_SkAlphaType },
+                { kRGBA_8888_SkColorType,   kUnpremul_SkAlphaType },
+                { kBGRA_8888_SkColorType,   kPremul_SkAlphaType },
+                { kBGRA_8888_SkColorType,   kUnpremul_SkAlphaType },
             };
             for (size_t rect = 0; rect < SK_ARRAY_COUNT(testRects); ++rect) {
                 const SkIRect& srcRect = testRects[rect];
-                for (BitmapInit bmi = kFirstBitmapInit;
-                     bmi < kBitmapInitCnt;
-                     bmi = nextBMI(bmi)) {
+                for (BitmapInit bmi = kFirstBitmapInit; bmi < kBitmapInitCnt; bmi = nextBMI(bmi)) {
                     for (size_t c = 0; c < SK_ARRAY_COUNT(gReadConfigs); ++c) {
-                        SkCanvas::Config8888 config8888 = gReadConfigs[c];
                         SkBitmap bmp;
-                        init_bitmap(&bmp, srcRect, bmi);
+                        init_bitmap(&bmp, srcRect, bmi,
+                                    gReadConfigs[c].fColorType, gReadConfigs[c].fAlphaType);
 
                         // if the bitmap has pixels allocated before the readPixels,
                         // note that and fill them with pattern
@@ -367,9 +354,7 @@
                             fillBitmap(&bmp);
                         }
                         uint32_t idBefore = canvas.getDevice()->accessBitmap(false).getGenerationID();
-                        bool success =
-                            canvas.readPixels(&bmp, srcRect.fLeft,
-                                              srcRect.fTop, config8888);
+                        bool success = canvas.readPixels(&bmp, srcRect.fLeft, srcRect.fTop);
                         uint32_t idAfter = canvas.getDevice()->accessBitmap(false).getGenerationID();
 
                         // we expect to succeed when the read isn't fully clipped
@@ -382,7 +367,7 @@
 
                         if (success || startsWithPixels) {
                             checkRead(reporter, bmp, srcRect.fLeft, srcRect.fTop,
-                                      success, startsWithPixels, config8888);
+                                      success, startsWithPixels);
                         } else {
                             // if we had no pixels beforehand and the readPixels
                             // failed then our bitmap should still not have pixels
@@ -396,9 +381,10 @@
                     SkIRect clippedRect = DEV_RECT;
                     if (clippedRect.intersect(srcRect)) {
                         REPORTER_ASSERT(reporter, success);
+                        REPORTER_ASSERT(reporter, kPMColor_SkColorType == wkbmp.colorType());
+                        REPORTER_ASSERT(reporter, kPremul_SkAlphaType == wkbmp.alphaType());
                         checkRead(reporter, wkbmp, clippedRect.fLeft,
-                                  clippedRect.fTop, true, false,
-                                  SkCanvas::kNative_Premul_Config8888);
+                                  clippedRect.fTop, true, false);
                     } else {
                         REPORTER_ASSERT(reporter, !success);
                     }
diff --git a/tools/sk_tool_utils.cpp b/tools/sk_tool_utils.cpp
index 1197388..d13b0d8 100644
--- a/tools/sk_tool_utils.cpp
+++ b/tools/sk_tool_utils.cpp
@@ -2,6 +2,21 @@
 
 namespace sk_tool_utils {
 
+const char* colortype_name(SkColorType ct) {
+    switch (ct) {
+        case kUnknown_SkColorType:      return "Unknown";
+        case kAlpha_8_SkColorType:      return "Alpha_8";
+        case kIndex_8_SkColorType:      return "Index_8";
+        case kARGB_4444_SkColorType:    return "ARGB_4444";
+        case kRGB_565_SkColorType:      return "RGB_565";
+        case kRGBA_8888_SkColorType:    return "RGBA_8888";
+        case kBGRA_8888_SkColorType:    return "BGRA_8888";
+        default:
+            SkASSERT(false);
+            return "unexpected colortype";
+    }
+}
+
 void config8888_to_imagetypes(SkCanvas::Config8888 config, SkColorType* ct, SkAlphaType* at) {
     switch (config) {
         case SkCanvas::kNative_Premul_Config8888:
diff --git a/tools/sk_tool_utils.h b/tools/sk_tool_utils.h
index 7747a12..a450aa1 100644
--- a/tools/sk_tool_utils.h
+++ b/tools/sk_tool_utils.h
@@ -12,6 +12,8 @@
 #include "SkBitmap.h"
 
 namespace sk_tool_utils {
+    
+    const char* colortype_name(SkColorType);
 
     /**
      *  Return the colorType and alphaType that correspond to the specified Config8888