Picture shader resource caching.

src/core/SkPictureShader.cpp

Index: src/core/SkPictureShader.cpp
diff --git a/src/core/SkPictureShader.cpp b/src/core/SkPictureShader.cpp
index 8a16ea3b0074c3221d73a683b935fb5d6783aa5b..462200878cd9c21d545121cae6c94e03eeb1f0b7 100644
--- a/src/core/SkPictureShader.cpp
+++ b/src/core/SkPictureShader.cpp
@@ -10,14 +10,119 @@
 #include "SkBitmap.h"
 #include "SkBitmapProcShader.h"
 #include "SkCanvas.h"
+#include "SkDiscardableMemory.h"
 #include "SkMatrixUtils.h"
 #include "SkPicture.h"
 #include "SkReadBuffer.h"
+#include "SkResourceCache.h"
+#include "SkThread.h"
 
 #if SK_SUPPORT_GPU
 #include "GrContext.h"
 #endif
 
+struct BitmapShaderKey : public SkResourceCache::Key {
+public:
+    BitmapShaderKey(uint32_t pictureID,
+                    const SkRect& tile,
+                    SkShader::TileMode tmx,
+                    SkShader::TileMode tmy,
+                    const SkSize& scale,
+                    const SkMatrix& localMatrix)
+        : fPictureID(pictureID)
+        , fTile(tile)
+        , fTmx(tmx)
+        , fTmy(tmy)
+        , fScale(scale)
+        , fLocalMatrix(localMatrix) {
+
+        static const size_t keySize = sizeof(fPictureID) +
+                                      sizeof(fTile) +
+                                      sizeof(fTmx) + sizeof(fTmy) +
+                                      sizeof(fScale) +
+                                      sizeof(fLocalMatrix);
+        // This better be packed.
+        SkASSERT(sizeof(uint32_t) * (&fEndOfStruct - &fPictureID) == keySize);
+        this->init(keySize);
+    }
+
+private:
+    uint32_t           fPictureID;
+    SkRect             fTile;
+    SkShader::TileMode fTmx, fTmy;
+    SkSize             fScale;
+    SkMatrix           fLocalMatrix;
+
+    SkDEBUGCODE(uint32_t fEndOfStruct;)
+};
+
+struct BitmapShaderRec : public SkResourceCache::Rec {
+    BitmapShaderRec(const BitmapShaderKey& key, SkShader* tileShader, size_t bitmapBytes)
+        : fKey(key)
+        , fShader(SkRef(tileShader))
+        , fBitmapBytes(bitmapBytes) {}
+
+    BitmapShaderKey        fKey;
+    SkAutoTUnref<SkShader> fShader;
+    size_t                 fBitmapBytes;
+
+    virtual const Key& getKey() const SK_OVERRIDE { return fKey; }
+    virtual size_t bytesUsed() const SK_OVERRIDE {
+        return sizeof(fKey) + sizeof(SkShader) + fBitmapBytes;
+    }
+
+    static bool Visitor(const SkResourceCache::Rec& baseRec, void* contextShader) {
+        const BitmapShaderRec& rec = static_cast<const BitmapShaderRec&>(baseRec);
+        SkAutoTUnref<SkShader>* result = reinterpret_cast<SkAutoTUnref<SkShader>*>(contextShader);
+
+        result->reset(SkRef(rec.fShader.get()));
+        return true;
+    }
+};
+
+// FIXME: there's considerable boilerplate/duplication here vs. the global resource cache.

[mtklein, 2014/10/21 17:27:45]

Why aren't we using the global cache?

+SK_DECLARE_STATIC_MUTEX(gBitmapShaderCacheMutex);
+static SkResourceCache* gBitmapShaderCache = NULL;
+
+#ifndef SK_DEFAULT_TILE_CACHE_LIMIT
+    #define SK_DEFAULT_TILE_CACHE_LIMIT     (2 * 1024 * 1024)
+#endif
+
+static void cleanup_cache() {
+    // We'll clean this up in our own tests, but disable for clients.
+    // Chrome seems to have funky multi-process things going on in unit tests that
+    // makes this unsafe to delete when the main process atexit()s.
+    // SkLazyPtr does the same sort of thing.
+#if SK_DEVELOPER
+    SkDELETE(gBitmapShaderCache);
+#endif
+}
+
+/** Must hold gBitmapShaderCacheMutex when calling. */
+static SkResourceCache* cache() {
+    // gTileCacheMutex is always held when this is called, so we don't need to be fancy in here.
+    gBitmapShaderCacheMutex.assertHeld();
+    if (NULL == gBitmapShaderCache) {
+#ifdef SK_USE_DISCARDABLE_SCALEDIMAGECACHE
+        gBitmapShaderCache = SkNEW_ARGS(SkResourceCache, (SkDiscardableMemory::Create));
+#else
+        gBitmapShaderCache = SkNEW_ARGS(SkResourceCache, (SK_DEFAULT_TILE_CACHE_LIMIT));
+#endif
+        atexit(cleanup_cache);
+    }
+    return gBitmapShaderCache;
+}
+
+static bool cache_find(const BitmapShaderKey& key, SkAutoTUnref<SkShader>* result) {
+    SkAutoMutexAcquire am(gBitmapShaderCacheMutex);
+    return cache()->find(key, BitmapShaderRec::Visitor, result);
+}
+
+static void cache_add(BitmapShaderRec* rec) {
+    SkAutoMutexAcquire am(gBitmapShaderCacheMutex);
+    cache()->add(rec);
+}
+
 SkPictureShader::SkPictureShader(const SkPicture* picture, TileMode tmx, TileMode tmy,
                                  const SkMatrix* localMatrix, const SkRect* tile)
     : INHERITED(localMatrix)
@@ -103,9 +208,15 @@ SkShader* SkPictureShader::refBitmapShader(const SkMatrix& matrix, const SkMatri
     SkSize tileScale = SkSize::Make(SkIntToScalar(tileSize.width()) / fTile.width(),
                                     SkIntToScalar(tileSize.height()) / fTile.height());
 
-    SkAutoMutexAcquire ama(fCachedBitmapShaderMutex);
+    SkAutoTUnref<SkShader> tileShader;
+    BitmapShaderKey key(fPicture->uniqueID(),
+                        fTile,
+                        fTmx,
+                        fTmy,
+                        tileScale,
+                        this->getLocalMatrix());
 
-    if (!fCachedBitmapShader || tileScale != fCachedTileScale) {
+    if (!cache_find(key, &tileShader)) {
         SkBitmap bm;
         if (!bm.tryAllocN32Pixels(tileSize.width(), tileSize.height())) {

[reed1, 2014/10/21 20:28:00]

need to use the cache's GetAllocator to ensure we're using discardable memory
when that is the cache's preference.

[f(malita), 2014/10/21 23:49:24]

Done.

             return NULL;
@@ -117,18 +228,14 @@ SkShader* SkPictureShader::refBitmapShader(const SkMatrix& matrix, const SkMatri
         canvas.translate(fTile.x(), fTile.y());
         canvas.drawPicture(fPicture);
 
-        fCachedTileScale = tileScale;
-
         SkMatrix shaderMatrix = this->getLocalMatrix();
         shaderMatrix.preScale(1 / tileScale.width(), 1 / tileScale.height());
-        fCachedBitmapShader.reset(CreateBitmapShader(bm, fTmx, fTmy, &shaderMatrix));
+        tileShader.reset(CreateBitmapShader(bm, fTmx, fTmy, &shaderMatrix));
+
+        cache_add(SkNEW_ARGS(BitmapShaderRec, (key, tileShader.get(), bm.getSize())));
     }
 
-    // Increment the ref counter inside the mutex to ensure the returned pointer is still valid.
-    // Otherwise, the pointer may have been overwritten on a different thread before the object's
-    // ref count was incremented.
-    fCachedBitmapShader.get()->ref();
-    return fCachedBitmapShader;
+    return tileShader.detach();
 }
 
 size_t SkPictureShader::contextSize() const {