Purge cached resources on SkImage destruction.
BUG=532981
R=reed@google.com,mtklein@google.com
Review URL: https://codereview.chromium.org/1352883004
diff --git a/gm/image_pict.cpp b/gm/image_pict.cpp
index 216b495..19271c1 100644
--- a/gm/image_pict.cpp
+++ b/gm/image_pict.cpp
@@ -303,14 +303,15 @@
static void draw_as_bitmap(SkCanvas* canvas, SkImageCacherator* cache, SkScalar x, SkScalar y) {
SkBitmap bitmap;
- cache->lockAsBitmap(&bitmap);
+ cache->lockAsBitmap(&bitmap, nullptr);
canvas->drawBitmap(bitmap, x, y);
}
static void draw_as_tex(SkCanvas* canvas, SkImageCacherator* cache, SkScalar x, SkScalar y) {
#if SK_SUPPORT_GPU
SkAutoTUnref<GrTexture> texture(cache->lockAsTexture(canvas->getGrContext(),
- kUntiled_SkImageUsageType));
+ kUntiled_SkImageUsageType,
+ nullptr));
if (!texture) {
// show placeholder if we have no texture
SkPaint paint;
diff --git a/src/core/SkBitmapProvider.cpp b/src/core/SkBitmapProvider.cpp
index 96e29f8..cae744b 100644
--- a/src/core/SkBitmapProvider.cpp
+++ b/src/core/SkBitmapProvider.cpp
@@ -62,7 +62,7 @@
void SkBitmapProvider::notifyAddedToCache() const {
if (fImage) {
- // TODO
+ as_IB(fImage)->notifyAddedToCache();
} else {
fBitmap.pixelRef()->notifyAddedToCache();
}
diff --git a/src/core/SkImageCacherator.cpp b/src/core/SkImageCacherator.cpp
index 91e342a..b35e053 100644
--- a/src/core/SkImageCacherator.cpp
+++ b/src/core/SkImageCacherator.cpp
@@ -7,6 +7,7 @@
#include "SkBitmap.h"
#include "SkBitmapCache.h"
+#include "SkImage_Base.h"
#include "SkImageCacherator.h"
#include "SkMallocPixelRef.h"
#include "SkNextID.h"
@@ -109,7 +110,7 @@
//////////////////////////////////////////////////////////////////////////////////////////////////
-bool SkImageCacherator::tryLockAsBitmap(SkBitmap* bitmap) {
+bool SkImageCacherator::tryLockAsBitmap(SkBitmap* bitmap, const SkImage* client) {
if (SkBitmapCache::Find(fUniqueID, bitmap)) {
return check_output_bitmap(*bitmap, fUniqueID);
}
@@ -120,11 +121,15 @@
bitmap->pixelRef()->setImmutableWithID(fUniqueID);
SkBitmapCache::Add(fUniqueID, *bitmap);
+ if (client) {
+ as_IB(client)->notifyAddedToCache();
+ }
+
return true;
}
-bool SkImageCacherator::lockAsBitmap(SkBitmap* bitmap) {
- if (this->tryLockAsBitmap(bitmap)) {
+bool SkImageCacherator::lockAsBitmap(SkBitmap* bitmap, const SkImage* client) {
+ if (this->tryLockAsBitmap(bitmap, client)) {
return check_output_bitmap(*bitmap, fUniqueID);
}
@@ -156,6 +161,10 @@
bitmap->pixelRef()->setImmutableWithID(fUniqueID);
SkBitmapCache::Add(fUniqueID, *bitmap);
+ if (client) {
+ as_IB(client)->notifyAddedToCache();
+ }
+
return check_output_bitmap(*bitmap, fUniqueID);
#else
return false;
@@ -209,7 +218,8 @@
* 4. Ask the generator to return YUV planes, which the GPU can convert
* 5. Ask the generator to return RGB(A) data, which the GPU can convert
*/
-GrTexture* SkImageCacherator::lockAsTexture(GrContext* ctx, SkImageUsageType usage) {
+GrTexture* SkImageCacherator::lockAsTexture(GrContext* ctx, SkImageUsageType usage,
+ const SkImage* client) {
#if SK_SUPPORT_GPU
if (!ctx) {
return nullptr;
@@ -262,7 +272,7 @@
// 5. Ask the generator to return RGB(A) data, which the GPU can convert
SkBitmap bitmap;
- if (this->tryLockAsBitmap(&bitmap)) {
+ if (this->tryLockAsBitmap(&bitmap, client)) {
return GrRefCachedBitmapTexture(ctx, bitmap, usage);
}
#endif
diff --git a/src/core/SkImageCacherator.h b/src/core/SkImageCacherator.h
index dc6b0f2..6812c72 100644
--- a/src/core/SkImageCacherator.h
+++ b/src/core/SkImageCacherator.h
@@ -14,6 +14,7 @@
class GrContext;
class SkBitmap;
+class SkImage;
/*
* Internal class to manage caching the output of an ImageGenerator.
@@ -29,16 +30,22 @@
/**
* On success (true), bitmap will point to the pixels for this generator. If this returns
* false, the bitmap will be reset to empty.
+ *
+ * If not NULL, the client will be notified (->notifyAddedToCache()) when resources are
+ * added to the cache on its behalf.
*/
- bool lockAsBitmap(SkBitmap*);
+ bool lockAsBitmap(SkBitmap*, const SkImage* client);
/**
* Returns a ref() on the texture produced by this generator. The caller must call unref()
* when it is done. Will return nullptr on failure.
*
+ * If not NULL, the client will be notified (->notifyAddedToCache()) when resources are
+ * added to the cache on its behalf.
+ *
* The caller is responsible for calling texture->unref() when they are done.
*/
- GrTexture* lockAsTexture(GrContext*, SkImageUsageType);
+ GrTexture* lockAsTexture(GrContext*, SkImageUsageType, const SkImage* client);
/**
* If the underlying src naturally is represented by an encoded blob (in SkData), this returns
@@ -50,7 +57,7 @@
SkImageCacherator(SkImageGenerator*, const SkImageInfo&, const SkIPoint&, uint32_t uniqueID);
bool generateBitmap(SkBitmap*);
- bool tryLockAsBitmap(SkBitmap*);
+ bool tryLockAsBitmap(SkBitmap*, const SkImage*);
class ScopedGenerator {
SkImageCacherator* fCacher;
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index 654b848..6f46557 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -6,6 +6,7 @@
*/
#include "SkBitmap.h"
+#include "SkBitmapCache.h"
#include "SkCanvas.h"
#include "SkData.h"
#include "SkImageGenerator.h"
@@ -210,6 +211,22 @@
return false;
}
+static SkSurfaceProps copy_or_safe_defaults(const SkSurfaceProps* props) {
+ return props ? *props : SkSurfaceProps(0, kUnknown_SkPixelGeometry);
+}
+
+SkImage_Base::SkImage_Base(int width, int height, uint32_t uniqueID, const SkSurfaceProps* props)
+ : INHERITED(width, height, uniqueID)
+ , fProps(copy_or_safe_defaults(props))
+ , fAddedToCache(false)
+{ }
+
+SkImage_Base::~SkImage_Base() {
+ if (fAddedToCache.load()) {
+ SkNotifyBitmapGenIDIsStale(this->uniqueID());
+ }
+}
+
bool SkImage_Base::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
int srcX, int srcY) const {
if (!raster_canvas_supports(dstInfo)) {
diff --git a/src/image/SkImage_Base.h b/src/image/SkImage_Base.h
index 737b30d..d0cb8d5 100644
--- a/src/image/SkImage_Base.h
+++ b/src/image/SkImage_Base.h
@@ -8,6 +8,7 @@
#ifndef SkImage_Base_DEFINED
#define SkImage_Base_DEFINED
+#include "SkAtomics.h"
#include "SkImage.h"
#include "SkSurface.h"
@@ -17,16 +18,10 @@
kNeedNewImageUniqueID = 0
};
-static SkSurfaceProps copy_or_safe_defaults(const SkSurfaceProps* props) {
- return props ? *props : SkSurfaceProps(0, kUnknown_SkPixelGeometry);
-}
-
class SkImage_Base : public SkImage {
public:
- SkImage_Base(int width, int height, uint32_t uniqueID, const SkSurfaceProps* props)
- : INHERITED(width, height, uniqueID)
- , fProps(copy_or_safe_defaults(props))
- {}
+ SkImage_Base(int width, int height, uint32_t uniqueID, const SkSurfaceProps* props);
+ virtual ~SkImage_Base();
/**
* If the props weren't know at constructor time, call this but only before the image is
@@ -74,9 +69,18 @@
virtual bool onIsLazyGenerated() const { return false; }
+ // Call when this image is part of the key to a resourcecache entry. This allows the cache
+ // to know automatically those entries can be purged when this SkImage deleted.
+ void notifyAddedToCache() const {
+ fAddedToCache.store(true);
+ }
+
private:
const SkSurfaceProps fProps;
+ // Set true by caches when they cache content that's derived from the current pixels.
+ mutable SkAtomic<bool> fAddedToCache;
+
typedef SkImage INHERITED;
};
diff --git a/src/image/SkImage_Generator.cpp b/src/image/SkImage_Generator.cpp
index 2bb508f..0f1fa36 100644
--- a/src/image/SkImage_Generator.cpp
+++ b/src/image/SkImage_Generator.cpp
@@ -77,11 +77,11 @@
}
bool SkImage_Generator::getROPixels(SkBitmap* bitmap) const {
- return fCache->lockAsBitmap(bitmap);
+ return fCache->lockAsBitmap(bitmap, this);
}
GrTexture* SkImage_Generator::asTextureRef(GrContext* ctx, SkImageUsageType usage) const {
- return fCache->lockAsTexture(ctx, usage);
+ return fCache->lockAsTexture(ctx, usage, this);
}
SkImage* SkImage::NewFromGenerator(SkImageGenerator* generator, const SkIRect* subset) {
diff --git a/tests/SkResourceCacheTest.cpp b/tests/SkResourceCacheTest.cpp
index 4432b87..9faddd0 100644
--- a/tests/SkResourceCacheTest.cpp
+++ b/tests/SkResourceCacheTest.cpp
@@ -10,6 +10,8 @@
#include "SkCanvas.h"
#include "SkDiscardableMemoryPool.h"
#include "SkGraphics.h"
+#include "SkPicture.h"
+#include "SkPictureRecorder.h"
#include "SkResourceCache.h"
#include "SkSurface.h"
@@ -219,3 +221,69 @@
test_bitmap_notify(reporter, cache);
test_mipmap_notify(reporter, cache);
}
+
+static void test_discarded_image(skiatest::Reporter* reporter, const SkMatrix& transform,
+ SkImage* (*buildImage)()) {
+ SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(10, 10));
+ SkCanvas* canvas = surface->getCanvas();
+
+ // SkBitmapCache is global, so other threads could be evicting our bitmaps. Loop a few times
+ // to mitigate this risk.
+ const unsigned kRepeatCount = 42;
+ for (unsigned i = 0; i < kRepeatCount; ++i) {
+ SkAutoCanvasRestore acr(canvas, true);
+
+ SkAutoTUnref<SkImage> image(buildImage());
+
+ // always use high quality to ensure caching when scaled
+ SkPaint paint;
+ paint.setFilterQuality(kHigh_SkFilterQuality);
+
+ // draw the image (with a transform, to tickle different code paths) to ensure
+ // any associated resources get cached
+ canvas->concat(transform);
+ canvas->drawImage(image, 0, 0, &paint);
+
+ auto imageId = image->uniqueID();
+
+ // delete the image
+ image.reset(nullptr);
+
+ // all resources should have been purged
+ SkBitmap result;
+ REPORTER_ASSERT(reporter, !SkBitmapCache::Find(imageId, &result));
+ }
+}
+
+
+// Verify that associated bitmap cache entries are purged on SkImage destruction.
+DEF_TEST(BitmapCache_discarded_image, reporter) {
+ // Cache entries associated with SkImages fall into two categories:
+ //
+ // 1) generated image bitmaps (managed by the image cacherator)
+ // 2) scaled/resampled bitmaps (cached when HQ filters are used)
+ //
+ // To exercise the first cache type, we use generated/picture-backed SkImages.
+ // To exercise the latter, we draw scaled bitmap images using HQ filters.
+
+ const SkMatrix xforms[] = {
+ SkMatrix::MakeScale(1, 1),
+ SkMatrix::MakeScale(1.7f, 0.5f),
+ };
+
+ for (size_t i = 0; i < SK_ARRAY_COUNT(xforms); ++i) {
+ test_discarded_image(reporter, xforms[i], []() {
+ SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(10, 10));
+ surface->getCanvas()->clear(SK_ColorCYAN);
+ return surface->newImageSnapshot();
+ });
+
+ test_discarded_image(reporter, xforms[i], []() {
+ SkPictureRecorder recorder;
+ SkCanvas* canvas = recorder.beginRecording(10, 10);
+ canvas->clear(SK_ColorCYAN);
+ SkAutoTUnref<SkPicture> picture(recorder.endRecording());
+ return SkImage::NewFromPicture(picture, SkISize::Make(10, 10), nullptr, nullptr);
+ });
+ }
+}