move readPixels from bitmap -> pixmap

BUG=skia:
TBR=

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

include/core/SkPixmap.h

@@ -44,7 +44,8 @@ public:
     SkAlphaType alphaType() const { return fInfo.alphaType(); }
     bool isOpaque() const { return fInfo.isOpaque(); }
 
-    int64_t getSafeSize64() const { return fInfo.getSafeSize64(fRowBytes); }
+    uint64_t getSize64() const { return sk_64_mul(fInfo.height(), fRowBytes); }
+    uint64_t getSafeSize64() const { return fInfo.getSafeSize64(fRowBytes); }
     size_t getSafeSize() const { return fInfo.getSafeSize(fRowBytes); }
 
     const uint32_t* addr32() const {
@@ -94,6 +95,20 @@ public:
         return const_cast<uint8_t*>(this->addr8(x, y));
     }
 
+    // copy methods
+
+    bool readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
+                    int srcX, int srcY) const;
+    bool readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes) const {
+        return this->readPixels(dstInfo, dstPixels, dstRowBytes, 0, 0);
+    }
+    bool readPixels(const SkPixmap& dst, int srcX, int srcY) const {
+        return this->readPixels(dst.info(), dst.writable_addr(), dst.rowBytes(), srcX, srcY);
+    }
+    bool readPixels(const SkPixmap& dst) const {
+        return this->readPixels(dst.info(), dst.writable_addr(), dst.rowBytes(), 0, 0);
+    }
+
 private:
     const void*     fPixels;
     SkColorTable*   fCTable;

src/core/SkBitmap.cpp

@@ -882,50 +882,13 @@ bool SkBitmap::canCopyTo(SkColorType dstColorType) const {
     return true;
 }
 
-#include "SkConfig8888.h"
-
 bool SkBitmap::readPixels(const SkImageInfo& requestedDstInfo, void* dstPixels, size_t dstRB,
                           int x, int y) const {
-    if (kUnknown_SkColorType == requestedDstInfo.colorType()) {
+    SkAutoPixmapUnlock src;
+    if (!this->requestLock(&src)) {
         return false;
     }
-    if (NULL == dstPixels || dstRB < requestedDstInfo.minRowBytes()) {
+    return src.pixmap().readPixels(requestedDstInfo, dstPixels, dstRB, x, y);
-        return false;
-    }
-    if (0 == requestedDstInfo.width() || 0 == requestedDstInfo.height()) {
-        return false;
-    }
-
-    SkIRect srcR = SkIRect::MakeXYWH(x, y, requestedDstInfo.width(), requestedDstInfo.height());
-    if (!srcR.intersect(0, 0, this->width(), this->height())) {
-        return false;
-    }
-
-    // the intersect may have shrunk info's logical size
-    const SkImageInfo dstInfo = requestedDstInfo.makeWH(srcR.width(), 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
-    dstPixels = ((char*)dstPixels - y * dstRB - x * dstInfo.bytesPerPixel());
-
-    //////////////
-
-    SkAutoPixmapUnlock result;
-    if (!this->requestLock(&result)) {
-        return false;
-    }
-    const SkPixmap& pmap = result.pixmap();
-    const SkImageInfo srcInfo = pmap.info().makeWH(dstInfo.width(), dstInfo.height());
-
-    const void* srcPixels = pmap.addr(srcR.x(), srcR.y());
-    return SkPixelInfo::CopyPixels(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, pmap.rowBytes(),
-                                   pmap.ctable());
 }
 
 bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc) const {
@@ -965,17 +928,13 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc)
         }
     }
 
-    // we lock this now, since we may need its colortable
+    SkAutoPixmapUnlock srcUnlocker;
-    SkAutoLockPixels srclock(*src);
+    if (!src->requestLock(&srcUnlocker)) {
-    if (!src->readyToDraw()) {
         return false;
     }
+    const SkPixmap& srcPM = srcUnlocker.pixmap();
 
-    // The only way to be readyToDraw is if fPixelRef is non NULL.
+    const SkImageInfo dstInfo = srcPM.info().makeColorType(dstColorType);
-    SkASSERT(fPixelRef != NULL);
-
-    const SkImageInfo dstInfo = src->info().makeColorType(dstColorType);
-
     SkBitmap tmpDst;
     if (!tmpDst.setInfo(dstInfo)) {
         return false;
@@ -984,22 +943,18 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc)
     // allocate colortable if srcConfig == kIndex8_Config
     SkAutoTUnref<SkColorTable> ctable;
     if (dstColorType == kIndex_8_SkColorType) {
-        ctable.reset(SkRef(src->getColorTable()));
+        ctable.reset(SkRef(srcPM.ctable()));
     }
     if (!tmpDst.tryAllocPixels(alloc, ctable)) {
         return false;
     }
 
-    if (!tmpDst.readyToDraw()) {
+    SkAutoPixmapUnlock dstUnlocker;
-        // allocator/lock failed
+    if (!tmpDst.requestLock(&dstUnlocker)) {
         return false;
     }
 
-    // pixelRef must be non NULL or tmpDst.readyToDraw() would have
+    if (!srcPM.readPixels(dstUnlocker.pixmap())) {
-    // returned false.
-    SkASSERT(tmpDst.pixelRef() != NULL);
-
-    if (!src->readPixels(tmpDst.info(), tmpDst.getPixels(), tmpDst.rowBytes(), 0, 0)) {
         return false;
     }
 
@@ -1009,7 +964,7 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc)
     //  TODO: should we ignore rowbytes (i.e. getSize)? Then it could just be
     //      if (src_pixelref->info == dst_pixelref->info)
     //
-    if (src->colorType() == dstColorType && tmpDst.getSize() == src->getSize()) {
+    if (srcPM.colorType() == dstColorType && tmpDst.getSize() == srcPM.getSize64()) {
         SkPixelRef* dstPixelRef = tmpDst.pixelRef();
         if (dstPixelRef->info() == fPixelRef->info()) {
             dstPixelRef->cloneGenID(*fPixelRef);

src/core/SkConfig8888.h

@@ -10,6 +10,8 @@
 
 #include "SkImageInfo.h"
 
+class SkColorTable;
+
 struct SkPixelInfo {
     SkColorType fColorType;
     SkAlphaType fAlphaType;

src/core/SkPixmap.cpp

@@ -5,6 +5,7 @@
  * found in the LICENSE file.
  */
 
+#include "SkConfig8888.h"
 #include "SkPixmap.h"
 
 void SkAutoPixmapUnlock::reset(const SkPixmap& pm, void (*unlock)(void*), void* ctx) {
@@ -16,3 +17,41 @@ void SkAutoPixmapUnlock::reset(const SkPixmap& pm, void (*unlock)(void*), void*
     fUnlockContext = ctx;
     fIsLocked = true;
 }
+
+/////////////////////////////////////////////////////////////////////////////////////////////////
+
+bool SkPixmap::readPixels(const SkImageInfo& requestedDstInfo, void* dstPixels, size_t dstRB,
+                          int x, int y) const {
+    if (kUnknown_SkColorType == requestedDstInfo.colorType()) {
+        return false;
+    }
+    if (NULL == dstPixels || dstRB < requestedDstInfo.minRowBytes()) {
+        return false;
+    }
+    if (0 == requestedDstInfo.width() || 0 == requestedDstInfo.height()) {
+        return false;
+    }
+    
+    SkIRect srcR = SkIRect::MakeXYWH(x, y, requestedDstInfo.width(), requestedDstInfo.height());
+    if (!srcR.intersect(0, 0, this->width(), this->height())) {
+        return false;
+    }
+    
+    // the intersect may have shrunk info's logical size
+    const SkImageInfo dstInfo = requestedDstInfo.makeWH(srcR.width(), 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
+    dstPixels = ((char*)dstPixels - y * dstRB - x * dstInfo.bytesPerPixel());
+
+    const SkImageInfo srcInfo = this->info().makeWH(dstInfo.width(), dstInfo.height());
+    const void* srcPixels = this->addr(srcR.x(), srcR.y());
+    return SkPixelInfo::CopyPixels(dstInfo, dstPixels, dstRB,
+                                   srcInfo, srcPixels, this->rowBytes(), this->ctable());
+}