optimize the disable-caching case for SkImage::readPixels

If the client wants no caching, and we haven't already cached it, pass the
caller's dst-buffer directly down to the generator, avoiding the (previous)
extra memcpy.

BUG=skia:4594

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

diff --git a/gm/image.cpp b/gm/image.cpp
index 03ed105..991b401 100644 (file)
--- a/gm/image.cpp
+++ b/gm/image.cpp
@@ -214,7 +214,7 @@ static void show_scaled_pixels(SkCanvas* canvas, SkImage* image) {
     storage.alloc(info);
 
     const SkImage::CachingHint chints[] = {
-        SkImage::kAllow_CachingHint, // SkImage::kDisallow_CachingHint,
+        SkImage::kAllow_CachingHint, SkImage::kDisallow_CachingHint,
     };
     const SkFilterQuality qualities[] = {
         kNone_SkFilterQuality, kLow_SkFilterQuality, kMedium_SkFilterQuality, kHigh_SkFilterQuality,
@@ -285,7 +285,7 @@ protected:
         const SkImageInfo info = SkImageInfo::MakeN32Premul(100, 100);
 
         const ImageMakerProc procs[] = {
-            make_raster, make_picture, make_codec, make_gpu,
+            make_codec, make_raster, make_picture, make_codec, make_gpu,
         };
         for (auto& proc : procs) {
             SkAutoTUnref<SkImage> image(proc(info, canvas->getGrContext()));

diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp
index 1c97656..f4a2ab7 100644 (file)
--- a/src/core/SkImageCacherator.cpp
+++ b/src/core/SkImageCacherator.cpp
@@ -108,14 +108,28 @@ bool SkImageCacherator::generateBitmap(SkBitmap* bitmap) {
     }
 }
 
+bool SkImageCacherator::directGeneratePixels(const SkImageInfo& info, void* pixels, size_t rb,
+                                             int srcX, int srcY) {
+    ScopedGenerator generator(this);
+    const SkImageInfo& genInfo = generator->getInfo();
+    // Currently generators do not natively handle subsets, so check that first.
+    if (srcX || srcY || genInfo.width() != info.width() || genInfo.height() != info.height()) {
+        return false;
+    }
+    return generator->getPixels(info, pixels, rb);
+}
+
 //////////////////////////////////////////////////////////////////////////////////////////////////
 
+bool SkImageCacherator::lockAsBitmapOnlyIfAlreadyCached(SkBitmap* bitmap) {
+    return SkBitmapCache::Find(fUniqueID, bitmap) && check_output_bitmap(*bitmap, fUniqueID);
+}
+
 bool SkImageCacherator::tryLockAsBitmap(SkBitmap* bitmap, const SkImage* client,
                                         SkImage::CachingHint chint) {
-    if (SkBitmapCache::Find(fUniqueID, bitmap)) {
-        return check_output_bitmap(*bitmap, fUniqueID);
+    if (this->lockAsBitmapOnlyIfAlreadyCached(bitmap)) {
+        return true;
     }
-
     if (!this->generateBitmap(bitmap)) {
         return false;
     }

diff --git a/src/core/SkImageCacherator.h b/src/core/SkImageCacherator.h
index 5ba5422..bce3967 100644 (file)
--- a/src/core/SkImageCacherator.h
+++ b/src/core/SkImageCacherator.h
@@ -57,6 +57,12 @@ public:
      */
     SkData* refEncoded();
 
+    // Only return true if the generate has already been cached.
+    bool lockAsBitmapOnlyIfAlreadyCached(SkBitmap*);
+    // Call the underlying generator directly
+    bool directGeneratePixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
+                              int srcX, int srcY);
+
 private:
     SkImageCacherator(SkImageGenerator*, const SkImageInfo&, const SkIPoint&, uint32_t uniqueID);
 

diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index fdaadc5..be837f2 100644 (file)
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -57,6 +57,10 @@ bool SkImage::readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dst
 }
 
 bool SkImage::scalePixels(const SkPixmap& dst, SkFilterQuality quality, CachingHint chint) const {
+    if (this->width() == dst.width() && this->height() == dst.height()) {
+        return this->readPixels(dst, 0, 0, chint);
+    }
+
     // Idea: If/when SkImageGenerator supports a native-scaling API (where the generator itself
     //       can scale more efficiently) we should take advantage of it here.
     //

diff --git a/src/image/SkImage_Generator.cpp b/src/image/SkImage_Generator.cpp
index 5031c24..fbde92d 100644 (file)
--- a/src/image/SkImage_Generator.cpp
+++ b/src/image/SkImage_Generator.cpp
@@ -41,6 +41,20 @@ private:
 bool SkImage_Generator::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
                                      int srcX, int srcY, CachingHint chint) const {
     SkBitmap bm;
+    if (kDisallow_CachingHint == chint) {
+        if (fCache->lockAsBitmapOnlyIfAlreadyCached(&bm)) {
+            return bm.readPixels(dstInfo, dstPixels, dstRB, srcX, srcY);
+        } else {
+            // Try passing the caller's buffer directly down to the generator. If this fails we
+            // may still succeed in the general case, as the generator may prefer some other
+            // config, which we could then convert via SkBitmap::readPixels.
+            if (fCache->directGeneratePixels(dstInfo, dstPixels, dstRB, srcX, srcY)) {
+                return true;
+            }
+            // else fall through
+        }
+    }
+
     if (this->getROPixels(&bm, chint)) {
         return bm.readPixels(dstInfo, dstPixels, dstRB, srcX, srcY);
     }