Break up SkLazyPixelRef functionally into class hierarchy.

src/lazy/SkCachingPixelRef.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 "SkCachingPixelRef.h"
+
+SkCachingPixelRef::SkCachingPixelRef(SkScaledImageCache* cache)
+    : fErrorInDecoding(false)
+    , fCache(cache)
+    , fScaledCacheId(NULL)
+    , fBitmapSize(SkISize::Make(0, 0)) {
+    if (fCache == NULL) {

[scroggo, 2013/10/31 21:53:06]

NULL == fCache.

[hal.canary, 2013/11/01 03:29:24]

Done.

+        fCache = SkScaledImageCache::Instance();
+    }
+}
+SkCachingPixelRef::~SkCachingPixelRef() {

[scroggo, 2013/10/31 21:53:06]

Maybe assert that fScaledCacheId is NULL?

[hal.canary, 2013/11/01 03:29:24]

Done.

+}
+
+bool SkCachingPixelRef::updateInfo() {
+    if (fErrorInDecoding || !fBitmapSize.isZero()) {

[scroggo, 2013/10/31 21:53:06]

This block is painful for me to look at. I think it'd be clearer if you kept the
fErrorInDecoding and size checks separate.

[hal.canary, 2013/11/01 03:29:24]

Done.

+        return !fErrorInDecoding;
+    }
+    if (!this->onDecodeInfo(&fBitmapSize, &fBitmapConfig, &fBitmapAlphaType)) {
+        fErrorInDecoding = true;
+        return false;
+    }
+    SkASSERT(!fBitmapSize.isZero());
+    return true;
+}
+
+bool SkCachingPixelRef::configure(SkBitmap* bitmap) {
+    if (!this->updateInfo()) {
+        return false;
+    }
+    return bitmap->setConfig(fBitmapConfig, fBitmapSize.width(),
+                             fBitmapSize.height(), 0, fBitmapAlphaType);
+}
+
+void* SkCachingPixelRef::onLockPixels(SkColorTable** colorTable) {
+    (void)colorTable;
+    if (!this->updateInfo()) {
+        return NULL;
+    }
+    SkBitmap bitmap;
+
+    fScaledCacheId = fCache->findAndLock(this->getGenerationID(),
+                                         fBitmapSize.fWidth,
+                                         fBitmapSize.fHeight,
+                                         &bitmap);
+    if (fScaledCacheId != NULL) {
+        SkAutoLockPixels autoLockPixels(bitmap);
+        void* pixels = bitmap.getPixels();
+        SkASSERT(NULL != pixels);
+        // At this point, the autoLockPixels will unlockPixels()
+        // to remove bitmap's lock on the pixels.  We will then
+        // destroy bitmap.  The *only* guarantee that this pointer
+        // remains valid is the guarantee made by
+        // SkScaledImageCache that it will not destroy the *other*
+        // bitmap (SkScaledImageCache::Rec.fBitmap) that holds a
+        // reference to the concrete PixelRef while this record is
+        // locked.
+        return pixels;
+    } else {
+        // Cache has been purged, must re-decode.
+        if (!this->onDecodeInto(0, &bitmap)) {
+            return NULL;
+        }
+        SkAutoLockPixels autoLockPixels(bitmap);
+        void* pixels = bitmap.getPixels();
+        SkASSERT(pixels != NULL);
+        fScaledCacheId = fCache->addAndLock(this->getGenerationID(),
+                                            fBitmapSize.fWidth,
+                                            fBitmapSize.fHeight,
+                                            bitmap);
+        SkASSERT(fScaledCacheId != NULL);
+        return pixels;

[scroggo, 2013/10/31 21:53:06]

Similar comment why pixels is still valid?

[hal.canary, 2013/11/01 03:29:24]

I cleaned up the code to make this clear.

+    }
+}
+
+void SkCachingPixelRef::onUnlockPixels() {
+    if (!fErrorInDecoding && (NULL != fScaledCacheId)) {

[scroggo, 2013/10/31 21:53:06]

If fErrorInDecoding is false, is it ever correct to have a NULL fScaledCacheId
here?

[hal.canary, 2013/11/01 03:29:24]

I don't think so.  Let me make this clear.

+        fCache->unlock(fScaledCacheId);
+        fScaledCacheId = NULL;
+    }
+}
+
+bool SkCachingPixelRef::onDecodeInto(int pow2, SkBitmap* bitmap) {
+    SkBitmap tmp;
+    if (!(this->configure(&tmp) && tmp.allocPixels(NULL, NULL))) {
+        fErrorInDecoding = true;
+        return false;
+    }
+    if (!this->onDecode(tmp.getPixels(), tmp.rowBytes())) {
+        fErrorInDecoding = true;
+        return false;
+    }
+    *bitmap = tmp;
+    return true;
+}