diff options
author | fmalita <fmalita@chromium.org> | 2015-09-18 08:07:31 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-09-18 08:07:31 -0700 |
commit | 3b0d532df72db806c255cad98538fcbb4d9678a8 (patch) | |
tree | 96d66c4b40e72f52a10067640f5c95025427a8da | |
parent | 3566d44d852b2fc1773e41e80c0c19610aa6d43b (diff) |
Purge cached resources on SkImage destruction.
BUG=532981
R=reed@google.com,mtklein@google.com
Review URL: https://codereview.chromium.org/1352883004
-rw-r--r-- | gm/image_pict.cpp | 5 | ||||
-rw-r--r-- | src/core/SkBitmapProvider.cpp | 2 | ||||
-rw-r--r-- | src/core/SkImageCacherator.cpp | 20 | ||||
-rw-r--r-- | src/core/SkImageCacherator.h | 13 | ||||
-rw-r--r-- | src/image/SkImage.cpp | 17 | ||||
-rw-r--r-- | src/image/SkImage_Base.h | 20 | ||||
-rw-r--r-- | src/image/SkImage_Generator.cpp | 4 | ||||
-rw-r--r-- | tests/SkResourceCacheTest.cpp | 68 |
8 files changed, 128 insertions, 21 deletions
diff --git a/gm/image_pict.cpp b/gm/image_pict.cpp index 216b495dd3..19271c1b42 100644 --- a/gm/image_pict.cpp +++ b/gm/image_pict.cpp @@ -303,14 +303,15 @@ protected: 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 96e29f81f2..cae744bcf0 100644 --- a/src/core/SkBitmapProvider.cpp +++ b/src/core/SkBitmapProvider.cpp @@ -62,7 +62,7 @@ SkBitmapCacheDesc SkBitmapProvider::makeCacheDesc() const { 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 91e342a79b..b35e053b0d 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::generateBitmap(SkBitmap* bitmap) { ////////////////////////////////////////////////////////////////////////////////////////////////// -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 @@ bool SkImageCacherator::tryLockAsBitmap(SkBitmap* bitmap) { 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 @@ bool SkImageCacherator::lockAsBitmap(SkBitmap* bitmap) { 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 @@ static GrTexture* set_key_and_return(GrTexture* tex, const GrUniqueKey& key) { * 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 @@ GrTexture* SkImageCacherator::lockAsTexture(GrContext* ctx, SkImageUsageType usa // 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 dc6b0f2c1a..6812c72fcc 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 @@ public: /** * 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 @@ private: 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 654b848fce..6f4655758e 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 @@ static bool raster_canvas_supports(const SkImageInfo& info) { 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 737b30d1db..d0cb8d5840 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 @@ enum { 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 @@ public: 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 2bb508f46d..0f1fa3692c 100644 --- a/src/image/SkImage_Generator.cpp +++ b/src/image/SkImage_Generator.cpp @@ -77,11 +77,11 @@ SkData* SkImage_Generator::onRefEncoded() const { } 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 4432b87d3b..9faddd016d 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 @@ DEF_TEST(BitmapCache_discarded_bitmap, reporter) { 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); + }); + } +} |