Changes the SkCanvas::readPixels API. Allows caller to read into prealloced bitmap pixels. Changes how clipping to device bounds is handled.

Review URL: http://codereview.appspot.com/5307077/



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

9 files changed, 161 insertions, 80 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) {
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 7028c91147..3fc5d7d773 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -1637,15 +1637,16 @@ bool GrContext::readTexturePixels(GrTexture* texture,
     if (NULL != target) {
         return fGpu->readPixels(target,
                                 left, top, width, height, 
-                                config, buffer);
+                                config, buffer, 0);
     } else {
         return false;
     }
 }
 
 bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
-                                      int left, int top, int width, int height,
-                                      GrPixelConfig config, void* buffer) {
+                                       int left, int top, int width, int height,
+                                       GrPixelConfig config, void* buffer,
+                                       size_t rowBytes) {
     SK_TRACE_EVENT0("GrContext::readRenderTargetPixels");
     uint32_t flushFlags = 0;
     if (NULL == target) { 
@@ -1655,7 +1656,7 @@ bool GrContext::readRenderTargetPixels(GrRenderTarget* target,
     this->flush(flushFlags);
     return fGpu->readPixels(target,
                             left, top, width, height, 
-                            config, buffer);
+                            config, buffer, rowBytes);
 }
 
 void GrContext::writePixels(int left, int top, int width, int height,
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index f0808d394f..1fc8d47339 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -221,10 +221,12 @@ void GrGpu::forceRenderTargetFlush() {
 
 bool GrGpu::readPixels(GrRenderTarget* target,
                        int left, int top, int width, int height,
-                       GrPixelConfig config, void* buffer) {
+                       GrPixelConfig config, void* buffer,
+                       size_t rowBytes) {
 
     this->handleDirtyContext();
-    return this->onReadPixels(target, left, top, width, height, config, buffer);
+    return this->onReadPixels(target, left, top, width, height,
+                              config, buffer, rowBytes);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 9107554cd6..cf29ed7555 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -180,6 +180,8 @@ public:
      * @param height        height of rectangle to read in pixels.
      * @param config        the pixel config of the destination buffer
      * @param buffer        memory to read the rectangle into.
+     * @param rowBytes      the number of bytes between consecutive rows. Zero
+     *                      means rows are tightly packed.
      *
      * @return true if the read succeeded, false if not. The read can fail
      *              because of a unsupported pixel config or because no render
@@ -187,7 +189,7 @@ public:
      */
     bool readPixels(GrRenderTarget* renderTarget,
                     int left, int top, int width, int height,
-                    GrPixelConfig config, void* buffer);
+                    GrPixelConfig config, void* buffer, size_t rowBytes);
 
     const GrGpuStats& getStats() const;
     void resetStats();
@@ -321,7 +323,7 @@ protected:
     // overridden by API-specific derived class to perform the read pixels.
     virtual bool onReadPixels(GrRenderTarget* target,
                               int left, int top, int width, int height,
-                              GrPixelConfig, void* buffer) = 0;
+                              GrPixelConfig, void* buffer, size_t rowBytes) = 0;
 
     // called to program the vertex data, indexCount will be 0 if drawing non-
     // indexed geometry. The subclass may adjust the startVertex and/or
diff --git a/src/gpu/GrGpuGL.cpp b/src/gpu/GrGpuGL.cpp
index 78655929f3..dc4d78ac43 100644
--- a/src/gpu/GrGpuGL.cpp
+++ b/src/gpu/GrGpuGL.cpp
@@ -1384,14 +1384,18 @@ void GrGpuGL::onForceRenderTargetFlush() {
 }
 
 bool GrGpuGL::onReadPixels(GrRenderTarget* target,
-                           int left, int top, int width, int height,
-                           GrPixelConfig config, void* buffer) {
+                           int left, int top,
+                           int width, int height,
+                           GrPixelConfig config, 
+                           void* buffer, size_t rowBytes) {
     GrGLenum internalFormat;  // we don't use this for glReadPixels
     GrGLenum format;
     GrGLenum type;
     if (!this->canBeTexture(config, &internalFormat, &format, &type)) {
         return false;
-    }    
+    }
+    
+    // resolve the render target if necessary
     GrGLRenderTarget* tgt = static_cast<GrGLRenderTarget*>(target);
     GrAutoTPtrValueRestore<GrRenderTarget*> autoTargetRestore;
     switch (tgt->getResolveType()) {
@@ -1417,26 +1421,62 @@ bool GrGpuGL::onReadPixels(GrRenderTarget* target,
     // the read rect is viewport-relative
     GrGLIRect readRect;
     readRect.setRelativeTo(glvp, left, top, width, height);
+    
+    size_t tightRowBytes = GrBytesPerPixel(config) * width;
+    if (0 == rowBytes) {
+        rowBytes = tightRowBytes;
+    }
+    size_t readDstRowBytes = tightRowBytes;
+    void* readDst = buffer;
+    
+    // determine if GL can read using the passed rowBytes or if we need
+    // a scratch buffer.
+    SkAutoSMalloc<32 * sizeof(GrColor)> scratch;
+    if (rowBytes != tightRowBytes) {
+        if (kDesktop_GrGLBinding == this->glBinding()) {
+            GrAssert(!(rowBytes % sizeof(GrColor)));
+            GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH, rowBytes / sizeof(GrColor)));
+            readDstRowBytes = rowBytes;
+        } else {
+            scratch.reset(tightRowBytes * height);
+            readDst = scratch.get();
+        }
+    }
     GL_CALL(ReadPixels(readRect.fLeft, readRect.fBottom,
                        readRect.fWidth, readRect.fHeight,
-                       format, type, buffer));
+                       format, type, readDst));
+    if (readDstRowBytes != tightRowBytes) {
+        GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH, 0));
+    }
 
     // now reverse the order of the rows, since GL's are bottom-to-top, but our
-    // API presents top-to-bottom
-    {
-        size_t stride = width * GrBytesPerPixel(config);
-        SkAutoMalloc rowStorage(stride);
-        void* tmp = rowStorage.get();
-
+    // API presents top-to-bottom. We must preserve the padding contents. Note
+    // that the above readPixels did not overwrite the padding.
+    if (readDst == buffer) {
+        GrAssert(rowBytes == readDstRowBytes);
+        scratch.reset(tightRowBytes);
+        void* tmpRow = scratch.get();
+        // flip y in-place by rows
         const int halfY = height >> 1;
         char* top = reinterpret_cast<char*>(buffer);
-        char* bottom = top + (height - 1) * stride;
+        char* bottom = top + (height - 1) * rowBytes;
         for (int y = 0; y < halfY; y++) {
-            memcpy(tmp, top, stride);
-            memcpy(top, bottom, stride);
-            memcpy(bottom, tmp, stride);
-            top += stride;
-            bottom -= stride;
+            memcpy(tmpRow, top, tightRowBytes);
+            memcpy(top, bottom, tightRowBytes);
+            memcpy(bottom, tmpRow, tightRowBytes);
+            top += rowBytes;
+            bottom -= rowBytes;
+        }
+    } else {
+        GrAssert(readDst != buffer);
+        // copy from readDst to buffer while flipping y
+        const int halfY = height >> 1;
+        const char* src = reinterpret_cast<const char*>(readDst);
+        char* dst = reinterpret_cast<char*>(buffer) + (height-1) * rowBytes;
+        for (int y = 0; y < height; y++) {
+            memcpy(dst, src, tightRowBytes);
+            src += readDstRowBytes;
+            dst -= rowBytes;
         }
     }
     return true;
diff --git a/src/gpu/GrGpuGL.h b/src/gpu/GrGpuGL.h
index 2d246e8479..51eb4aed02 100644
--- a/src/gpu/GrGpuGL.h
+++ b/src/gpu/GrGpuGL.h
@@ -90,8 +90,10 @@ protected:
     virtual void onForceRenderTargetFlush();
 
     virtual bool onReadPixels(GrRenderTarget* target,
-                              int left, int top, int width, int height,
-                              GrPixelConfig, void* buffer);
+                              int left, int top, 
+                              int width, int height,
+                              GrPixelConfig, 
+                              void* buffer, size_t rowBytes) SK_OVERRIDE;
 
     virtual void onGpuDrawIndexed(GrPrimitiveType type,
                                   uint32_t startVertex,
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index d75838350e..790cf6d09e 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -256,36 +256,19 @@ void SkGpuDevice::makeRenderTargetCurrent() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-bool SkGpuDevice::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) {
-    SkIRect bounds;
-    bounds.set(0, 0, this->width(), this->height());
-    if (!bounds.intersect(srcRect)) {
-        return false;
-    }
-
-    const int w = bounds.width();
-    const int h = bounds.height();
-    SkBitmap tmp;
-    // note we explicitly specify our rowBytes to be snug (no gap between rows)
-    tmp.setConfig(SkBitmap::kARGB_8888_Config, w, h, w * 4);
-    if (!tmp.allocPixels()) {
-        return false;
-    }
-
-    tmp.lockPixels();
-
-    bool read = fContext->readRenderTargetPixels(fRenderTarget,
-                                                 bounds.fLeft, bounds.fTop,
-                                                 bounds.width(), bounds.height(),
-                                                 kRGBA_8888_GrPixelConfig,
-                                                 tmp.getPixels());
-    tmp.unlockPixels();
-    if (!read) {
-        return false;
-    }
-
-    tmp.swap(*bitmap);
-    return true;
+bool SkGpuDevice::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())));
+
+    SkAutoLockPixels alp(*bitmap);
+    return fContext->readRenderTargetPixels(fRenderTarget,
+                                            x, y,
+                                            bitmap->width(),
+                                            bitmap->height(),
+                                            kRGBA_8888_GrPixelConfig,
+                                            bitmap->getPixels(),
+                                            bitmap->rowBytes());
 }
 
 void SkGpuDevice::writePixels(const SkBitmap& bitmap, int x, int y) {