Add onDrawBitmapLattice(), avoid unnecessary bitmap->image copy

src/core/SkCanvas.cpp

Index: src/core/SkCanvas.cpp
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 0b0434dab895963f0132fcbabe7ca99d93b3e8a2..17294d8e861e64d5b8f88c8120d3177c5fe415f5 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -17,6 +17,7 @@
 #include "SkErrorInternals.h"
 #include "SkImage.h"
 #include "SkImage_Base.h"
+#include "SkImage_Raster.h"
 #include "SkImageFilter.h"
 #include "SkImageFilterCache.h"
 #include "SkLatticeIter.h"
@@ -1992,6 +1993,19 @@ void SkCanvas::drawImageNine(const SkImage* image, const SkIRect& center, const
     }
 }
 
+void SkCanvas::drawImageLattice(const SkImage* image, const Lattice& lattice, const SkRect& dst,

[msarett, 2016/08/03 15:07:57]

Just moved this.

+                                const SkPaint* paint) {
+    RETURN_ON_NULL(image);
+    if (dst.isEmpty()) {
+        return;
+    }
+    if (SkLatticeIter::Valid(image->width(), image->height(), lattice)) {
+        this->onDrawImageLattice(image, lattice, dst, paint);
+    } else {
+        this->drawImageRect(image, dst, paint);
+    }
+}
+
 void SkCanvas::drawBitmap(const SkBitmap& bitmap, SkScalar dx, SkScalar dy, const SkPaint* paint) {
     if (bitmap.drawsNothing()) {
         return;
@@ -2028,25 +2042,17 @@ void SkCanvas::drawBitmapNine(const SkBitmap& bitmap, const SkIRect& center, con
     } else {
         this->drawBitmapRect(bitmap, dst, paint);
     }
-
 }
 
 void SkCanvas::drawBitmapLattice(const SkBitmap& bitmap, const Lattice& lattice, const SkRect& dst,
                                  const SkPaint* paint) {
-    sk_sp<SkImage> image = SkImage::MakeFromBitmap(bitmap);
-    this->drawImageLattice(image.get(), lattice, dst, paint);
-}
-
-void SkCanvas::drawImageLattice(const SkImage* image, const Lattice& lattice, const SkRect& dst,
-                                const SkPaint* paint) {
-    RETURN_ON_NULL(image);
-    if (dst.isEmpty()) {
+    if (bitmap.drawsNothing() || dst.isEmpty()) {
         return;
     }
-    if (SkLatticeIter::Valid(image->width(), image->height(), lattice)) {
-        this->onDrawImageLattice(image, lattice, dst, paint);
+    if (SkLatticeIter::Valid(bitmap.width(), bitmap.height(), lattice)) {
+        this->onDrawBitmapLattice(bitmap, lattice, dst, paint);
     } else {
-        this->drawImageRect(image, dst, paint);
+        this->drawBitmapRect(bitmap, dst, paint);
     }
 }
 
@@ -2354,29 +2360,6 @@ void SkCanvas::onDrawImage(const SkImage* image, SkScalar x, SkScalar y, const S
     LOOPER_END
 }
 
-void SkCanvas::onDrawImageLattice(const SkImage* image, const Lattice& lattice, const SkRect& dst,
-                                  const SkPaint* paint) {
-    if (nullptr == paint || paint->canComputeFastBounds()) {
-        SkRect storage;
-        if (this->quickReject(paint ? paint->computeFastBounds(dst, &storage) : dst)) {
-            return;
-        }
-    }
-
-    SkLazyPaint lazy;
-    if (nullptr == paint) {
-        paint = lazy.init();
-    }
-
-    LOOPER_BEGIN(*paint, SkDrawFilter::kBitmap_Type, &dst)
-
-    while (iter.next()) {
-        iter.fDevice->drawImageLattice(iter, image, lattice, dst, looper.paint());
-    }
-
-    LOOPER_END
-}
-
 void SkCanvas::onDrawImageRect(const SkImage* image, const SkRect* src, const SkRect& dst,
                                const SkPaint* paint, SrcRectConstraint constraint) {
     TRACE_EVENT0("disabled-by-default-skia", "SkCanvas::drawImageRect()");
@@ -2547,6 +2530,35 @@ void SkCanvas::onDrawBitmapNine(const SkBitmap& bitmap, const SkIRect& center, c
     LOOPER_END
 }
 
+void SkCanvas::onDrawImageLattice(const SkImage* image, const Lattice& lattice, const SkRect& dst,

[msarett, 2016/08/03 15:07:57]

Also moved.

+                                  const SkPaint* paint) {
+    if (nullptr == paint || paint->canComputeFastBounds()) {
+        SkRect storage;
+        if (this->quickReject(paint ? paint->computeFastBounds(dst, &storage) : dst)) {
+            return;
+        }
+    }
+
+    SkLazyPaint lazy;
+    if (nullptr == paint) {
+        paint = lazy.init();
+    }
+
+    LOOPER_BEGIN(*paint, SkDrawFilter::kBitmap_Type, &dst)
+
+    while (iter.next()) {
+        iter.fDevice->drawImageLattice(iter, image, lattice, dst, looper.paint());
+    }
+
+    LOOPER_END
+}
+
+void SkCanvas::onDrawBitmapLattice(const SkBitmap& bitmap, const Lattice& lattice,

[msarett, 2016/08/03 15:07:57]

This is new.  Could we also use this implementation for SkPictureRecord and
SkRecorder?  Or do they need to copy when converting to SkImage?

[mtklein, 2016/08/03 15:11:51]

They need to copy if immutable (though I think SkPictureRecord will never
actually see a mutable bitmap today... they should all have been previously
snapped by SkRecorder).

[mtklein, 2016/08/03 15:12:19]

They need to copy if *mutable...

[msarett, 2016/08/03 15:49:23]

Gotcha, I think the Picture/Record impls are as they should be then.

+                                   const SkRect& dst, const SkPaint* paint) {
+    sk_sp<SkImage> image = sk_sp<SkImage>((SkImage*) new SkImage_Raster(bitmap, true));

[mtklein, 2016/08/03 15:11:51]

I think we spell this,

   sk_sp<SkImage> image = SkMakeImageFromRasterBitmap(bitmap,
kNever_ForceCopyMode)


Are we sure we know this is safe here?  I thought we had to be down in the
raster device before we know it's not going to be retained somehow (e.g. by PDF
or GPU devices).

[reed1, 2016/08/03 15:21:05]

I had suggested this point in the pipeline to commit to a volatile image, but
perhaps you're right. Not sure about pdf, but gpu is allowed to not look at the
image pixels until later. Maybe we need to push down further to the device level
virtual (i.e. add drawBitmapLattice to device), at least when the bitmap is not
immutable...

[msarett, 2016/08/03 15:49:23]

Changed to use SkMakeImageFromRasterBitmap.  Pushing bitmap virtual down to
device level.

+    this->onDrawImageLattice(image.get(), lattice, dst, paint);
+}
+
 class SkDeviceFilteredPaint {
 public:
     SkDeviceFilteredPaint(SkBaseDevice* device, const SkPaint& paint) {