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 (file)
--- 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 (file)
--- 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 (file)
--- 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<Value, Key>::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<SkSpecialImage> 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 (file)
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 (file)
--- 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 (file)
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<SkImageFilter::Cache> 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<SkImageFilter::Cache> 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<SkImageFilter::Cache> 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<SkImageFilter::Cache> 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<SkSpecialImage> fullImg(SkSpecialImage::NewFromRaster(nullptr, full, srcBM));
+
+    const SkIRect& subset = SkIRect::MakeXYWH(kPad, kPad, kSmallerSize, kSmallerSize);
+
+    SkAutoTUnref<SkSpecialImage> 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<SkImage> srcImage(SkImage::NewFromBitmap(srcBM));
+
+    const SkIRect& full = SkIRect::MakeWH(kFullSize, kFullSize);
+
+    SkAutoTUnref<SkSpecialImage> fullImg(SkSpecialImage::NewFromImage(full, srcImage));
+
+    const SkIRect& subset = SkIRect::MakeXYWH(kPad, kPad, kSmallerSize, kSmallerSize);
+
+    SkAutoTUnref<SkSpecialImage> 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<GrTexture> srcTexture(context->textureProvider()->createTexture(desc, false,
+                                                                                 srcBM.getPixels(),
+                                                                                 0));
+    if (!srcTexture) {
+        return;
+    }
+
+    const SkIRect& full = SkIRect::MakeWH(kFullSize, kFullSize);
+
+    SkAutoTUnref<SkSpecialImage> fullImg(SkSpecialImage::NewFromGpu(
+                                                                nullptr, full, 
+                                                                kNeedNewImageUniqueID_SpecialImage,
+                                                                srcTexture));
+
+    const SkIRect& subset = SkIRect::MakeXYWH(kPad, kPad, kSmallerSize, kSmallerSize);
+
+    SkAutoTUnref<SkSpecialImage> 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
+