diff --git a/include/core/SkImageFilter.h b/include/core/SkImageFilter.h index d7a3f33b75..094e67b394 100644 --- a/include/core/SkImageFilter.h +++ b/include/core/SkImageFilter.h @@ -437,8 +437,7 @@ private: bool fUsesSrcInput; CropRect fCropRect; uint32_t fUniqueID; // Globally unique - mutable SkTArray fCacheKeys; - mutable SkMutex fMutex; + typedef SkFlattenable INHERITED; }; diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp index 7ce2cf36cc..fac63e5102 100644 --- a/src/core/SkImageFilter.cpp +++ b/src/core/SkImageFilter.cpp @@ -167,8 +167,7 @@ SkImageFilter::SkImageFilter(sk_sp const* inputs, } SkImageFilter::~SkImageFilter() { - SkAutoMutexAcquire lock(fMutex); - SkImageFilterCache::Get()->purgeByKeys(fCacheKeys.begin(), fCacheKeys.count()); + SkImageFilterCache::Get()->purgeByImageFilter(this); } SkImageFilter::SkImageFilter(int inputCount, SkReadBuffer& buffer) @@ -224,30 +223,11 @@ sk_sp SkImageFilter::filterImage(SkSpecialImage* src, const Cont if (result && context.cache()) { context.cache()->set(key, result.get(), *offset, this); - SkAutoMutexAcquire mutex(fMutex); - fCacheKeys.push_back(key); } return result; } -void SkImageFilter::removeKey(const SkImageFilterCacheKey& key) const { - SkAutoMutexAcquire mutex(fMutex); - for (int i = 0; i < fCacheKeys.count(); i++) { - if (fCacheKeys[i] == key) { - fCacheKeys.removeShuffle(i); - break; - } - } -#ifdef SK_DEBUG - for (int i = 0; i < fCacheKeys.count(); i++) { - if (fCacheKeys[i] == key) { - SkASSERT(false); - } - } -#endif -} - SkIRect SkImageFilter::filterBounds(const SkIRect& src, const SkMatrix& ctm, MapDirection direction) const { if (kReverse_MapDirection == direction) { diff --git a/src/core/SkImageFilterCache.cpp b/src/core/SkImageFilterCache.cpp index 51249ef962..291bae76ad 100644 --- a/src/core/SkImageFilterCache.cpp +++ b/src/core/SkImageFilterCache.cpp @@ -7,6 +7,8 @@ #include "SkImageFilterCache.h" +#include + #include "SkImageFilter.h" #include "SkMutex.h" #include "SkOnce.h" @@ -14,6 +16,7 @@ #include "SkRefCnt.h" #include "SkSpecialImage.h" #include "SkTDynamicHash.h" +#include "SkTHash.h" #include "SkTInternalLList.h" #ifdef SK_BUILD_FOR_IOS @@ -76,6 +79,12 @@ public: fLookup.add(v); fLRU.addToHead(v); fCurrentBytes += image->getSize(); + if (auto* values = fImageFilterValues.find(filter)) { + values->push_back(v); + } else { + fImageFilterValues.set(filter, {v}); + } + while (fCurrentBytes > fMaxBytes) { Value* tail = fLRU.tail(); SkASSERT(tail); @@ -95,18 +104,19 @@ public: } } - void purgeByKeys(const Key keys[], int count) override { + void purgeByImageFilter(const SkImageFilter* filter) override { SkAutoMutexAcquire mutex(fMutex); - // This function is only called in the destructor of SkImageFilter. - // Because the destructor will destroy the fCacheKeys anyway, we set the - // filter to be null so that removeInternal() won't call the - // SkImageFilter::removeKey() function. - for (int i = 0; i < count; i++) { - if (Value* v = fLookup.find(keys[i])) { - v->fFilter = nullptr; - this->removeInternal(v); - } + auto* values = fImageFilterValues.find(filter); + if (!values) { + return; } + for (Value* v : *values) { + // We set the filter to be null so that removeInternal() won't delete from values while + // we're iterating over it. + v->fFilter = nullptr; + this->removeInternal(v); + } + fImageFilterValues.remove(filter); } SkDEBUGCODE(int count() const override { return fLookup.count(); }) @@ -114,7 +124,18 @@ private: void removeInternal(Value* v) { SkASSERT(v->fImage); if (v->fFilter) { - v->fFilter->removeKey(v->fKey); + if (auto* values = fImageFilterValues.find(v->fFilter)) { + if (values->size() == 1 && (*values)[0] == v) { + fImageFilterValues.remove(v->fFilter); + } else { + for (auto it = values->begin(); it != values->end(); ++it) { + if (*it == v) { + values->erase(it); + break; + } + } + } + } } fCurrentBytes -= v->fImage->getSize(); fLRU.remove(v); @@ -122,11 +143,13 @@ private: delete v; } private: - SkTDynamicHash fLookup; - mutable SkTInternalLList fLRU; - size_t fMaxBytes; - size_t fCurrentBytes; - mutable SkMutex fMutex; + SkTDynamicHash fLookup; + mutable SkTInternalLList fLRU; + // Value* always points to an item in fLookup. + SkTHashMap> fImageFilterValues; + size_t fMaxBytes; + size_t fCurrentBytes; + mutable SkMutex fMutex; }; } // namespace diff --git a/src/core/SkImageFilterCache.h b/src/core/SkImageFilterCache.h index 2211a7aaca..710de86892 100644 --- a/src/core/SkImageFilterCache.h +++ b/src/core/SkImageFilterCache.h @@ -59,7 +59,7 @@ public: virtual void set(const SkImageFilterCacheKey& key, SkSpecialImage* image, const SkIPoint& offset, const SkImageFilter* filter) = 0; virtual void purge() = 0; - virtual void purgeByKeys(const SkImageFilterCacheKey[], int) = 0; + virtual void purgeByImageFilter(const SkImageFilter*) = 0; SkDEBUGCODE(virtual int count() const = 0;) }; diff --git a/tests/ImageFilterCacheTest.cpp b/tests/ImageFilterCacheTest.cpp index e9c9f0b14e..f4640d1439 100644 --- a/tests/ImageFilterCacheTest.cpp +++ b/tests/ImageFilterCacheTest.cpp @@ -8,6 +8,8 @@ #include "Test.h" #include "SkBitmap.h" +#include "SkColorFilter.h" +#include "SkColorFilterImageFilter.h" #include "SkImage.h" #include "SkImageFilter.h" #include "SkImageFilterCache.h" @@ -25,6 +27,12 @@ static SkBitmap create_bm() { return bm; } +static sk_sp make_filter() { + sk_sp filter(SkColorFilter::MakeModeFilter(SK_ColorBLUE, + SkBlendMode::kSrcIn)); + return SkColorFilterImageFilter::Make(std::move(filter), nullptr, nullptr); +} + // Ensure the cache can return a cached image static void test_find_existing(skiatest::Reporter* reporter, const sk_sp& image, @@ -37,7 +45,8 @@ static void test_find_existing(skiatest::Reporter* reporter, SkImageFilterCacheKey key2(0, SkMatrix::I(), clip, subset->uniqueID(), subset->subset()); SkIPoint offset = SkIPoint::Make(3, 4); - cache->set(key1, image.get(), offset, nullptr); + auto filter = make_filter(); + cache->set(key1, image.get(), offset, filter.get()); SkIPoint foundOffset; @@ -66,7 +75,8 @@ static void test_dont_find_if_diff_key(skiatest::Reporter* reporter, SkImageFilterCacheKey key4(0, SkMatrix::I(), clip1, subset->uniqueID(), subset->subset()); SkIPoint offset = SkIPoint::Make(3, 4); - cache->set(key0, image.get(), offset, nullptr); + auto filter = make_filter(); + cache->set(key0, image.get(), offset, filter.get()); SkIPoint foundOffset; REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); @@ -86,14 +96,16 @@ static void test_internal_purge(skiatest::Reporter* reporter, const sk_spuniqueID(), image->subset()); SkIPoint offset = SkIPoint::Make(3, 4); - cache->set(key1, image.get(), offset, nullptr); + auto filter1 = make_filter(); + cache->set(key1, image.get(), offset, filter1.get()); SkIPoint foundOffset; REPORTER_ASSERT(reporter, cache->get(key1, &foundOffset)); // This should knock the first one out of the cache - cache->set(key2, image.get(), offset, nullptr); + auto filter2 = make_filter(); + cache->set(key2, image.get(), offset, filter2.get()); REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset)); REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); @@ -111,8 +123,10 @@ static void test_explicit_purging(skiatest::Reporter* reporter, SkImageFilterCacheKey key2(1, SkMatrix::I(), clip, subset->uniqueID(), image->subset()); SkIPoint offset = SkIPoint::Make(3, 4); - cache->set(key1, image.get(), offset, nullptr); - cache->set(key2, image.get(), offset, nullptr); + auto filter1 = make_filter(); + auto filter2 = make_filter(); + cache->set(key1, image.get(), offset, filter1.get()); + cache->set(key2, image.get(), offset, filter2.get()); SkDEBUGCODE(REPORTER_ASSERT(reporter, 2 == cache->count());) SkIPoint foundOffset; @@ -120,7 +134,7 @@ static void test_explicit_purging(skiatest::Reporter* reporter, REPORTER_ASSERT(reporter, cache->get(key1, &foundOffset)); REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset)); - cache->purgeByKeys(&key1, 1); + cache->purgeByImageFilter(filter1.get()); SkDEBUGCODE(REPORTER_ASSERT(reporter, 1 == cache->count());) REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset));