detect if the scaledimagecache returns a purged bitmap
BUG=
R=scroggo@google.com
Review URL: https://codereview.chromium.org/110383005
git-svn-id: http://skia.googlecode.com/svn/trunk/src@12654 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/core/SkBitmapProcState.cpp b/core/SkBitmapProcState.cpp
index 39f6a78..cdc582b 100644
--- a/core/SkBitmapProcState.cpp
+++ b/core/SkBitmapProcState.cpp
@@ -106,11 +106,33 @@
return SkMaxScalar(v1.lengthSqd(), v2.lengthSqd());
}
+class AutoScaledCacheUnlocker {
+public:
+ AutoScaledCacheUnlocker(SkScaledImageCache::ID** idPtr) : fIDPtr(idPtr) {}
+ ~AutoScaledCacheUnlocker() {
+ if (fIDPtr && *fIDPtr) {
+ SkScaledImageCache::Unlock(*fIDPtr);
+ *fIDPtr = NULL;
+ }
+ }
+
+ // forgets the ID, so it won't call Unlock
+ void release() {
+ fIDPtr = NULL;
+ }
+
+private:
+ SkScaledImageCache::ID** fIDPtr;
+};
+#define AutoScaledCacheUnlocker(...) SK_REQUIRE_LOCAL_VAR(AutoScaledCacheUnlocker)
+
// 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);
@@ -132,6 +154,17 @@
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);
@@ -154,18 +187,19 @@
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;
@@ -174,6 +208,7 @@
// no need for any further filtering; we just did it!
fFilterLevel = SkPaint::kNone_FilterLevel;
+ unlocker.release();
return true;
}
@@ -248,6 +283,7 @@
fScaledBitmap.setPixels(level.fPixels);
fBitmap = &fScaledBitmap;
fFilterLevel = SkPaint::kLow_FilterLevel;
+ unlocker.release();
return true;
}
}
@@ -273,15 +309,34 @@
}
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();
+ if (NULL == fScaledBitmap.getPixels()) {
+ return false;
+ }
} 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;
@@ -299,13 +354,8 @@
}
}
}
- 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.release();
return true;
}
@@ -334,6 +384,8 @@
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
diff --git a/core/SkBitmapScaler.cpp b/core/SkBitmapScaler.cpp
index c28d477..67a9508 100644
--- a/core/SkBitmapScaler.cpp
+++ b/core/SkBitmapScaler.cpp
@@ -301,6 +301,8 @@
convolveProcs, true);
*resultPtr = result;
+ resultPtr->lockPixels();
+ SkASSERT(NULL != resultPtr->getPixels());
return true;
}
diff --git a/core/SkScaledImageCache.cpp b/core/SkScaledImageCache.cpp
index b3956f4..7c8b664 100644
--- a/core/SkScaledImageCache.cpp
+++ b/core/SkScaledImageCache.cpp
@@ -239,9 +239,18 @@
return fDM->data();
}
- SkASSERT(!fIsLocked);
- fIsLocked = fDM->lock();
- return fIsLocked ? fDM->data() : NULL;
+ // A previous call to onUnlock may have deleted our DM, so check for that
+ if (NULL == fDM) {
+ return NULL;
+ }
+
+ if (!fDM->lock()) {
+ // since it failed, we delete it now, to free-up the resource
+ delete fDM;
+ fDM = NULL;
+ return NULL;
+ }
+ return fDM->data();
}
void SkOneShotDiscardablePixelRef::onUnlockPixels() {
@@ -613,6 +622,8 @@
this->validate();
}
+///////////////////////////////////////////////////////////////////////////////
+
#ifdef SK_DEBUG
void SkScaledImageCache::validate() const {
if (NULL == fHead) {
@@ -658,6 +669,21 @@
}
#endif
+void SkScaledImageCache::dump() const {
+ this->validate();
+
+ const Rec* rec = fHead;
+ int locked = 0;
+ while (rec) {
+ locked += rec->fLockCount > 0;
+ rec = rec->fNext;
+ }
+
+ SkDebugf("SkScaledImageCache: count=%d bytes=%d locked=%d %s\n",
+ fCount, fBytesUsed, locked,
+ fDiscardableFactory ? "discardable" : "malloc");
+}
+
///////////////////////////////////////////////////////////////////////////////
#include "SkThread.h"
@@ -730,7 +756,9 @@
void SkScaledImageCache::Unlock(SkScaledImageCache::ID* id) {
SkAutoMutexAcquire am(gMutex);
- return get_cache()->unlock(id);
+ get_cache()->unlock(id);
+
+// get_cache()->dump();
}
size_t SkScaledImageCache::GetBytesUsed() {
@@ -753,6 +781,11 @@
return get_cache()->allocator();
}
+void SkScaledImageCache::Dump() {
+ SkAutoMutexAcquire am(gMutex);
+ get_cache()->dump();
+}
+
///////////////////////////////////////////////////////////////////////////////
#include "SkGraphics.h"
diff --git a/core/SkScaledImageCache.h b/core/SkScaledImageCache.h
index 311db32..fe07230 100644
--- a/core/SkScaledImageCache.h
+++ b/core/SkScaledImageCache.h
@@ -66,6 +66,11 @@
static SkBitmap::Allocator* GetAllocator();
+ /**
+ * Call SkDebugf() with diagnostic information about the state of the cache
+ */
+ static void Dump();
+
///////////////////////////////////////////////////////////////////////////
/**
@@ -151,6 +156,11 @@
SkBitmap::Allocator* allocator() const { return fAllocator; };
+ /**
+ * Call SkDebugf() with diagnostic information about the state of the cache
+ */
+ void dump() const;
+
public:
struct Rec;
struct Key;
@@ -174,6 +184,7 @@
Rec* findAndLock(const Key& key);
ID* addAndLock(Rec* rec);
+ void purgeRec(Rec*);
void purgeAsNeeded();
// linklist management
