3 files changed, 75 insertions, 24 deletions

diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 9683654db6..760bab7d8a 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -456,8 +456,8 @@ Sk64 SkBitmap::getSafeSize64() const {
     return ComputeSafeSize64(getConfig(), fWidth, fHeight, fRowBytes);
 }
 
-bool SkBitmap::copyPixelsTo(void* const dst, size_t dstSize, int dstRowBytes)
-     const {
+bool SkBitmap::copyPixelsTo(void* const dst, size_t dstSize, 
+                            int dstRowBytes, bool preserveDstPad) const {
 
     if (dstRowBytes == -1)
         dstRowBytes = fRowBytes;
@@ -468,7 +468,7 @@ bool SkBitmap::copyPixelsTo(void* const dst, size_t dstSize, int dstRowBytes)
         dst == NULL || (getPixels() == NULL && pixelRef() == NULL))
         return false;
 
-    if (static_cast<uint32_t>(dstRowBytes) == fRowBytes) {
+    if (!preserveDstPad && static_cast<uint32_t>(dstRowBytes) == fRowBytes) {
         size_t safeSize = getSafeSize();
         if (safeSize > dstSize || safeSize == 0)
             return false;
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 3ea9a9c36b..da7aeb971a 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -550,24 +550,32 @@ SkDevice* SkCanvas::setBitmapDevice(const SkBitmap& bitmap) {
     return device;
 }
 
-bool SkCanvas::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) {
+bool SkCanvas::readPixels(SkBitmap* bitmap, int x, int y) {
     SkDevice* device = this->getDevice();
     if (!device) {
         return false;
     }
-    return device->readPixels(srcRect, bitmap);
+    return device->readPixels(bitmap, x, y);
 }
 
-//////////////////////////////////////////////////////////////////////////////
-
-bool SkCanvas::readPixels(SkBitmap* bitmap) {
+bool SkCanvas::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) {
     SkDevice* device = this->getDevice();
-    if (!device) {
+    

+    SkIRect bounds;

+    bounds.set(0, 0, device->width(), device->height());
+    if (!bounds.intersect(srcRect)) {

+        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;
     }
-    SkIRect bounds;
-    bounds.set(0, 0, device->width(), device->height());
-    return this->readPixels(bounds, bitmap);
 }
 
 void SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
index 18087ae04d..f5523bd9db 100644
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -102,27 +102,70 @@ void SkDevice::setMatrixClip(const SkMatrix& matrix, const SkRegion& region,
 
 ///////////////////////////////////////////////////////////////////////////////
 
-bool SkDevice::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) {
-    const SkBitmap& src = this->accessBitmap(false);
-
-    SkIRect bounds;
-    bounds.set(0, 0, src.width(), src.height());
-    if (!bounds.intersect(srcRect)) {
+bool SkDevice::readPixels(SkBitmap* bitmap, int x, int y) {
+    if (SkBitmap::kARGB_8888_Config != bitmap->config() ||
+        NULL != bitmap->getTexture()) {
         return false;
     }
 
-    SkBitmap subset;
-    if (!src.extractSubset(&subset, bounds)) {
+    const SkBitmap& src = this->accessBitmap(false);
+
+    SkIRect srcRect = SkIRect::MakeXYWH(x, y, bitmap->width(),
+                                              bitmap->height());
+    SkIRect devbounds = SkIRect::MakeWH(src.width(), src.height());
+    if (!srcRect.intersect(devbounds)) {
         return false;
     }
 
     SkBitmap tmp;
-    if (!subset.copyTo(&tmp, SkBitmap::kARGB_8888_Config)) {
-        return false;
+    SkBitmap* bmp;
+    if (bitmap->isNull()) {
+        tmp.setConfig(SkBitmap::kARGB_8888_Config, bitmap->width(),
+                                                   bitmap->height());
+        if (!tmp.allocPixels()) {
+            return false;
+        }
+        bmp = &tmp;
+    } else {
+        bmp = bitmap;
+    }
+
+    SkIRect subrect = srcRect;
+    subrect.offset(-x, -y);
+    SkBitmap bmpSubset;
+    bmp->extractSubset(&bmpSubset, subrect);
+
+    bool result = this->onReadPixels(&bmpSubset, srcRect.fLeft, srcRect.fTop);
+    if (result && bmp == &tmp) {
+        tmp.swap(*bitmap);
     }
+    return result;
+}
+
+bool SkDevice::onReadPixels(const SkBitmap* bitmap, int x, int y) {
+    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())));
+
+    SkIRect srcRect = SkIRect::MakeXYWH(x, y, bitmap->width(),
+                                              bitmap->height());
+    const SkBitmap& src = this->accessBitmap(false);
 
-    tmp.swap(*bitmap);
-    return true;
+    SkBitmap subset;
+    if (!src.extractSubset(&subset, srcRect)) {
+        return false;
+    }
+    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);
 }
 
 void SkDevice::writePixels(const SkBitmap& bitmap, int x, int y) {