AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix memory leak in SkImageFilter

Browse Source 
In our current implementation of SkImageFilterCache, when the
removeInternal() function is called, the Value is removed, but their
corresponding keys are not always removed in SkImageFilter. That could
result in memory leak.

In this CL, we made changes such that the Value structure now keeps
a pointer to the SkImageFilter. Each time when the removeInternal()
is called, we ask the SkImageFilter to remove the associated keys.

Bug: 689740
Change-Id: I0807fa3581881ad1530536df5289e3976792281f
Reviewed-on: https://skia-review.googlesource.com/20960
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
This commit is contained in:
xidachen 2017-06-29 11:19:42 -04:00 committed by Skia Commit-Bot
parent 9026fe13a7
commit 185a3798db
5 changed files with 43 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/core/SkImageFilter.h
View File

@ -173,6 +173,8 @@ public:
return this->isColorFilterNode(filterPtr);
}
void removeKey(const SkImageFilterCacheKey& key) const;
/**
* Returns true (and optionally returns a ref'd filter) if this imagefilter can be completely
* replaced by the returned colorfilter. i.e. the two effects will affect drawing in the

19
src/core/SkImageFilter.cpp
View File

@ -221,7 +221,7 @@ sk_sp<SkSpecialImage> SkImageFilter::filterImage(SkSpecialImage* src, const Cont
#endif
if (result && context.cache()) {
context.cache()->set(key, result.get(), *offset);
context.cache()->set(key, result.get(), *offset, this);
SkAutoMutexAcquire mutex(fMutex);
fCacheKeys.push_back(key);
}
@ -229,6 +229,23 @@ sk_sp<SkSpecialImage> SkImageFilter::filterImage(SkSpecialImage* src, const Cont
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) {

18
src/core/SkImageFilterCache.cpp
View File

@ -7,6 +7,7 @@
#include "SkImageFilterCache.h"
#include "SkImageFilter.h"
#include "SkMutex.h"
#include "SkOnce.h"
#include "SkOpts.h"
@ -37,12 +38,13 @@ public:
}
}
struct Value {
Value(const Key& key, SkSpecialImage* image, const SkIPoint& offset)
: fKey(key), fImage(SkRef(image)), fOffset(offset) {}
Value(const Key& key, SkSpecialImage* image, const SkIPoint& offset, const SkImageFilter* filter)
: fKey(key), fImage(SkRef(image)), fOffset(offset), fFilter(filter) {}
Key fKey;
sk_sp<SkSpecialImage> fImage;
SkIPoint fOffset;
const SkImageFilter* fFilter;
static const Key& GetKey(const Value& v) {
return v.fKey;
}
@ -65,12 +67,12 @@ public:
return nullptr;
}
void set(const Key& key, SkSpecialImage* image, const SkIPoint& offset) override {
void set(const Key& key, SkSpecialImage* image, const SkIPoint& offset, const SkImageFilter* filter) override {
SkAutoMutexAcquire mutex(fMutex);
if (Value* v = fLookup.find(key)) {
this->removeInternal(v);
}
Value* v = new Value(key, image, offset);
Value* v = new Value(key, image, offset, filter);
fLookup.add(v);
fLRU.addToHead(v);
fCurrentBytes += image->getSize();
@ -95,8 +97,13 @@ public:
void purgeByKeys(const Key keys[], int count) 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);
}
}
@ -106,6 +113,9 @@ public:
private:
void removeInternal(Value* v) {
SkASSERT(v->fImage);
if (v->fFilter) {
v->fFilter->removeKey(v->fKey);
}
fCurrentBytes -= v->fImage->getSize();
fLRU.remove(v);
fLookup.remove(v->fKey);

3
src/core/SkImageFilterCache.h
View File

@ -12,6 +12,7 @@
#include "SkRefCnt.h"
struct SkIPoint;
class SkImageFilter;
class SkSpecialImage;
struct SkImageFilterCacheKey {
@ -55,7 +56,7 @@ public:
static SkImageFilterCache* Get();
virtual sk_sp<SkSpecialImage> get(const SkImageFilterCacheKey& key, SkIPoint* offset) const = 0;
virtual void set(const SkImageFilterCacheKey& key, SkSpecialImage* image,
const SkIPoint& offset) = 0;
const SkIPoint& offset, const SkImageFilter* filter) = 0;
virtual void purge() = 0;
virtual void purgeByKeys(const SkImageFilterCacheKey[], int) = 0;
SkDEBUGCODE(virtual int count() const = 0;)

14
tests/ImageFilterCacheTest.cpp
View File

@ -37,7 +37,7 @@ 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);
cache->set(key1, image.get(), offset, nullptr);
SkIPoint foundOffset;
@ -66,7 +66,7 @@ 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);
cache->set(key0, image.get(), offset, nullptr);
SkIPoint foundOffset;
REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset));
@ -86,20 +86,20 @@ static void test_internal_purge(skiatest::Reporter* reporter, const sk_sp<SkSpec
SkImageFilterCacheKey key2(1, SkMatrix::I(), clip, image->uniqueID(), image->subset());
SkIPoint offset = SkIPoint::Make(3, 4);
cache->set(key1, image.get(), offset);
cache->set(key1, image.get(), offset, nullptr);
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);
cache->set(key2, image.get(), offset, nullptr);
REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset));
REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset));
}
// Exercise the purgeByKeys and purge methods
// Exercise the purgeByKey and purge methods
static void test_explicit_purging(skiatest::Reporter* reporter,
const sk_sp<SkSpecialImage>& image,
const sk_sp<SkSpecialImage>& subset) {
@ -111,8 +111,8 @@ 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);
cache->set(key2, image.get(), offset);
cache->set(key1, image.get(), offset, nullptr);
cache->set(key2, image.get(), offset, nullptr);
SkDEBUGCODE(REPORTER_ASSERT(reporter, 2 == cache->count());)
SkIPoint foundOffset;