Allow texture-backed bitmaps to perform a read-back when lockPixels is called.

This means we have to be even more cautious about when we call lock, and we should
always check getTexture() first if we can handle a texture directly, rather than
forcing the read-back to get the bits.



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

8 files changed, 127 insertions, 35 deletions

diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h
index 58af571b95..de3e83ea28 100644
--- a/include/core/SkBitmap.h
+++ b/include/core/SkBitmap.h
@@ -330,6 +330,14 @@ public:
     */
     void unlockPixels() const;
 
+    /**
+     *  Some bitmaps can return a copy of their pixels for lockPixels(), but
+     *  that copy, if modified, will not be pushed back. These bitmaps should
+     *  not be used as targets for a raster device/canvas (since all pixels
+     *  modifications will be lost when unlockPixels() is called.)
+     */
+    bool lockPixelsAreWritable() const;
+
     /** Call this to be sure that the bitmap is valid enough to be drawn (i.e.
         it has non-null pixels, and if required by its config, it has a
         non-null colortable. Returns true if all of the above are met.
@@ -713,17 +721,23 @@ private:
     void inval16BitCache();
 };
 
-class SkAutoLockPixels {
+class SkAutoLockPixels : public SkNoncopyable {
 public:
-    SkAutoLockPixels(const SkBitmap& bitmap) : fBitmap(bitmap) {
-        bitmap.lockPixels();
+    SkAutoLockPixels(const SkBitmap& bm, bool doLock = true) : fBitmap(bm) {
+        fDidLock = doLock;
+        if (doLock) {
+            bm.lockPixels();
+        }
     }
     ~SkAutoLockPixels() {
-        fBitmap.unlockPixels();
+        if (fDidLock) {
+            fBitmap.unlockPixels();
+        }
     }
 
 private:
     const SkBitmap& fBitmap;
+    bool            fDidLock;
 };
 
 /** Helper class that performs the lock/unlockColors calls on a colortable.
diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h
index 8fb368ac32..17cf6b4391 100644
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -66,6 +66,14 @@ public:
     */
     void unlockPixels();
 
+    /**
+     *  Some bitmaps can return a copy of their pixels for lockPixels(), but
+     *  that copy, if modified, will not be pushed back. These bitmaps should
+     *  not be used as targets for a raster device/canvas (since all pixels
+     *  modifications will be lost when unlockPixels() is called.)
+     */
+    bool lockPixelsAreWritable() const;
+
     /** Returns a non-zero, unique value corresponding to the pixels in this
         pixelref. Each time the pixels are changed (and notifyPixelsChanged is
         called), a different generation ID will be returned.
@@ -161,6 +169,9 @@ protected:
     */
     virtual void onUnlockPixels() = 0;
 
+    /** Default impl returns true */
+    virtual bool onLockPixelsAreWritable() const;
+
     /**
      *  For pixelrefs that don't have access to their raw pixels, they may be
      *  able to make a copy of them (e.g. if the pixels are on the GPU).
diff --git a/include/gpu/SkGrTexturePixelRef.h b/include/gpu/SkGrTexturePixelRef.h
index 5bc64f532a..2de842e3b4 100644
--- a/include/gpu/SkGrTexturePixelRef.h
+++ b/include/gpu/SkGrTexturePixelRef.h
@@ -18,36 +18,54 @@
 #ifndef SkGrTexturePixelRef_DEFINED
 #define SkGrTexturePixelRef_DEFINED
 
+#include "SkBitmap.h"
 #include "SkPixelRef.h"
 #include "GrGpu.h"
 
-class SkGrTexturePixelRef : public SkPixelRef {
+/**
+ *  Common baseclass that implements onLockPixels() by calling onReadPixels().
+ *  Since it has a copy, it always returns false for onLockPixelsAreWritable().
+ */
+class SkROLockPixelsPixelRef : public SkPixelRef {
+public:
+    SkROLockPixelsPixelRef();
+    virtual ~SkROLockPixelsPixelRef();
+
+protected:
+    // override from SkPixelRef
+    virtual void* onLockPixels(SkColorTable** ptr);
+    virtual void onUnlockPixels();
+    virtual bool onLockPixelsAreWritable() const;   // return false;
+
+private:
+    SkBitmap    fBitmap;
+    typedef SkPixelRef INHERITED;
+};
+
+/**
+ *  PixelRef that wraps a GrTexture
+ */
+class SkGrTexturePixelRef : public SkROLockPixelsPixelRef {
 public:
             SkGrTexturePixelRef(GrTexture*);
     virtual ~SkGrTexturePixelRef();
 
     // override from SkPixelRef
-    virtual SkGpuTexture* getTexture() { return (SkGpuTexture*)fTexture; }
+    virtual SkGpuTexture* getTexture();
 
 protected:
     // override from SkPixelRef
-    virtual void* onLockPixels(SkColorTable** ptr) {
-        if (ptr) {
-            *ptr = NULL;
-        }
-        return NULL;
-    }
-
-    // override from SkPixelRef
-    virtual void onUnlockPixels() {}
     virtual bool onReadPixels(SkBitmap* dst, const SkIRect* subset);
 
 private:
     GrTexture*  fTexture;
-    typedef SkPixelRef INHERITED;
+    typedef SkROLockPixelsPixelRef INHERITED;
 };
 
-class SkGrRenderTargetPixelRef : public SkPixelRef {
+/**
+ *  PixelRef that wraps a GrRenderTarget
+ */
+class SkGrRenderTargetPixelRef : public SkROLockPixelsPixelRef {
 public:
             SkGrRenderTargetPixelRef(GrRenderTarget* rt);
     virtual ~SkGrRenderTargetPixelRef();
@@ -57,20 +75,11 @@ public:
 
 protected:
     // override from SkPixelRef
-    virtual void* onLockPixels(SkColorTable** ptr) {
-        if (ptr) {
-            *ptr = NULL;
-        }
-        return NULL;
-    }
-
-    // override from SkPixelRef
-    virtual void onUnlockPixels() {}
     virtual bool onReadPixels(SkBitmap* dst, const SkIRect* subset);
 
 private:
     GrRenderTarget*  fRenderTarget;
-    typedef SkPixelRef INHERITED;
+    typedef SkROLockPixelsPixelRef INHERITED;
 };
 
 #endif
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index cdb74bb1d6..92ff88ae66 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -350,6 +350,14 @@ void SkBitmap::unlockPixels() const {
     SkDEBUGCODE(this->validate();)
 }
 
+bool SkBitmap::lockPixelsAreWritable() const {
+    if (fPixelRef) {
+        return fPixelRef->lockPixelsAreWritable();
+    } else {
+        return fPixels != NULL;
+    }
+}
+
 void SkBitmap::setPixels(void* p, SkColorTable* ctable) {
     this->freePixels();
     fPixels = p;
diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
index 78b2dcc2ef..b4f5866981 100644
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -57,11 +57,15 @@ SkMetaData& SkDevice::getMetaData() {
 }
 
 void SkDevice::lockPixels() {
-    fBitmap.lockPixels();
+    if (fBitmap.lockPixelsAreWritable()) {
+        fBitmap.lockPixels();
+    }
 }
 
 void SkDevice::unlockPixels() {
-    fBitmap.unlockPixels();
+    if (fBitmap.lockPixelsAreWritable()) {
+        fBitmap.unlockPixels();
+    }
 }
 
 const SkBitmap& SkDevice::accessBitmap(bool changePixels) {
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index 967a8721b9..f8508911e7 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -4,7 +4,8 @@
 
 static SkMutex  gPixelRefMutex;
 
-extern int32_t SkNextPixelRefGenerationID() {
+extern int32_t SkNextPixelRefGenerationID();
+int32_t SkNextPixelRefGenerationID() {
     static int32_t  gPixelRefGenerationID;
     // do a loop in case our global wraps around, as we never want to
     // return a 0
@@ -63,6 +64,14 @@ void SkPixelRef::unlockPixels() {
     }
 }
 
+bool SkPixelRef::lockPixelsAreWritable() const {
+    return this->onLockPixelsAreWritable();
+}
+
+bool SkPixelRef::onLockPixelsAreWritable() const {
+    return true;
+}
+
 uint32_t SkPixelRef::getGenerationID() const {
     if (0 == fGenerationID) {
         fGenerationID = SkNextPixelRefGenerationID();
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index b8cc2bb4ce..508ace0246 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -1088,8 +1088,9 @@ void SkGpuDevice::internalDrawBitmap(const SkDraw& draw,
     SkASSERT(bitmap.width() <= fContext->getMaxTextureSize() &&
              bitmap.height() <= fContext->getMaxTextureSize());
 
-    SkAutoLockPixels alp(bitmap);
+    SkAutoLockPixels alp(bitmap, !bitmap.getTexture());
     if (!bitmap.getTexture() && !bitmap.readyToDraw()) {
+        SkDebugf("nothing to draw\n");
         return;
     }
 
diff --git a/src/gpu/SkGrTexturePixelRef.cpp b/src/gpu/SkGrTexturePixelRef.cpp
index 8becfd42c7..4fc03ab156 100644
--- a/src/gpu/SkGrTexturePixelRef.cpp
+++ b/src/gpu/SkGrTexturePixelRef.cpp
@@ -16,11 +16,42 @@
 
 
 #include "SkGrTexturePixelRef.h"
-
 #include "GrTexture.h"
-
 #include "SkRect.h"
-#include "SkBitmap.h"
+
+// since we call lockPixels recursively on fBitmap, we need a distinct mutex,
+// to avoid deadlock with the default one provided by SkPixelRef.
+static SkMutex  gROLockPixelsPixelRefMutex;
+
+SkROLockPixelsPixelRef::SkROLockPixelsPixelRef() : INHERITED(&gROLockPixelsPixelRefMutex) {
+}
+
+SkROLockPixelsPixelRef::~SkROLockPixelsPixelRef() {
+}
+
+void* SkROLockPixelsPixelRef::onLockPixels(SkColorTable** ctable) {
+    if (ctable) {
+        *ctable = NULL;
+    }
+    fBitmap.reset();
+//    SkDebugf("---------- calling readpixels in support of lockpixels\n");
+    if (!this->onReadPixels(&fBitmap, NULL)) {
+        SkDebugf("SkROLockPixelsPixelRef::onLockPixels failed!\n");
+        return NULL;
+    }
+    fBitmap.lockPixels();
+    return fBitmap.getPixels();
+}
+
+void SkROLockPixelsPixelRef::onUnlockPixels() {
+    fBitmap.unlockPixels();
+}
+
+bool SkROLockPixelsPixelRef::onLockPixelsAreWritable() const {
+    return false;
+}
+
+///////////////////////////////////////////////////////////////////////////////
 
 SkGrTexturePixelRef::SkGrTexturePixelRef(GrTexture* tex) {
     fTexture = tex;
@@ -31,6 +62,10 @@ SkGrTexturePixelRef::~SkGrTexturePixelRef() {
     GrSafeUnref(fTexture);
 }
 
+SkGpuTexture* SkGrTexturePixelRef::getTexture() {
+    return (SkGpuTexture*)fTexture;
+}
+
 bool SkGrTexturePixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
     if (NULL != fTexture && fTexture->isValid()) {
         int left, top, width, height;
@@ -57,7 +92,7 @@ bool SkGrTexturePixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
     }
 }
 
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////
 
 SkGrRenderTargetPixelRef::SkGrRenderTargetPixelRef(GrRenderTarget* rt) {
     fRenderTarget = rt;
@@ -100,3 +135,4 @@ bool SkGrRenderTargetPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset
         return false;
     }
 }
+