Always make SkImageShaders in heap.

I made a couple of measurments, and it looks like any differences is
well below the noise threshold.

Just for the record run1: .9991 of baseline and run2 .9988 of baseline.
I was using top25 .skps as workload.

TBR=mtklein@google.com

Change-Id: If4fa06e5d5df72fb67dbb4bbb99c926f05765897
Reviewed-on: https://skia-review.googlesource.com/8341
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>

8 files changed, 19 insertions, 62 deletions

diff --git a/src/core/SkBitmapDevice.cpp b/src/core/SkBitmapDevice.cpp
index 2b5c37fa2e..45ec2b70ae 100644
--- a/src/core/SkBitmapDevice.cpp
+++ b/src/core/SkBitmapDevice.cpp
@@ -334,15 +334,13 @@ void SkBitmapDevice::drawBitmapRect(const SkDraw& draw, const SkBitmap& bitmap,
     // Since the shader need only live for our stack-frame, pass in a custom allocator. This
     // can save malloc calls, and signals to SkMakeBitmapShader to not try to copy the bitmap
     // if its mutable, since that precaution is not needed (give the short lifetime of the shader).
-    SkTBlitterAllocator allocator;
+
     // construct a shader, so we can call drawRect with the dst
     auto s = SkMakeBitmapShader(*bitmapPtr, SkShader::kClamp_TileMode, SkShader::kClamp_TileMode,
-                                &matrix, kNever_SkCopyPixelsMode, &allocator);
+                                &matrix, kNever_SkCopyPixelsMode);
     if (!s) {
         return;
     }
-    // we deliberately add a ref, since the allocator wants to be the last owner
-    s.get()->ref();
 
     SkPaint paintWithShader(paint);
     paintWithShader.setStyle(SkPaint::kFill_Style);
diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp
index 4308000457..4d1c5a4998 100644
--- a/src/core/SkDraw.cpp
+++ b/src/core/SkDraw.cpp
@@ -85,23 +85,10 @@ public:
     SkAutoBitmapShaderInstall(const SkBitmap& src, const SkPaint& paint,
                               const SkMatrix* localMatrix = nullptr)
             : fPaint(paint) /* makes a copy of the paint */ {
-        // TODO(herb): Move this over to SkArenaAlloc when arena alloc has a
-        // facility to return sk_sps.
+
         fPaint.setShader(SkMakeBitmapShader(src, SkShader::kClamp_TileMode,
                                             SkShader::kClamp_TileMode, localMatrix,
-                                            kNever_SkCopyPixelsMode,
-                                            &fAllocator));
-        // we deliberately left the shader with an owner-count of 2
-        fPaint.getShader()->ref();
-        SkASSERT(2 == fPaint.getShader()->getRefCnt());
-    }
-
-    ~SkAutoBitmapShaderInstall() {
-        // since fAllocator will destroy shader, we insist that owners == 2
-        SkASSERT(2 == fPaint.getShader()->getRefCnt());
-
-        fPaint.setShader(nullptr); // unref the shader by 1
-
+                                            kNever_SkCopyPixelsMode));
     }
 
     // return the new paint that has the shader applied
@@ -110,8 +97,6 @@ public:
 private:
     // copy of caller's paint (which we then modify)
     SkPaint             fPaint;
-    // Stores the shader.
-    SkTBlitterAllocator fAllocator;
 };
 #define SkAutoBitmapShaderInstall(...) SK_REQUIRE_LOCAL_VAR(SkAutoBitmapShaderInstall)
 
diff --git a/src/core/SkImagePriv.h b/src/core/SkImagePriv.h
index 601108665d..d6e58d3c11 100644
--- a/src/core/SkImagePriv.h
+++ b/src/core/SkImagePriv.h
@@ -30,8 +30,7 @@ typedef SkSmallAllocator<3, kSkBlitterContextSize> SkTBlitterAllocator;
 // If alloc is non-nullptr, it will be used to allocate the returned SkShader, and MUST outlive
 // the SkShader.
 sk_sp<SkShader> SkMakeBitmapShader(const SkBitmap& src, SkShader::TileMode, SkShader::TileMode,
-                                   const SkMatrix* localMatrix, SkCopyPixelsMode,
-                                   SkTBlitterAllocator* alloc);
+                                   const SkMatrix* localMatrix, SkCopyPixelsMode);
 
 /**
  *  Examines the bitmap to decide if it can share the existing pixelRef, or
@@ -51,8 +50,7 @@ sk_sp<SkShader> SkMakeBitmapShader(const SkBitmap& src, SkShader::TileMode, SkSh
  *  SkImageInfo, or the bitmap's pixels cannot be accessed, this will return
  *  nullptr.
  */
-extern sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap&, SkCopyPixelsMode,
-                                                  SkTBlitterAllocator* = nullptr);
+extern sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap&, SkCopyPixelsMode);
 
 // Given an image created from SkNewImageFromBitmap, return its pixelref. This
 // may be called to see if the surface and the image share the same pixelref,
diff --git a/src/core/SkShader.cpp b/src/core/SkShader.cpp
index 81aae116d5..cfdd8b946e 100644
--- a/src/core/SkShader.cpp
+++ b/src/core/SkShader.cpp
@@ -232,7 +232,7 @@ sk_sp<SkShader> SkShader::MakeBitmapShader(const SkBitmap& src, TileMode tmx, Ti
     if (localMatrix && !localMatrix->invert(nullptr)) {
         return nullptr;
     }
-    return SkMakeBitmapShader(src, tmx, tmy, localMatrix, kIfMutable_SkCopyPixelsMode, nullptr);
+    return SkMakeBitmapShader(src, tmx, tmy, localMatrix, kIfMutable_SkCopyPixelsMode);
 }
 
 sk_sp<SkShader> SkShader::MakePictureShader(sk_sp<SkPicture> src, TileMode tmx, TileMode tmy,
diff --git a/src/image/SkImageShader.cpp b/src/image/SkImageShader.cpp
index 64a09dd2dc..7e3874891c 100644
--- a/src/image/SkImageShader.cpp
+++ b/src/image/SkImageShader.cpp
@@ -97,23 +97,12 @@ static bool bitmap_is_too_big(int w, int h) {
 }
 
 sk_sp<SkShader> SkImageShader::Make(sk_sp<SkImage> image, TileMode tx, TileMode ty,
-                                    const SkMatrix* localMatrix,
-                                    SkTBlitterAllocator* allocator) {
-    SkShader* shader;
+                                    const SkMatrix* localMatrix) {
     if (!image || bitmap_is_too_big(image->width(), image->height())) {
-        if (nullptr == allocator) {
-            shader = new SkEmptyShader;
-        } else {
-            shader = allocator->createT<SkEmptyShader>();
-        }
+        return sk_make_sp<SkEmptyShader>();
     } else {
-        if (nullptr == allocator) {
-            shader = new SkImageShader(image, tx, ty, localMatrix);
-        } else {
-            shader = allocator->createT<SkImageShader>(image, tx, ty, localMatrix);
-        }
+        return sk_make_sp<SkImageShader>(image, tx, ty, localMatrix);
     }
-    return sk_sp<SkShader>(shader);
 }
 
 #ifndef SK_IGNORE_TO_STRING
@@ -197,15 +186,9 @@ sk_sp<GrFragmentProcessor> SkImageShader::asFragmentProcessor(const AsFPArgs& ar
 
 sk_sp<SkShader> SkMakeBitmapShader(const SkBitmap& src, SkShader::TileMode tmx,
                                    SkShader::TileMode tmy, const SkMatrix* localMatrix,
-                                   SkCopyPixelsMode cpm, SkTBlitterAllocator* allocator) {
-    // Until we learn otherwise, it seems that any caller that is passing an allocator must be
-    // assuming that the returned shader will have a stack-frame lifetime, so we assert that
-    // they are also asking for kNever_SkCopyPixelsMode. If that proves otherwise, we can remove
-    // or modify this assert.
-    SkASSERT(!allocator || (kNever_SkCopyPixelsMode == cpm));
-
-    return SkImageShader::Make(SkMakeImageFromRasterBitmap(src, cpm, allocator),
-                               tmx, tmy, localMatrix, allocator);
+                                   SkCopyPixelsMode cpm) {
+    return SkImageShader::Make(SkMakeImageFromRasterBitmap(src, cpm),
+                               tmx, tmy, localMatrix);
 }
 
 static sk_sp<SkFlattenable> SkBitmapProcShader_CreateProc(SkReadBuffer& buffer) {
diff --git a/src/image/SkImageShader.h b/src/image/SkImageShader.h
index ef5e87c66d..8960812a06 100644
--- a/src/image/SkImageShader.h
+++ b/src/image/SkImageShader.h
@@ -15,7 +15,7 @@
 class SkImageShader : public SkShader {
 public:
     static sk_sp<SkShader> Make(sk_sp<SkImage>, TileMode tx, TileMode ty,
-                                const SkMatrix* localMatrix, SkTBlitterAllocator* = nullptr);
+                                const SkMatrix* localMatrix);
 
     bool isOpaque() const override;
 
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index b287789ea8..0364fc6b2a 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -301,8 +301,7 @@ sk_sp<SkImage> SkImage::MakeFromRaster(const SkPixmap& pmap, RasterReleaseProc p
     return sk_make_sp<SkImage_Raster>(pmap.info(), std::move(data), pmap.rowBytes(), pmap.ctable());
 }
 
-sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap& bm, SkCopyPixelsMode cpm,
-                                           SkTBlitterAllocator* allocator) {
+sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap& bm, SkCopyPixelsMode cpm) {
     bool hasColorTable = false;
     if (kIndex_8_SkColorType == bm.colorType()) {
         SkAutoLockPixels autoLockPixels(bm);
@@ -313,23 +312,17 @@ sk_sp<SkImage> SkMakeImageFromRasterBitmap(const SkBitmap& bm, SkCopyPixelsMode
         return nullptr;
     }
 
-    sk_sp<SkImage> image;
     if (kAlways_SkCopyPixelsMode == cpm || (!bm.isImmutable() && kNever_SkCopyPixelsMode != cpm)) {
         SkBitmap tmp(bm);
         tmp.lockPixels();
         SkPixmap pmap;
         if (tmp.getPixels() && tmp.peekPixels(&pmap)) {
-            image = SkImage::MakeRasterCopy(pmap);
+            return SkImage::MakeRasterCopy(pmap);
         }
     } else {
-        if (allocator) {
-            image.reset(allocator->createT<SkImage_Raster>(bm, kNever_SkCopyPixelsMode == cpm));
-            image.get()->ref(); // account for the allocator being an owner
-        } else {
-            image = sk_make_sp<SkImage_Raster>(bm, kNever_SkCopyPixelsMode == cpm);
-        }
+        return sk_make_sp<SkImage_Raster>(bm, kNever_SkCopyPixelsMode == cpm);
     }
-    return image;
+    return sk_sp<SkImage>();
 }
 
 const SkPixelRef* SkBitmapImageGetPixelRef(const SkImage* image) {
diff --git a/src/xps/SkXPSDevice.cpp b/src/xps/SkXPSDevice.cpp
index 8dca7f0156..f32ca86d92 100644
--- a/src/xps/SkXPSDevice.cpp
+++ b/src/xps/SkXPSDevice.cpp
@@ -2274,7 +2274,7 @@ void SkXPSDevice::drawBitmapRect(const SkDraw& draw,
 
     auto bitmapShader = SkMakeBitmapShader(bitmap, SkShader::kClamp_TileMode,
                                            SkShader::kClamp_TileMode, &matrix,
-                                           kNever_SkCopyPixelsMode, nullptr);
+                                           kNever_SkCopyPixelsMode);
     SkASSERT(bitmapShader);
     if (!bitmapShader) { return; }
     SkPaint paintWithShader(paint);