Make peekPixels() usable with raster surface snapshots

SkSurface_Raster snapshots do not lock their backing bitmaps when the
pixel ref is shared - they only lock on deep-copy.

But since for raster surfaces the pixels are always in memory, I think
it would be OK to also lock in the former case.

This allows for optimized (zero-copy) reads of raster surface snapshot
data.

R=reed@google.com

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

7 files changed, 34 insertions, 15 deletions

diff --git a/include/core/SkPixmap.h b/include/core/SkPixmap.h
index 05c7101735..faae85e797 100644
--- a/include/core/SkPixmap.h
+++ b/include/core/SkPixmap.h
@@ -18,7 +18,7 @@ struct SkMask;
  *  Pairs SkImageInfo with actual pixels and rowbytes. This class does not try to manage the
  *  lifetime of the pixel memory (nor the colortable if provided).
  */
-class SkPixmap {
+class SK_API SkPixmap {
 public:
     SkPixmap()
         : fPixels(NULL), fCTable(NULL), fRowBytes(0), fInfo(SkImageInfo::MakeUnknown(0, 0))
@@ -151,7 +151,7 @@ private:
 
 /////////////////////////////////////////////////////////////////////////////////////////////
 
-class SkAutoPixmapStorage : public SkPixmap {
+class SK_API SkAutoPixmapStorage : public SkPixmap {
 public:
     SkAutoPixmapStorage();
     ~SkAutoPixmapStorage();
@@ -206,7 +206,7 @@ private:
 
 /////////////////////////////////////////////////////////////////////////////////////////////
 
-class SkAutoPixmapUnlock : ::SkNoncopyable {
+class SK_API SkAutoPixmapUnlock : ::SkNoncopyable {
 public:
     SkAutoPixmapUnlock() : fUnlockProc(NULL), fIsLocked(false) {}
     SkAutoPixmapUnlock(const SkPixmap& pm, void (*unlock)(void*), void* ctx)
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index ff0b8c9819..cde9d76172 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -254,7 +254,7 @@ SkImage* SkImage::NewFromBitmap(const SkBitmap& bm) {
 #endif
 
     // This will check for immutable (share or copy)
-    return SkNewImageFromRasterBitmap(bm, false, NULL);
+    return SkNewImageFromRasterBitmap(bm, false, nullptr, kUnlocked_SharedPixelRefMode);
 }
 
 bool SkImage::asLegacyBitmap(SkBitmap* bitmap, LegacyBitmapMode mode) const {
diff --git a/src/image/SkImagePriv.h b/src/image/SkImagePriv.h
index afaf3f1736..876c4279b6 100644
--- a/src/image/SkImagePriv.h
+++ b/src/image/SkImagePriv.h
@@ -19,9 +19,15 @@ extern SkImage* SkNewImageFromPixelRef(const SkImageInfo&, SkPixelRef*,
 
 /**
  *  Examines the bitmap to decide if it can share the existing pixelRef, or
- *  if it needs to make a deep-copy of the pixels. The bitmap's pixelref will
- *  be shared if either the bitmap is marked as immutable, or canSharePixelRef
- *  is true.
+ *  if it needs to make a deep-copy of the pixels.
+ *
+ *  The bitmap's pixelref will be shared if either the bitmap is marked as
+ *  immutable, or forceSharePixelRef is true.  Shared pixel refs are also
+ *  locked when kLocked_SharedPixelRefMode is specified.
+ *
+ *  Passing kLocked_SharedPixelRefMode allows the image's peekPixels() method
+ *  to succeed, but it will force any lazy decodes/generators to execute if
+ *  they exist on the pixelref.
  *
  *  It is illegal to call this with a texture-backed bitmap.
  *
@@ -29,8 +35,12 @@ extern SkImage* SkNewImageFromPixelRef(const SkImageInfo&, SkPixelRef*,
  *  SkImageInfo, or the bitmap's pixels cannot be accessed, this will return
  *  NULL.
  */
+enum SharedPixelRefMode {
+    kLocked_SharedPixelRefMode,
+    kUnlocked_SharedPixelRefMode
+};
 extern SkImage* SkNewImageFromRasterBitmap(const SkBitmap&, bool forceSharePixelRef,
-                                           const SkSurfaceProps*);
+                                           const SkSurfaceProps*, SharedPixelRefMode);
 
 static inline size_t SkImageMinRowBytes(const SkImageInfo& info) {
     size_t minRB = info.minRowBytes();
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index aac4995cfb..41198c09ed 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -81,9 +81,13 @@ public:
     bool isOpaque() const override;
     bool onAsLegacyBitmap(SkBitmap*, LegacyBitmapMode) const override;
 
-    SkImage_Raster(const SkBitmap& bm, const SkSurfaceProps* props)
+    SkImage_Raster(const SkBitmap& bm, const SkSurfaceProps* props, bool lockPixels = false)
         : INHERITED(bm.width(), bm.height(), props)
-        , fBitmap(bm) {}
+        , fBitmap(bm) {
+        if (lockPixels) {
+            fBitmap.lockPixels();
+        }
+    }
 
 private:
     SkImage_Raster() : INHERITED(0, 0, NULL) {}
@@ -229,7 +233,7 @@ SkImage* SkNewImageFromPixelRef(const SkImageInfo& info, SkPixelRef* pr,
 }
 
 SkImage* SkNewImageFromRasterBitmap(const SkBitmap& bm, bool forceSharePixelRef,
-                                    const SkSurfaceProps* props) {
+                                    const SkSurfaceProps* props, SharedPixelRefMode mode) {
     SkASSERT(NULL == bm.getTexture());
 
     if (!SkImage_Raster::ValidArgs(bm.info(), bm.rowBytes(), NULL, NULL)) {
@@ -238,7 +242,7 @@ SkImage* SkNewImageFromRasterBitmap(const SkBitmap& bm, bool forceSharePixelRef,
 
     SkImage* image = NULL;
     if (forceSharePixelRef || bm.isImmutable()) {
-        image = SkNEW_ARGS(SkImage_Raster, (bm, props));
+        image = SkNEW_ARGS(SkImage_Raster, (bm, props, kLocked_SharedPixelRefMode == mode));
     } else {
         SkBitmap tmp(bm);
         tmp.lockPixels();
diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp
index d66aed213a..d59aef7c75 100644
--- a/src/image/SkSurface_Raster.cpp
+++ b/src/image/SkSurface_Raster.cpp
@@ -118,7 +118,10 @@ void SkSurface_Raster::onDraw(SkCanvas* canvas, SkScalar x, SkScalar y,
 }
 
 SkImage* SkSurface_Raster::onNewImageSnapshot(Budgeted) {
-    return SkNewImageFromRasterBitmap(fBitmap, fWeOwnThePixels, &this->props());
+    // Our pixels are in memory, so read access on the snapshot SkImage could be cheap.
+    // Lock the shared pixel ref to ensure peekPixels() is usable.
+    return SkNewImageFromRasterBitmap(fBitmap, fWeOwnThePixels, &this->props(),
+                                      kLocked_SharedPixelRefMode);
 }
 
 void SkSurface_Raster::onCopyOnWrite(ContentChangeMode mode) {
diff --git a/tests/DeferredCanvasTest.cpp b/tests/DeferredCanvasTest.cpp
index a34f5bcedc..0aa0af68c1 100644
--- a/tests/DeferredCanvasTest.cpp
+++ b/tests/DeferredCanvasTest.cpp
@@ -64,7 +64,8 @@ public:
     }
 
     SkImage* onNewImageSnapshot(Budgeted) override {
-        return SkNewImageFromRasterBitmap(fBitmap, true, &this->props());
+        return SkNewImageFromRasterBitmap(fBitmap, true, &this->props(),
+                                          kUnlocked_SharedPixelRefMode);
     }
 
     void onCopyOnWrite(ContentChangeMode mode) override {
diff --git a/tests/SkImageTest.cpp b/tests/SkImageTest.cpp
index 2f23b3f661..508982cea6 100644
--- a/tests/SkImageTest.cpp
+++ b/tests/SkImageTest.cpp
@@ -26,7 +26,8 @@ DEF_TEST(SkImageFromBitmap_extractSubset, reporter) {
         canvas.drawIRect(r, p);
         SkBitmap dstBitmap;
         srcBitmap.extractSubset(&dstBitmap, r);
-        image.reset(SkNewImageFromRasterBitmap(dstBitmap, true, NULL));
+        image.reset(SkNewImageFromRasterBitmap(dstBitmap, true, NULL,
+                                               kUnlocked_SharedPixelRefMode));
     }
 
     SkBitmap tgt;