Allow SkLazyPixelRef to use SkScaledImageCache

src/core/SkScaledImageCache.cpp

Index: src/core/SkScaledImageCache.cpp
diff --git a/src/core/SkScaledImageCache.cpp b/src/core/SkScaledImageCache.cpp
index 11a0ee448f9c7cd590ff81c3409afded24163d8d..480056fb596f769216e7eb93c1aeb31070b01ce4 100644
--- a/src/core/SkScaledImageCache.cpp
+++ b/src/core/SkScaledImageCache.cpp
@@ -61,6 +61,19 @@ struct Key {
         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;
@@ -174,6 +187,15 @@ SkScaledImageCache::~SkScaledImageCache() {
     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) {
@@ -185,15 +207,8 @@ SkScaledImageCache::Rec* SkScaledImageCache::findAndLock(const SkBitmap& orig,
 #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;
@@ -201,6 +216,33 @@ SkScaledImageCache::Rec* SkScaledImageCache::findAndLock(const SkBitmap& orig,
     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,
@@ -230,6 +272,30 @@ SkScaledImageCache::ID* SkScaledImageCache::findAndLockMip(const SkBitmap& orig,
     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,
@@ -459,6 +525,32 @@ static SkScaledImageCache* get_cache() {
     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,