Allow SkLazyPixelRef to use SkScaledImageCache

src/core/SkScaledImageCache.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkScaledImageCache.h"
 #include "SkMipMap.h"
 #include "SkPixelRef.h"
@@ skipping 43 matching lines @@
 
         fGenID = pr->getGenerationID();
         fBounds.set(x, y, x + bm.width(), y + bm.height());
         fScaleX = scaleX;
         fScaleY = scaleY;
 
         fHash = compute_hash(&fGenID, 7);
         return true;
     }
 
+    bool init(int32_t width,

[mtklein, 2013/10/23 18:50:14]

Can't hurt to have init take its parameters in the same order as your new
methods?

[hal.canary, 2013/10/23 22:57:41]

The parameters are in the order of the key's fields.

+              int32_t height,
+              uint32_t genID,
+              SkScalar scaleX,
+              SkScalar scaleY) {
+        fBounds.set(0, 0, width, height);
+        fGenID = genID;
+        fScaleX = scaleX;
+        fScaleY = scaleY;
+        fHash = compute_hash(&fGenID, 7);
+        return true;

[mtklein, 2013/10/23 18:50:14]

I'm skeptical of methods that  always return true.  --> void?

+    }
+
     bool operator<(const Key& other) const {
         const uint32_t* a = &fGenID;
         const uint32_t* b = &other.fGenID;
         for (int i = 0; i < 7; ++i) {
             if (a[i] < b[i]) {
                 return true;
             }
             if (a[i] > b[i]) {
                 return false;
             }
@@ skipping 93 matching lines @@
 SkScaledImageCache::~SkScaledImageCache() {
     Rec* rec = fHead;
     while (rec) {
         Rec* next = rec->fNext;
         SkDELETE(rec);
         rec = next;
     }
     delete fHash;
 }
 
+static inline SkScaledImageCache::Rec* find_rec_in_list(
+        SkScaledImageCache::Rec* head, const Key & key) {
+    SkScaledImageCache::Rec* rec = head;
+    while ((rec != NULL) && !(rec->fKey == key)) {
+        rec = rec->fNext;
+    }
+    return rec;
+}
+
 SkScaledImageCache::Rec* SkScaledImageCache::findAndLock(const SkBitmap& orig,
                                                         SkScalar scaleX,
                                                         SkScalar scaleY) {
     Key key;
     if (!key.init(orig, scaleX, scaleY)) {
         return NULL;
     }
 
 #ifdef USE_HASH
     Rec* rec = fHash->find(key);
 #else
-    Rec* rec = fHead;
-    while (rec != NULL) {
-        if (rec->fKey == key) {
-            break;
-        }
-        rec = rec->fNext;
-    }
+    Rec* rec = find_rec_in_list(fHead, key);
 #endif
-
     if (rec) {
         this->moveToHead(rec);  // for our LRU
         rec->fLockCount += 1;
     }
     return rec;
 }
 
+
+SkScaledImageCache::ID* SkScaledImageCache::findAndLock(
+                                uint32_t pixelGenerationID,
+                                int32_t width,
+                                int32_t height,
+                                SkScalar scaleX,  // SK_Scalar1 default
+                                SkScalar scaleY,
+                                SkBitmap* scaled) {
+    Key key;
+    if (!key.init(width, height, pixelGenerationID, scaleX, scaleY)) {
+        return NULL;
+    }
+#ifdef USE_HASH

[mtklein, 2013/10/23 18:50:14]

It would make me feel warmer and fuzzier if you found a way to factor out more
of this common code rather than duplicating it.  As it is I'm left wondering if
it's exactly the same and keep moving my head back and forth.  

[scroggo, 2013/10/23 23:38:33]

+1. It also means that if we fix a bug/modify behavior in one of them we do it
in both.

+    Rec* rec = fHash->find(key);
+#else
+    Rec* rec = find_rec_in_list(fHead, key);
+#endif
+    if (rec) {
+        this->moveToHead(rec);  // for our LRU
+        rec->fLockCount += 1;
+        SkASSERT(NULL == rec->fMip);
+        SkASSERT(rec->fBitmap.pixelRef());
+        *scaled = rec->fBitmap;
+    }
+    return reinterpret_cast<ID*>(rec);

[mtklein, 2013/10/23 18:50:14]

Most of these are using (ID*) to do the cast.  Should we be using
reinterpret_cast everywhere?  What exactly is the relationship between Rec*,
intptr_t, ID* and the other ID*?

+}
+
 SkScaledImageCache::ID* SkScaledImageCache::findAndLock(const SkBitmap& orig,
                                                         SkScalar scaleX,
                                                         SkScalar scaleY,
                                                         SkBitmap* scaled) {
     if (0 == scaleX || 0 == scaleY) {
         // degenerate, and the key we use for mipmaps
         return NULL;
     }
 
     Rec* rec = this->findAndLock(orig, scaleX, scaleY);
     if (rec) {
         SkASSERT(NULL == rec->fMip);
         SkASSERT(rec->fBitmap.pixelRef());
         *scaled = rec->fBitmap;
     }
     return (ID*)rec;
 }
 
 SkScaledImageCache::ID* SkScaledImageCache::findAndLockMip(const SkBitmap& orig,
                                                            SkMipMap const ** mip) {
     Rec* rec = this->findAndLock(orig, 0, 0);
     if (rec) {
         SkASSERT(rec->fMip);
         SkASSERT(NULL == rec->fBitmap.pixelRef());
         *mip = rec->fMip;
     }
     return (ID*)rec;
 }
 
+SkScaledImageCache::ID* SkScaledImageCache::addAndLock(
+                               uint32_t pixelGenerationID,
+                               int32_t width,
+                               int32_t height,
+                               SkScalar scaleX,
+                               SkScalar scaleY,
+                               const SkBitmap& scaled) {
+    Key key;
+    if (!key.init(width, height, pixelGenerationID, scaleX, scaleY)) {
+        return NULL;
+    }
+    Rec* rec = SkNEW_ARGS(Rec, (key, scaled));
+    this->addToHead(rec);
+    SkASSERT(1 == rec->fLockCount);
+
+#ifdef USE_HASH
+    fHash->add(rec);
+#endif
+
+    // We may (now) be overbudget, so see if we need to purge something.
+    this->purgeAsNeeded();
+    return reinterpret_cast<ID*>(rec);
+}
+
 SkScaledImageCache::ID* SkScaledImageCache::addAndLock(const SkBitmap& orig,
                                                        SkScalar scaleX,
                                                        SkScalar scaleY,
                                                        const SkBitmap& scaled) {
     if (0 == scaleX || 0 == scaleY) {
         // degenerate, and the key we use for mipmaps
         return NULL;
     }
 
     Key key;
@@ skipping 202 matching lines @@
 }
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "SkThread.h"
 
 SK_DECLARE_STATIC_MUTEX(gMutex);
 
 static SkScaledImageCache* get_cache() {
     static SkScaledImageCache* gCache;

[scroggo, 2013/10/23 15:27:49]

Doesn't need to be in this CL, but is this a candidate for sk_once?

[hal.canary, 2013/10/23 16:11:52]

I think so.  I meant to ask mtklein.  I'm not strong on these sorts of data
structures.

[mtklein, 2013/10/23 18:50:14]

Yep, perfect place for it.

#include "SkOnce.h"

static void create_cache(SkScaledImageCache** cache) {
  *cache = SkNEW_ARGS(SkScaledImageCache, (SK_DEFAULT_IMAGE_CACHE_LIMIT));
}

static SkScaledImageCache* get_cache() {
  static SkScaledImageCache* gCache = NULL;
  SK_DECLARE_STATIC_ONCE(once);
  SkOnce(&once, create_cache, &gCache);
  SkASSERT(NULL != gCache);
  return gCache;
}

[hal.canary, 2013/10/23 22:57:41]

Done.

     if (!gCache) {
         gCache = SkNEW_ARGS(SkScaledImageCache, (SK_DEFAULT_IMAGE_CACHE_LIMIT));
     }
     return gCache;
 }
 
+
+SkScaledImageCache::ID* SkScaledImageCache::FindAndLock(
+                                uint32_t pixelGenerationID,
+                                int32_t width,
+                                int32_t height,
+                                SkScalar scaleX,
+                                SkScalar scaleY,
+                                SkBitmap* scaled) {
+    SkAutoMutexAcquire am(gMutex);
+    return get_cache()->findAndLock(pixelGenerationID, width, height,
+                                    scaleX, scaleY, scaled);
+}
+
+SkScaledImageCache::ID* SkScaledImageCache::AddAndLock(
+                               uint32_t pixelGenerationID,
+                               int32_t width,
+                               int32_t height,
+                               SkScalar scaleX,
+                               SkScalar scaleY,
+                               const SkBitmap& scaled) {
+    SkAutoMutexAcquire am(gMutex);
+    return get_cache()->addAndLock(pixelGenerationID, width, height,
+                                   scaleX, scaleY, scaled);
+}
+
+
 SkScaledImageCache::ID* SkScaledImageCache::FindAndLock(const SkBitmap& orig,
                                                         SkScalar scaleX,
                                                         SkScalar scaleY,
                                                         SkBitmap* scaled) {
     SkAutoMutexAcquire am(gMutex);
     return get_cache()->findAndLock(orig, scaleX, scaleY, scaled);
 }
 
 SkScaledImageCache::ID* SkScaledImageCache::FindAndLockMip(const SkBitmap& orig,
                                                        SkMipMap const ** mip) {
@@ skipping 43 matching lines @@
     return SkScaledImageCache::GetBytesUsed();
 }
 
 size_t SkGraphics::GetImageCacheByteLimit() {
     return SkScaledImageCache::GetByteLimit();
 }
 
 size_t SkGraphics::SetImageCacheByteLimit(size_t newLimit) {
     return SkScaledImageCache::SetByteLimit(newLimit);
 }