From 19f0ed5b83a6ddf163a78bcb1c701f6020e6a17c Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Fri, 6 Jan 2017 13:54:58 -0500 Subject: [PATCH] Purge clip masks when they are no longer findable. This improves memory usage when the content contains frequently changing clips implemented as masks. BUG=chromium:676459 Change-Id: I06ea5f9fe1cff9564ea136bad9fe97f6ecd77ad9 Reviewed-on: https://skia-review.googlesource.com/6629 Commit-Queue: Brian Salomon Reviewed-by: Robert Phillips --- gn/core.gni | 2 +- include/core/SkClipStack.h | 46 +++++++++++++++++++------ {src/core => include/private}/SkMessageBus.h | 0 src/gpu/GrClipStackClip.cpp | 38 +++++++++++++++------ src/gpu/GrClipStackClip.h | 18 +++++----- tests/ClipStackTest.cpp | 50 ++++++++++++++++++++++++++++ tests/ResourceCacheTest.cpp | 1 - 7 files changed, 123 insertions(+), 32 deletions(-) rename {src/core => include/private}/SkMessageBus.h (100%) diff --git a/gn/core.gni b/gn/core.gni index 2e45753..e791f45 100644 --- a/gn/core.gni +++ b/gn/core.gni @@ -191,7 +191,6 @@ skia_core_sources = [ "$_src/core/SkMatrixImageFilter.cpp", "$_src/core/SkMatrixImageFilter.h", "$_src/core/SkMatrixUtils.h", - "$_src/core/SkMessageBus.h", "$_src/core/SkMetaData.cpp", "$_src/core/SkMipMap.cpp", "$_src/core/SkMipMap.h", @@ -454,6 +453,7 @@ skia_core_sources = [ "$_include/private/SkFloatBits.h", "$_include/private/SkFloatingPoint.h", "$_include/private/SkMiniRecorder.h", + "$_include/private/SkMessageBus.h", "$_include/private/SkMutex.h", "$_include/private/SkOnce.h", "$_include/private/SkRecords.h", diff --git a/include/core/SkClipStack.h b/include/core/SkClipStack.h index 55e0942..2f24d69 100644 --- a/include/core/SkClipStack.h +++ b/include/core/SkClipStack.h @@ -8,14 +8,19 @@ #ifndef SkClipStack_DEFINED #define SkClipStack_DEFINED +#include "../private/SkMessageBus.h" #include "SkCanvas.h" #include "SkDeque.h" #include "SkPath.h" -#include "SkRect.h" #include "SkRRect.h" +#include "SkRect.h" #include "SkRegion.h" #include "SkTLazy.h" +#if SK_SUPPORT_GPU +#include "GrResourceKey.h" +#endif + class SkCanvasClipVisitor; // Because a single save/restore state can have multiple clips, this class @@ -72,6 +77,14 @@ public: this->initPath(0, path, op, doAA); } + ~Element() { +#if SK_SUPPORT_GPU + for (int i = 0; i < fMessages.count(); ++i) { + SkMessageBus::Post(*fMessages[i]); + } +#endif + } + bool operator== (const Element& element) const; bool operator!= (const Element& element) const { return !(*this == element); } @@ -196,15 +209,26 @@ public: void dump() const; #endif +#if SK_SUPPORT_GPU + /** + * This is used to purge any GPU resource cache items that become unreachable when + * the element is destroyed because their key is based on this element's gen ID. + */ + void addResourceInvalidationMessage( + std::unique_ptr msg) const { + fMessages.emplace_back(std::move(msg)); + } +#endif + private: friend class SkClipStack; SkTLazy fPath; - SkRRect fRRect; - int fSaveCount; // save count of stack when this element was added. - SkClipOp fOp; - Type fType; - bool fDoAA; + SkRRect fRRect; + int fSaveCount; // save count of stack when this element was added. + SkClipOp fOp; + Type fType; + bool fDoAA; /* fFiniteBoundType and fFiniteBound are used to incrementally update the clip stack's bound. When fFiniteBoundType is kNormal_BoundsType, fFiniteBound represents the @@ -217,14 +241,16 @@ public: can capture the cancelling out of the extensions to infinity when two inverse filled clips are Booleaned together. */ SkClipStack::BoundsType fFiniteBoundType; - SkRect fFiniteBound; + SkRect fFiniteBound; // When element is applied to the previous elements in the stack is the result known to be // equivalent to a single rect intersection? IIOW, is the clip effectively a rectangle. - bool fIsIntersectionOfRects; - - int fGenID; + bool fIsIntersectionOfRects; + int fGenID; +#if SK_SUPPORT_GPU + mutable SkTArray> fMessages; +#endif Element(int saveCount) { this->initCommon(saveCount, SkClipOp::kReplace_deprecated, false); this->setEmpty(); diff --git a/src/core/SkMessageBus.h b/include/private/SkMessageBus.h similarity index 100% rename from src/core/SkMessageBus.h rename to include/private/SkMessageBus.h diff --git a/src/gpu/GrClipStackClip.cpp b/src/gpu/GrClipStackClip.cpp index a7bcce4..7b50d52 100644 --- a/src/gpu/GrClipStackClip.cpp +++ b/src/gpu/GrClipStackClip.cpp @@ -27,6 +27,7 @@ typedef GrReducedClip::InitialState InitialState; typedef GrReducedClip::ElementList ElementList; static const int kMaxAnalyticElements = 4; +const char GrClipStackClip::kMaskTestTag[] = "clip_mask"; bool GrClipStackClip::quickContains(const SkRect& rect) const { if (!fStack || fStack->isWideOpen()) { @@ -341,9 +342,9 @@ bool GrClipStackClip::apply(GrContext* context, GrRenderTargetContext* renderTar if (UseSWOnlyPath(context, hasUserStencilSettings, renderTargetContext, reducedClip)) { // The clip geometry is complex enough that it will be more efficient to create it // entirely in software - result = CreateSoftwareClipMask(context, reducedClip); + result = this->createSoftwareClipMask(context, reducedClip); } else { - result = CreateAlphaClipMask(context, reducedClip); + result = this->createAlphaClipMask(context, reducedClip); } if (result) { @@ -384,9 +385,9 @@ bool GrClipStackClip::apply(GrContext* context, GrRenderTargetContext* renderTar //////////////////////////////////////////////////////////////////////////////// // Create a 8-bit clip mask in alpha -static void GetClipMaskKey(int32_t clipGenID, const SkIRect& bounds, GrUniqueKey* key) { +static void create_clip_mask_key(int32_t clipGenID, const SkIRect& bounds, GrUniqueKey* key) { static const GrUniqueKey::Domain kDomain = GrUniqueKey::GenerateDomain(); - GrUniqueKey::Builder builder(key, kDomain, 3); + GrUniqueKey::Builder builder(key, kDomain, 3, GrClipStackClip::kMaskTestTag); builder[0] = clipGenID; // SkToS16 because image filters outset layers to a size indicated by the filter, which can // sometimes result in negative coordinates from clip space. @@ -394,11 +395,25 @@ static void GetClipMaskKey(int32_t clipGenID, const SkIRect& bounds, GrUniqueKey builder[2] = SkToS16(bounds.fTop) | (SkToS16(bounds.fBottom) << 16); } -sk_sp GrClipStackClip::CreateAlphaClipMask(GrContext* context, - const GrReducedClip& reducedClip) { +static void add_invalidate_on_pop_message(const SkClipStack& stack, int32_t clipGenID, + const GrUniqueKey& clipMaskKey) { + SkClipStack::Iter iter(stack, SkClipStack::Iter::kTop_IterStart); + while (const Element* element = iter.prev()) { + if (element->getGenID() == clipGenID) { + std::unique_ptr msg( + new GrUniqueKeyInvalidatedMessage(clipMaskKey)); + element->addResourceInvalidationMessage(std::move(msg)); + return; + } + } + SkDEBUGFAIL("Gen ID was not found in stack."); +} + +sk_sp GrClipStackClip::createAlphaClipMask(GrContext* context, + const GrReducedClip& reducedClip) const { GrResourceProvider* resourceProvider = context->resourceProvider(); GrUniqueKey key; - GetClipMaskKey(reducedClip.elementsGenID(), reducedClip.ibounds(), &key); + create_clip_mask_key(reducedClip.elementsGenID(), reducedClip.ibounds(), &key); if (GrTexture* texture = resourceProvider->findAndRefTextureByUniqueKey(key)) { return sk_sp(texture); } @@ -423,13 +438,14 @@ sk_sp GrClipStackClip::CreateAlphaClipMask(GrContext* context, } texture->resourcePriv().setUniqueKey(key); + add_invalidate_on_pop_message(*fStack, reducedClip.elementsGenID(), key); return texture; } -sk_sp GrClipStackClip::CreateSoftwareClipMask(GrContext* context, - const GrReducedClip& reducedClip) { +sk_sp GrClipStackClip::createSoftwareClipMask(GrContext* context, + const GrReducedClip& reducedClip) const { GrUniqueKey key; - GetClipMaskKey(reducedClip.elementsGenID(), reducedClip.ibounds(), &key); + create_clip_mask_key(reducedClip.elementsGenID(), reducedClip.ibounds(), &key); if (GrTexture* texture = context->textureProvider()->findAndRefTextureByUniqueKey(key)) { return sk_sp(texture); } @@ -493,6 +509,6 @@ sk_sp GrClipStackClip::CreateSoftwareClipMask(GrContext* context, } tex->resourcePriv().setUniqueKey(key); - + add_invalidate_on_pop_message(*fStack, reducedClip.elementsGenID(), key); return sk_ref_sp(tex); } diff --git a/src/gpu/GrClipStackClip.h b/src/gpu/GrClipStackClip.h index f5b8411..1c4d40b 100644 --- a/src/gpu/GrClipStackClip.h +++ b/src/gpu/GrClipStackClip.h @@ -40,6 +40,9 @@ public: bool isRRect(const SkRect& rtBounds, SkRRect* rr, GrAA* aa) const override; + sk_sp testingOnly_createClipMask(GrContext*) const; + static const char kMaskTestTag[]; + private: static bool PathNeedsSWRenderer(GrContext* context, bool hasUserStencilSettings, @@ -51,18 +54,15 @@ private: // Creates an alpha mask of the clip. The mask is a rasterization of elements through the // rect specified by clipSpaceIBounds. - static sk_sp CreateAlphaClipMask(GrContext*, const GrReducedClip&); + sk_sp createAlphaClipMask(GrContext*, const GrReducedClip&) const; // Similar to createAlphaClipMask but it rasterizes in SW and uploads to the result texture. - static sk_sp CreateSoftwareClipMask(GrContext*, const GrReducedClip&); - - static bool UseSWOnlyPath(GrContext*, - bool hasUserStencilSettings, - const GrRenderTargetContext*, - const GrReducedClip&); + sk_sp createSoftwareClipMask(GrContext*, const GrReducedClip&) const; - static GrTexture* CreateCachedMask(int width, int height, const GrUniqueKey& key, - bool renderTarget); + static bool UseSWOnlyPath(GrContext*, + bool hasUserStencilSettings, + const GrRenderTargetContext*, + const GrReducedClip&); SkIPoint fOrigin; sk_sp fStack; diff --git a/tests/ClipStackTest.cpp b/tests/ClipStackTest.cpp index bb5cc4a..a85f016 100644 --- a/tests/ClipStackTest.cpp +++ b/tests/ClipStackTest.cpp @@ -13,7 +13,9 @@ #include "SkRegion.h" #if SK_SUPPORT_GPU +#include "GrClipStackClip.h" #include "GrReducedClip.h" +#include "GrResourceCache.h" typedef GrReducedClip::ElementList ElementList; typedef GrReducedClip::InitialState InitialState; #endif @@ -1407,3 +1409,51 @@ DEF_TEST(ClipStack, reporter) { test_reduced_clip_stack_aa(reporter); #endif } + +////////////////////////////////////////////////////////////////////////////// + +#if SK_SUPPORT_GPU +sk_sp GrClipStackClip::testingOnly_createClipMask(GrContext* context) const { + const GrReducedClip reducedClip(*fStack, SkRect::MakeWH(512, 512), 0); + return this->createSoftwareClipMask(context, reducedClip); +} + +// Verify that clip masks are freed up when the clip state that generated them goes away. +DEF_GPUTEST_FOR_ALL_CONTEXTS(ClipMaskCache, reporter, ctxInfo) { + // This test uses resource key tags which only function in debug builds. +#ifdef SK_DEBUG + GrContext* context = ctxInfo.grContext(); + SkClipStack stack; + + SkPath path; + path.addCircle(10, 10, 8); + path.addCircle(15, 15, 8); + path.setFillType(SkPath::kEvenOdd_FillType); + + static const char* kTag = GrClipStackClip::kMaskTestTag; + GrResourceCache* cache = context->getResourceCache(); + + static constexpr int kN = 5; + + for (int i = 0; i < kN; ++i) { + SkMatrix m; + m.setTranslate(0.5, 0.5); + stack.save(); + stack.clipPath(path, m, SkClipOp::kIntersect, true); + auto mask = GrClipStackClip(&stack).testingOnly_createClipMask(context); + REPORTER_ASSERT(reporter, 0 == strcmp(mask->getUniqueKey().tag(), kTag)); + // Make sure mask isn't pinned in cache. + mask.reset(nullptr); + context->flush(); + REPORTER_ASSERT(reporter, i + 1 == cache->countUniqueKeysWithTag(kTag)); + } + + for (int i = 0; i < kN; ++i) { + stack.restore(); + cache->purgeAsNeeded(); + REPORTER_ASSERT(reporter, kN - (i + 1) == cache->countUniqueKeysWithTag(kTag)); + } +#endif +} + +#endif diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp index 1782093..39f64be 100644 --- a/tests/ResourceCacheTest.cpp +++ b/tests/ResourceCacheTest.cpp @@ -1482,5 +1482,4 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(GPUMemorySize, reporter, ctxInfo) { REPORTER_ASSERT(reporter, kSize*kSize*4+(kSize*kSize*4)/3 == size); } - #endif -- 2.7.4