From: robertphillips Date: Fri, 19 Feb 2016 16:19:40 +0000 (-0800) Subject: Update SkImageFilter's cache to handle SkSpecialImages and add unit test X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~129^2~1940 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=df7bb471e5455dece2784a970d9ae50d3ab0ca75;p=platform%2Fupstream%2FlibSkiaSharp.git Update SkImageFilter's cache to handle SkSpecialImages and add unit test This is calved off of: https://codereview.chromium.org/1695823002/ (Get OffsetImageFilter really working with SkSpecialImages) GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1709263002 TBR=bsalomon@google.com Review URL: https://codereview.chromium.org/1709263002 --- diff --git a/gyp/core.gypi b/gyp/core.gypi index 223839a..748eb99 100644 --- a/gyp/core.gypi +++ b/gyp/core.gypi @@ -140,6 +140,7 @@ '<(skia_src_path)/core/SkHalf.cpp', '<(skia_src_path)/core/SkHalf.h', '<(skia_src_path)/core/SkImageFilter.cpp', + '<(skia_src_path)/core/SkImageFilterCacheKey.h', '<(skia_src_path)/core/SkImageInfo.cpp', '<(skia_src_path)/core/SkImageCacherator.h', '<(skia_src_path)/core/SkImageCacherator.cpp', diff --git a/include/core/SkImageFilter.h b/include/core/SkImageFilter.h index 29f6f7b..4f4d11d 100644 --- a/include/core/SkImageFilter.h +++ b/include/core/SkImageFilter.h @@ -23,6 +23,7 @@ class SkBaseDevice; class SkBitmap; class SkColorFilter; struct SkIPoint; +class SkSpecialImage; /** * Base class for image filters. If one is installed in the paint, then @@ -42,7 +43,9 @@ public: static Cache* Create(size_t maxBytes); static Cache* Get(); virtual bool get(const Key& key, SkBitmap* result, SkIPoint* offset) const = 0; + virtual SkSpecialImage* get(const Key& key, SkIPoint* offset) const = 0; virtual void set(const Key& key, const SkBitmap& result, const SkIPoint& offset) = 0; + virtual void set(const Key& key, SkSpecialImage* image, const SkIPoint& offset) = 0; virtual void purge() {} virtual void purgeByKeys(const Key[], int) {} }; diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp index a38124b..13401af 100644 --- a/src/core/SkImageFilter.cpp +++ b/src/core/SkImageFilter.cpp @@ -6,6 +6,7 @@ */ #include "SkImageFilter.h" +#include "SkImageFilterCacheKey.h" #include "SkBitmap.h" #include "SkBitmapDevice.h" @@ -16,6 +17,7 @@ #include "SkOncePtr.h" #include "SkReadBuffer.h" #include "SkRect.h" +#include "SkSpecialImage.h" #include "SkTDynamicHash.h" #include "SkTInternalLList.h" #include "SkValidationUtils.h" @@ -102,26 +104,6 @@ static int32_t next_image_filter_unique_id() { return id; } -struct SkImageFilter::Cache::Key { - Key(const uint32_t uniqueID, const SkMatrix& matrix, const SkIRect& clipBounds, uint32_t srcGenID) - : fUniqueID(uniqueID), fMatrix(matrix), fClipBounds(clipBounds), fSrcGenID(srcGenID) { - // Assert that Key is tightly-packed, since it is hashed. - static_assert(sizeof(Key) == sizeof(uint32_t) + sizeof(SkMatrix) + sizeof(SkIRect) + - sizeof(uint32_t), "image_filter_key_tight_packing"); - fMatrix.getType(); // force initialization of type, so hashes match - } - uint32_t fUniqueID; - SkMatrix fMatrix; - SkIRect fClipBounds; - uint32_t fSrcGenID; - bool operator==(const Key& other) const { - return fUniqueID == other.fUniqueID - && fMatrix == other.fMatrix - && fClipBounds == other.fClipBounds - && fSrcGenID == other.fSrcGenID; - } -}; - SkImageFilter::Common::~Common() { for (int i = 0; i < fInputs.count(); ++i) { SkSafeUnref(fInputs[i]); @@ -237,7 +219,8 @@ bool SkImageFilter::filterImageDeprecated(Proxy* proxy, const SkBitmap& src, SkASSERT(result); SkASSERT(offset); uint32_t srcGenID = fUsesSrcInput ? src.getGenerationID() : 0; - Cache::Key key(fUniqueID, context.ctm(), context.clipBounds(), srcGenID); + Cache::Key key(fUniqueID, context.ctm(), context.clipBounds(), + srcGenID, SkIRect::MakeWH(0, 0)); if (context.cache()) { if (context.cache()->get(key, result, offset)) { return true; @@ -531,9 +514,8 @@ namespace { class CacheImpl : public SkImageFilter::Cache { public: - CacheImpl(size_t maxBytes) : fMaxBytes(maxBytes), fCurrentBytes(0) { - } - virtual ~CacheImpl() { + CacheImpl(size_t maxBytes) : fMaxBytes(maxBytes), fCurrentBytes(0) { } + ~CacheImpl() override { SkTDynamicHash::Iter iter(&fLookup); while (!iter.done()) { @@ -545,8 +527,12 @@ public: struct Value { Value(const Key& key, const SkBitmap& bitmap, const SkIPoint& offset) : fKey(key), fBitmap(bitmap), fOffset(offset) {} + Value(const Key& key, SkSpecialImage* image, const SkIPoint& offset) + : fKey(key), fImage(SkRef(image)), fOffset(offset) {} + Key fKey; SkBitmap fBitmap; + SkAutoTUnref fImage; SkIPoint fOffset; static const Key& GetKey(const Value& v) { return v.fKey; @@ -556,6 +542,7 @@ public: } SK_DECLARE_INTERNAL_LLIST_INTERFACE(Value); }; + bool get(const Key& key, SkBitmap* result, SkIPoint* offset) const override { SkAutoMutexAcquire mutex(fMutex); if (Value* v = fLookup.find(key)) { @@ -569,10 +556,24 @@ public: } return false; } + + SkSpecialImage* get(const Key& key, SkIPoint* offset) const override { + SkAutoMutexAcquire mutex(fMutex); + if (Value* v = fLookup.find(key)) { + *offset = v->fOffset; + if (v != fLRU.head()) { + fLRU.remove(v); + fLRU.addToHead(v); + } + return v->fImage; + } + return nullptr; + } + void set(const Key& key, const SkBitmap& result, const SkIPoint& offset) override { SkAutoMutexAcquire mutex(fMutex); if (Value* v = fLookup.find(key)) { - removeInternal(v); + this->removeInternal(v); } Value* v = new Value(key, result, offset); fLookup.add(v); @@ -584,7 +585,26 @@ public: if (tail == v) { break; } - removeInternal(tail); + this->removeInternal(tail); + } + } + + void set(const Key& key, SkSpecialImage* image, const SkIPoint& offset) override { + SkAutoMutexAcquire mutex(fMutex); + if (Value* v = fLookup.find(key)) { + this->removeInternal(v); + } + Value* v = new Value(key, image, offset); + fLookup.add(v); + fLRU.addToHead(v); + fCurrentBytes += image->getSize(); + while (fCurrentBytes > fMaxBytes) { + Value* tail = fLRU.tail(); + SkASSERT(tail); + if (tail == v) { + break; + } + this->removeInternal(tail); } } @@ -608,7 +628,11 @@ public: private: void removeInternal(Value* v) { - fCurrentBytes -= v->fBitmap.getSize(); + if (v->fImage) { + fCurrentBytes -= v->fImage->getSize(); + } else { + fCurrentBytes -= v->fBitmap.getSize(); + } fLRU.remove(v); fLookup.remove(v->fKey); delete v; diff --git a/src/core/SkImageFilterCacheKey.h b/src/core/SkImageFilterCacheKey.h new file mode 100644 index 0000000..76280db --- /dev/null +++ b/src/core/SkImageFilterCacheKey.h @@ -0,0 +1,42 @@ +/* + * Copyright 2016 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#ifndef SkImageFilterCacheKey_DEFINED +#define SkImageFilterCacheKey_DEFINED + +struct SkImageFilter::Cache::Key { + Key(const uint32_t uniqueID, const SkMatrix& matrix, + const SkIRect& clipBounds, uint32_t srcGenID, const SkIRect& srcSubset) + : fUniqueID(uniqueID) + , fMatrix(matrix) + , fClipBounds(clipBounds) + , fSrcGenID(srcGenID) + , fSrcSubset(srcSubset) { + // Assert that Key is tightly-packed, since it is hashed. + static_assert(sizeof(Key) == sizeof(uint32_t) + sizeof(SkMatrix) + sizeof(SkIRect) + + sizeof(uint32_t) + 4 * sizeof(int32_t), + "image_filter_key_tight_packing"); + fMatrix.getType(); // force initialization of type, so hashes match + } + + uint32_t fUniqueID; + SkMatrix fMatrix; + SkIRect fClipBounds; + uint32_t fSrcGenID; + SkIRect fSrcSubset; + + bool operator==(const Key& other) const { + return fUniqueID == other.fUniqueID && + fMatrix == other.fMatrix && + fClipBounds == other.fClipBounds && + fSrcGenID == other.fSrcGenID && + fSrcSubset == other.fSrcSubset; + } +}; + +#endif + diff --git a/src/core/SkSpecialImage.h b/src/core/SkSpecialImage.h index 8dd4a92..79fceeb 100644 --- a/src/core/SkSpecialImage.h +++ b/src/core/SkSpecialImage.h @@ -41,6 +41,8 @@ class SkSpecialImage : public SkRefCnt { public: int width() const { return fSubset.width(); } int height() const { return fSubset.height(); } + const SkIRect& subset() const { return fSubset; } + uint32_t uniqueID() const { return fUniqueID; } virtual bool isOpaque() const { return false; } virtual size_t getSize() const = 0; @@ -78,10 +80,9 @@ protected: , fProxy(proxy) { } - // The following 3 are for testing and shouldn't be used. (see skbug.com/4965) + // The following 2 are for testing and shouldn't be used. friend class TestingSpecialImageAccess; friend class TestingSpecialSurfaceAccess; - const SkIRect& subset() const { return fSubset; } /** * If the SpecialImage is backed by cpu pixels, return the const address diff --git a/tests/ImageFilterCacheTest.cpp b/tests/ImageFilterCacheTest.cpp new file mode 100644 index 0000000..f2e662f --- /dev/null +++ b/tests/ImageFilterCacheTest.cpp @@ -0,0 +1,208 @@ + /* + * Copyright 2016 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "Test.h" + +#include "SkBitmap.h" +#include "SkImage.h" +#include "SkImageFilter.h" +#include "SkImageFilterCacheKey.h" +#include "SkMatrix.h" +#include "SkSpecialImage.h" + +static const int kSmallerSize = 10; +static const int kPad = 3; +static const int kFullSize = kSmallerSize + 2 * kPad; + +static SkBitmap create_bm() { + SkBitmap bm; + bm.allocN32Pixels(kFullSize, kFullSize, true); + bm.eraseColor(SK_ColorTRANSPARENT); + return bm; +} + +// Ensure the cache can return a cached image +static void test_find_existing(skiatest::Reporter* reporter, + SkSpecialImage* image, + SkSpecialImage* subset) { + static const size_t kCacheSize = 1000000; + SkAutoTUnref cache(SkImageFilter::Cache::Create(kCacheSize)); + + SkIRect clip = SkIRect::MakeWH(100, 100); + SkImageFilter::Cache::Key key1(0, SkMatrix::I(), clip, image->uniqueID(), image->subset()); + SkImageFilter::Cache::Key key2(0, SkMatrix::I(), clip, subset->uniqueID(), subset->subset()); + + SkIPoint offset = SkIPoint::Make(3, 4); + cache->set(key1, image, offset); + + SkIPoint foundOffset; + + SkSpecialImage* foundImage = cache->get(key1, &foundOffset); + REPORTER_ASSERT(reporter, foundImage); + REPORTER_ASSERT(reporter, offset == foundOffset); + + REPORTER_ASSERT(reporter, !cache->get(key2, &foundOffset)); +} + +// If either id is different or the clip or the matrix are different the +// cached image won't be found. Even if it is caching the same bitmap. +static void test_dont_find_if_diff_key(skiatest::Reporter* reporter, + SkSpecialImage* image, + SkSpecialImage* subset) { + static const size_t kCacheSize = 1000000; + SkAutoTUnref cache(SkImageFilter::Cache::Create(kCacheSize)); + + SkIRect clip1 = SkIRect::MakeWH(100, 100); + SkIRect clip2 = SkIRect::MakeWH(200, 200); + SkImageFilter::Cache::Key key0(0, SkMatrix::I(), clip1, image->uniqueID(), image->subset()); + SkImageFilter::Cache::Key key1(1, SkMatrix::I(), clip1, image->uniqueID(), image->subset()); + SkImageFilter::Cache::Key key2(0, SkMatrix::MakeTrans(5, 5), clip1, + image->uniqueID(), image->subset()); + SkImageFilter::Cache::Key key3(0, SkMatrix::I(), clip2, image->uniqueID(), image->subset()); + SkImageFilter::Cache::Key key4(0, SkMatrix::I(), clip1, subset->uniqueID(), subset->subset()); + + SkIPoint offset = SkIPoint::Make(3, 4); + cache->set(key0, image, offset); + + SkIPoint foundOffset; + REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); + REPORTER_ASSERT(reporter, !cache->get(key2, &foundOffset)); + REPORTER_ASSERT(reporter, !cache->get(key3, &foundOffset)); + REPORTER_ASSERT(reporter, !cache->get(key4, &foundOffset)); +} + +// Test purging when the max cache size is exceeded +static void test_internal_purge(skiatest::Reporter* reporter, SkSpecialImage* image) { + SkASSERT(image->getSize()); + static const size_t kCacheSize = image->getSize() + 10; + SkAutoTUnref cache(SkImageFilter::Cache::Create(kCacheSize)); + + SkIRect clip = SkIRect::MakeWH(100, 100); + SkImageFilter::Cache::Key key1(0, SkMatrix::I(), clip, image->uniqueID(), image->subset()); + SkImageFilter::Cache::Key key2(1, SkMatrix::I(), clip, image->uniqueID(), image->subset()); + + SkIPoint offset = SkIPoint::Make(3, 4); + cache->set(key1, image, offset); + + SkIPoint foundOffset; + + REPORTER_ASSERT(reporter, cache->get(key1, &foundOffset)); + + // This should knock the first one out of the cache + cache->set(key2, image, offset); + + REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset)); + REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); +} + +// Exercise the purgeByKeys and purge methods +static void test_explicit_purging(skiatest::Reporter* reporter, + SkSpecialImage* image, + SkSpecialImage* subset) { + static const size_t kCacheSize = 1000000; + SkAutoTUnref cache(SkImageFilter::Cache::Create(kCacheSize)); + + SkIRect clip = SkIRect::MakeWH(100, 100); + SkImageFilter::Cache::Key key1(0, SkMatrix::I(), clip, image->uniqueID(), image->subset()); + SkImageFilter::Cache::Key key2(1, SkMatrix::I(), clip, subset->uniqueID(), image->subset()); + + SkIPoint offset = SkIPoint::Make(3, 4); + cache->set(key1, image, offset); + cache->set(key2, image, offset); + + SkIPoint foundOffset; + + REPORTER_ASSERT(reporter, cache->get(key1, &foundOffset)); + REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset)); + + cache->purgeByKeys(&key1, 1); + + REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); + REPORTER_ASSERT(reporter, cache->get(key2, &foundOffset)); + + cache->purge(); + + REPORTER_ASSERT(reporter, !cache->get(key1, &foundOffset)); + REPORTER_ASSERT(reporter, !cache->get(key2, &foundOffset)); +} + +DEF_TEST(ImageFilterCache_RasterBacked, reporter) { + SkBitmap srcBM = create_bm(); + + const SkIRect& full = SkIRect::MakeWH(kFullSize, kFullSize); + + SkAutoTUnref fullImg(SkSpecialImage::NewFromRaster(nullptr, full, srcBM)); + + const SkIRect& subset = SkIRect::MakeXYWH(kPad, kPad, kSmallerSize, kSmallerSize); + + SkAutoTUnref subsetImg(SkSpecialImage::NewFromRaster(nullptr, subset, srcBM)); + + test_find_existing(reporter, fullImg, subsetImg); + test_dont_find_if_diff_key(reporter, fullImg, subsetImg); + test_internal_purge(reporter, fullImg); + test_explicit_purging(reporter, fullImg, subsetImg); +} + +DEF_TEST(ImageFilterCache_ImageBacked, reporter) { + SkBitmap srcBM = create_bm(); + + SkAutoTUnref srcImage(SkImage::NewFromBitmap(srcBM)); + + const SkIRect& full = SkIRect::MakeWH(kFullSize, kFullSize); + + SkAutoTUnref fullImg(SkSpecialImage::NewFromImage(full, srcImage)); + + const SkIRect& subset = SkIRect::MakeXYWH(kPad, kPad, kSmallerSize, kSmallerSize); + + SkAutoTUnref subsetImg(SkSpecialImage::NewFromImage(subset, srcImage)); + + test_find_existing(reporter, fullImg, subsetImg); + test_dont_find_if_diff_key(reporter, fullImg, subsetImg); + test_internal_purge(reporter, fullImg); + test_explicit_purging(reporter, fullImg, subsetImg); +} + +#if SK_SUPPORT_GPU +#include "GrContext.h" + +DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ImageFilterCache_GPUBacked, reporter, context) { + SkBitmap srcBM = create_bm(); + + GrSurfaceDesc desc; + desc.fConfig = kSkia8888_GrPixelConfig; + desc.fFlags = kNone_GrSurfaceFlags; + desc.fWidth = kFullSize; + desc.fHeight = kFullSize; + + SkAutoTUnref srcTexture(context->textureProvider()->createTexture(desc, false, + srcBM.getPixels(), + 0)); + if (!srcTexture) { + return; + } + + const SkIRect& full = SkIRect::MakeWH(kFullSize, kFullSize); + + SkAutoTUnref fullImg(SkSpecialImage::NewFromGpu( + nullptr, full, + kNeedNewImageUniqueID_SpecialImage, + srcTexture)); + + const SkIRect& subset = SkIRect::MakeXYWH(kPad, kPad, kSmallerSize, kSmallerSize); + + SkAutoTUnref subsetImg(SkSpecialImage::NewFromGpu( + nullptr, subset, + kNeedNewImageUniqueID_SpecialImage, + srcTexture)); + + test_find_existing(reporter, fullImg, subsetImg); + test_dont_find_if_diff_key(reporter, fullImg, subsetImg); + test_internal_purge(reporter, fullImg); + test_explicit_purging(reporter, fullImg, subsetImg); +} +#endif +