detect if the scaledimagecache returns a purged bitmap

src/core/SkBitmapProcState.cpp

 
 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 #include "SkBitmapProcState.h"
 #include "SkColorPriv.h"
 #include "SkFilterProc.h"
@@ skipping 88 matching lines @@
 
     v1.fX = mat.getScaleX();
     v1.fY = mat.getSkewY();
 
     v2.fX = mat.getSkewX();
     v2.fY = mat.getScaleY();
 
     return SkMaxScalar(v1.lengthSqd(), v2.lengthSqd());
 }
 
+class AutoScaledCacheUnlocker {

[scroggo, 2013/12/12 20:34:55]

Maybe not necessary, since this is hidden in a source file, but should this use
SK_REQUIRE_LOCAL_VAR?

[reed1, 2013/12/12 21:13:49]

Done.

+public:
+    AutoScaledCacheUnlocker(SkScaledImageCache::ID** idPtr) : fIDPtr(idPtr) {}
+    ~AutoScaledCacheUnlocker() {
+        if (fIDPtr && *fIDPtr) {
+            SkScaledImageCache::Unlock(*fIDPtr);
+            *fIDPtr = NULL;
+        }
+    }
+
+    void keepLock() {

[scroggo, 2013/12/12 20:34:55]

How about reset? That keeps it consistent with our other Auto classes. I also
think it's a clearer name (since "reset" applies to the class who owns it, where
as "keepLock" refers to its private member).

[hal.canary, 2013/12/12 20:48:43]

it's more of a detach() than a reset()

[reed1, 2013/12/12 21:13:49]

How about release() -- release, since detach usually returns something, which
isn't the case here.

+        fIDPtr = NULL;
+    }
+
+private:
+    SkScaledImageCache::ID** fIDPtr;
+};
+
 // TODO -- we may want to pass the clip into this function so we only scale
 // the portion of the image that we're going to need.  This will complicate
 // the interface to the cache, but might be well worth it.
 
 bool SkBitmapProcState::possiblyScaleImage() {
+    AutoScaledCacheUnlocker unlocker(&fScaledCacheID);
+
     SkASSERT(NULL == fBitmap);
     SkASSERT(NULL == fScaledCacheID);
 
     if (fFilterLevel <= SkPaint::kLow_FilterLevel) {
         return false;
     }
 
     // Check to see if the transformation matrix is simple, and if we're
     // doing high quality scaling.  If so, do the bitmap scale here and
     // remove the scaling component from the matrix.
 
     if (SkPaint::kHigh_FilterLevel == fFilterLevel &&
         fInvMatrix.getType() <= (SkMatrix::kScale_Mask | SkMatrix::kTranslate_Mask) &&
         fOrigBitmap.config() == SkBitmap::kARGB_8888_Config) {
 
         SkScalar invScaleX = fInvMatrix.getScaleX();
         SkScalar invScaleY = fInvMatrix.getScaleY();
 
         fScaledCacheID = SkScaledImageCache::FindAndLock(fOrigBitmap,
                                                          invScaleX, invScaleY,
                                                          &fScaledBitmap);
+        if (fScaledCacheID) {
+            fScaledBitmap.lockPixels();
+            if (!fScaledBitmap.getPixels()) {
+                fScaledBitmap.unlockPixels();
+                // found a purged entry (discardablememory?), release it
+                SkScaledImageCache::Unlock(fScaledCacheID);
+                fScaledCacheID = NULL;
+                // fall through to rebuild
+            }
+        }
+
         if (NULL == fScaledCacheID) {
             int dest_width  = SkScalarCeilToInt(fOrigBitmap.width() / invScaleX);
             int dest_height = SkScalarCeilToInt(fOrigBitmap.height() / invScaleY);
 
             // All the criteria are met; let's make a new bitmap.
 
             SkConvolutionProcs simd;
             sk_bzero(&simd, sizeof(simd));
             this->platformConvolutionProcs(&simd);
 
             if (!SkBitmapScaler::Resize(&fScaledBitmap,
                                         fOrigBitmap,
                                         SkBitmapScaler::RESIZE_BEST,
                                         dest_width,
                                         dest_height,
                                         simd,
                                         SkScaledImageCache::GetAllocator())) {
                 // we failed to create fScaledBitmap, so just return and let
                 // the scanline proc handle it.
                 return false;
 
             }
+            SkASSERT(NULL != fScaledBitmap.getPixels());
             fScaledCacheID = SkScaledImageCache::AddAndLock(fOrigBitmap,
                                                             invScaleX,
                                                             invScaleY,
                                                             fScaledBitmap);
-        }
-        fScaledBitmap.lockPixels(); // wonder if Resize() should have locked this
-        if (!fScaledBitmap.getPixels()) {
-            // TODO: find out how this can happen, and add a unittest to exercise
-            // inspired by BUG=chromium:295895
-            return false;
+            if (!fScaledCacheID) {
+                fScaledBitmap.reset();
+                return false;
+            }
+            SkASSERT(NULL != fScaledBitmap.getPixels());
         }
 
+        SkASSERT(NULL != fScaledBitmap.getPixels());
         fBitmap = &fScaledBitmap;
 
         // set the inv matrix type to translate-only;
         fInvMatrix.setTranslate(fInvMatrix.getTranslateX() / fInvMatrix.getScaleX(),
                                 fInvMatrix.getTranslateY() / fInvMatrix.getScaleY());
 
         // no need for any further filtering; we just did it!
         fFilterLevel = SkPaint::kNone_FilterLevel;
+        unlocker.keepLock();
         return true;
     }
 
     /*
      *  If High, then our special-case for scale-only did not take, and so we
      *  have to make a choice:
      *      1. fall back on mipmaps + bilerp
      *      2. fall back on scanline bicubic filter
      *  For now, we compute the "scale" value from the matrix, and have a
      *  threshold to decide when bicubic is better, and when mips are better.
@@ skipping 54 matching lines @@
             if (mip->extractLevel(levelScale, &level)) {
                 SkScalar invScaleFixup = level.fScale;
                 fInvMatrix.postScale(invScaleFixup, invScaleFixup);
 
                 fScaledBitmap.setConfig(fOrigBitmap.config(),
                                         level.fWidth, level.fHeight,
                                         level.fRowBytes);
                 fScaledBitmap.setPixels(level.fPixels);
                 fBitmap = &fScaledBitmap;
                 fFilterLevel = SkPaint::kLow_FilterLevel;
+                unlocker.keepLock();
                 return true;
             }
         }
     }
 
     return false;
 }
 
 static bool get_locked_pixels(const SkBitmap& src, int pow2, SkBitmap* dst) {
     SkPixelRef* pr = src.pixelRef();
     if (pr && pr->decodeInto(pow2, dst)) {
         return true;
     }
 
     /*
      *  If decodeInto() fails, it is possibe that we have an old subclass that
      *  does not, or cannot, implement that. In that case we fall back to the
      *  older protocol of having the pixelRef handle the caching for us.
      */
     *dst = src;
     dst->lockPixels();
     return SkToBool(dst->getPixels());
 }
 
 bool SkBitmapProcState::lockBaseBitmap() {
+    AutoScaledCacheUnlocker unlocker(&fScaledCacheID);
+
     SkPixelRef* pr = fOrigBitmap.pixelRef();
 
+    SkASSERT(NULL == fScaledCacheID);
+
     if (pr->isLocked() || !pr->implementsDecodeInto()) {
         // fast-case, no need to look in our cache
         fScaledBitmap = fOrigBitmap;
+        fScaledBitmap.lockPixels();
+        SkASSERT(NULL != fScaledBitmap.getPixels());
     } else {
         fScaledCacheID = SkScaledImageCache::FindAndLock(fOrigBitmap,
                                                          SK_Scalar1, SK_Scalar1,
                                                          &fScaledBitmap);
+        if (fScaledCacheID) {
+            fScaledBitmap.lockPixels();
+            if (!fScaledBitmap.getPixels()) {
+                fScaledBitmap.unlockPixels();
+                // found a purged entry (discardablememory?), release it
+                SkScaledImageCache::Unlock(fScaledCacheID);
+                fScaledCacheID = NULL;
+                // fall through to rebuild
+            }
+        }
+
         if (NULL == fScaledCacheID) {
             if (!get_locked_pixels(fOrigBitmap, 0, &fScaledBitmap)) {
                 return false;
             }
 
             // TODO: if fScaled comes back at a different width/height than fOrig,
             // we need to update the matrix we are using to sample from this guy.
 
             fScaledCacheID = SkScaledImageCache::AddAndLock(fOrigBitmap,
                                                             SK_Scalar1, SK_Scalar1,
                                                             fScaledBitmap);
             if (!fScaledCacheID) {
                 fScaledBitmap.reset();
                 return false;
             }
         }
     }
-    fScaledBitmap.lockPixels(); // just 'cause the cache made a copy :(
-    if (!fScaledBitmap.getPixels()) {
-        // TODO: find out how this can happen, and add a unittest to exercise
-        // inspired by BUG=chromium:295895
-        return false;
-    }
     fBitmap = &fScaledBitmap;
+    unlocker.keepLock();
     return true;
 }
 
 void SkBitmapProcState::endContext() {
     SkDELETE(fBitmapFilter);
     fBitmapFilter = NULL;
     fScaledBitmap.reset();
 
     if (fScaledCacheID) {
         SkScaledImageCache::Unlock(fScaledCacheID);
         fScaledCacheID = NULL;
     }
 }
 
 SkBitmapProcState::~SkBitmapProcState() {
     if (fScaledCacheID) {
         SkScaledImageCache::Unlock(fScaledCacheID);
     }
     SkDELETE(fBitmapFilter);
 }
 
 bool SkBitmapProcState::chooseProcs(const SkMatrix& inv, const SkPaint& paint) {
     SkASSERT(fOrigBitmap.width() && fOrigBitmap.height());
 
     fBitmap = NULL;
     fInvMatrix = inv;
     fFilterLevel = paint.getFilterLevel();
 
+    SkASSERT(NULL == fScaledCacheID);
+
     // possiblyScaleImage will look to see if it can rescale the image as a
     // preprocess; either by scaling up to the target size, or by selecting
     // a nearby mipmap level.  If it does, it will adjust the working
     // matrix as well as the working bitmap.  It may also adjust the filter
     // quality to avoid re-filtering an already perfectly scaled image.
     if (!this->possiblyScaleImage()) {
         if (!this->lockBaseBitmap()) {
             return false;
         }
     }
@@ skipping 634 matching lines @@
     } else {
         size >>= 2;
     }
 
     if (fFilterLevel != SkPaint::kNone_FilterLevel) {
         size >>= 1;
     }
 
     return size;
 }