Make SkBitmapCache remove invalid bitmaps from the SkResourceCache.
This adds SkResourceCache::Remove() which will remove a resource from
its cache. The resource is required to be unlocked at the time Remove()
is called.
Then SkBitmapCache::Find() makes use of this to Remove() bitmaps from
the cache whose pixels have been evicted. This allows the bitmap to be
re-added to the cache with pixels again.
After this change, background a tab (and discarding all the bitmaps'
contents) no longer disables image caching for those discarded images
once the tab is visible again.
BUG=skia:2926
NOTRY=true
R=reed@android.com, tomhudson@google.com, reed@google.com
Author: danakj@chromium.org
Review URL: https://codereview.chromium.org/561953002
diff --git a/gyp/common.gypi b/gyp/common.gypi
index 2d3fddd..7ea1578 100644
--- a/gyp/common.gypi
+++ b/gyp/common.gypi
@@ -16,6 +16,7 @@
'SK_GAMMA_SRGB',
'SK_GAMMA_APPLY_TO_A8',
'SK_SCALAR_TO_FLOAT_EXCLUDED', # temporary to allow Chrome to call SkFloatToScalar
+ 'SK_USE_DISCARDABLE_SCALEDIMAGECACHE',
],
# Validate the 'skia_os' setting against 'OS', because only certain
diff --git a/src/core/SkBitmapCache.cpp b/src/core/SkBitmapCache.cpp
index c954a30..6d4f4b4 100644
--- a/src/core/SkBitmapCache.cpp
+++ b/src/core/SkBitmapCache.cpp
@@ -71,8 +71,8 @@
if (result->getPixels()) {
return true;
}
- // todo: we should explicitly purge rec from the cache at this point, since
- // it is effectively purged already (has no memory behind it)
+
+ SkResourceCache::Remove(rec);
result->reset();
// fall-through to false
}
diff --git a/src/core/SkResourceCache.cpp b/src/core/SkResourceCache.cpp
index 73f788c..6400e8a 100644
--- a/src/core/SkResourceCache.cpp
+++ b/src/core/SkResourceCache.cpp
@@ -286,6 +286,23 @@
}
}
+void SkResourceCache::remove(Rec* rec) {
+ SkASSERT(0 == rec->fLockCount);
+
+ size_t used = rec->bytesUsed();
+ SkASSERT(used <= fTotalBytesUsed);
+
+ this->detach(rec);
+#ifdef USE_HASH
+ fHash->remove(rec->getKey());
+#endif
+
+ SkDELETE(rec);
+
+ fTotalBytesUsed -= used;
+ fCount -= 1;
+}
+
void SkResourceCache::purgeAsNeeded() {
size_t byteLimit;
int countLimit;
@@ -298,34 +315,18 @@
byteLimit = fTotalByteLimit;
}
- size_t bytesUsed = fTotalBytesUsed;
- int countUsed = fCount;
-
Rec* rec = fTail;
while (rec) {
- if (bytesUsed < byteLimit && countUsed < countLimit) {
+ if (fTotalBytesUsed < byteLimit && fCount < countLimit) {
break;
}
Rec* prev = rec->fPrev;
if (0 == rec->fLockCount) {
- size_t used = rec->bytesUsed();
- SkASSERT(used <= bytesUsed);
- this->detach(rec);
-#ifdef USE_HASH
- fHash->remove(rec->getKey());
-#endif
-
- SkDELETE(rec);
-
- bytesUsed -= used;
- countUsed -= 1;
+ this->remove(rec);
}
rec = prev;
}
-
- fTotalBytesUsed = bytesUsed;
- fCount = countUsed;
}
size_t SkResourceCache::setTotalByteLimit(size_t newLimit) {
@@ -506,6 +507,28 @@
// get_cache()->dump();
}
+void SkResourceCache::Remove(SkResourceCache::ID id) {
+ SkAutoMutexAcquire am(gMutex);
+ SkASSERT(id);
+
+#ifdef SK_DEBUG
+ {
+ bool found = false;
+ Rec* rec = get_cache()->fHead;
+ while (rec != NULL) {
+ if (rec == id) {
+ found = true;
+ break;
+ }
+ rec = rec->fNext;
+ }
+ SkASSERT(found);
+ }
+#endif
+ const Rec* rec = id;
+ get_cache()->remove(const_cast<Rec*>(rec));
+}
+
size_t SkResourceCache::GetTotalBytesUsed() {
SkAutoMutexAcquire am(gMutex);
return get_cache()->getTotalBytesUsed();
diff --git a/src/core/SkResourceCache.h b/src/core/SkResourceCache.h
index f2fd8fc..93f2ca4 100644
--- a/src/core/SkResourceCache.h
+++ b/src/core/SkResourceCache.h
@@ -76,7 +76,6 @@
Rec* fNext;
Rec* fPrev;
int32_t fLockCount;
- int32_t fPad;
friend class SkResourceCache;
};
@@ -98,6 +97,7 @@
static const Rec* AddAndLock(Rec*);
static void Add(Rec*);
static void Unlock(ID);
+ static void Remove(ID);
static size_t GetTotalBytesUsed();
static size_t GetTotalByteLimit();
@@ -140,6 +140,7 @@
const Rec* findAndLock(const Key& key);
const Rec* addAndLock(Rec*);
void add(Rec*);
+ void remove(Rec*);
/**
* Given a non-null ID ptr returned by either findAndLock or addAndLock,
@@ -189,7 +190,6 @@
size_t fSingleAllocationByteLimit;
int fCount;
- void purgeRec(Rec*);
void purgeAsNeeded();
// linklist management
diff --git a/tests/SkResourceCacheTest.cpp b/tests/SkResourceCacheTest.cpp
index b660890..20c3f7f 100644
--- a/tests/SkResourceCacheTest.cpp
+++ b/tests/SkResourceCacheTest.cpp
@@ -8,6 +8,7 @@
#include "SkCanvas.h"
#include "SkGraphics.h"
#include "SkBitmapCache.h"
+#include "SkDiscardableMemoryPool.h"
static const int kCanvasSize = 1;
static const int kBitmapSize = 16;
@@ -109,4 +110,36 @@
// Should be in the cache, we just added it
REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm));
}
+
+DEF_TEST(BitmapCache_discarded_bitmap, reporter) {
+ SkBitmap bm;
+ SkIRect rect = SkIRect::MakeWH(5, 5);
+ SkBitmap cachedBitmap = createAllocatedBitmap(SkImageInfo::MakeN32Premul(5, 5));
+ cachedBitmap.setImmutable();
+ cachedBitmap.unlockPixels();
+
+ // Add a bitmap to the cache.
+ REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.getGenerationID(), rect, cachedBitmap));
+ REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm));
+
+ // Finding more than once works fine.
+ REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm));
+ bm.unlockPixels();
+
+ // Drop the pixels in the bitmap.
+ REPORTER_ASSERT(reporter, SkGetGlobalDiscardableMemoryPool()->getRAMUsed() > 0);
+ SkGetGlobalDiscardableMemoryPool()->dumpPool();
+ REPORTER_ASSERT(reporter, SkGetGlobalDiscardableMemoryPool()->getRAMUsed() == 0);
+
+ // The bitmap is not in the cache since it has been dropped.
+ REPORTER_ASSERT(reporter, !SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm));
+
+ cachedBitmap = createAllocatedBitmap(SkImageInfo::MakeN32Premul(5, 5));
+ cachedBitmap.setImmutable();
+ cachedBitmap.unlockPixels();
+
+ // We can add the bitmap back to the cache and find it again.
+ REPORTER_ASSERT(reporter, SkBitmapCache::Add(cachedBitmap.getGenerationID(), rect, cachedBitmap));
+ REPORTER_ASSERT(reporter, SkBitmapCache::Find(cachedBitmap.getGenerationID(), rect, &bm));
+}
#endif
