From 9ef0426e7c126f6ad6ba833d4543b92a197c95af Mon Sep 17 00:00:00 2001 From: "robertphillips@google.com" Date: Tue, 29 Oct 2013 14:06:15 +0000 Subject: [PATCH] Don't reuse scratch textures patch https://codereview.chromium.org/24222004/ git-svn-id: http://skia.googlecode.com/svn/trunk@11997 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/gpu/GrResource.h | 36 ++++++++++++++++++++++++++++++++---- src/gpu/GrContext.cpp | 39 +++++++++++++++++++++++++++++++-------- src/gpu/GrResource.cpp | 2 +- tests/ClipCacheTest.cpp | 5 ----- 4 files changed, 64 insertions(+), 18 deletions(-) diff --git a/include/gpu/GrResource.h b/include/gpu/GrResource.h index a3291e0c05..3fb2e7765d 100644 --- a/include/gpu/GrResource.h +++ b/include/gpu/GrResource.h @@ -66,10 +66,24 @@ public: void setCacheEntry(GrResourceEntry* cacheEntry) { fCacheEntry = cacheEntry; } GrResourceEntry* getCacheEntry() { return fCacheEntry; } - void incDeferredRefCount() const { SkASSERT(fDeferredRefCount >= 0); ++fDeferredRefCount; } - void decDeferredRefCount() const { SkASSERT(fDeferredRefCount > 0); --fDeferredRefCount; } + void incDeferredRefCount() const { + SkASSERT(fDeferredRefCount >= 0); + ++fDeferredRefCount; + } + + void decDeferredRefCount() const { + SkASSERT(fDeferredRefCount > 0); + --fDeferredRefCount; + if (0 == fDeferredRefCount && this->needsDeferredUnref()) { + SkASSERT(this->getRefCnt() > 1); + this->unref(); + } + } + int getDeferredRefCount() const { return fDeferredRefCount; } + void setNeedsDeferredUnref() { fFlags |= kDeferredUnref_FlagBit; } + protected: /** * isWrapped indicates we have wrapped a client-created backend resource in a GrResource. If it @@ -87,7 +101,8 @@ protected: virtual void onAbandon() {}; bool isInCache() const { return NULL != fCacheEntry; } - bool isWrapped() const { return kWrapped_Flag & fFlags; } + bool isWrapped() const { return kWrapped_FlagBit & fFlags; } + bool needsDeferredUnref() const { return SkToBool(kDeferredUnref_FlagBit & fFlags); } private: #ifdef SK_DEBUG @@ -105,7 +120,20 @@ private: mutable int fDeferredRefCount; // How many references in deferred drawing buffers. enum Flags { - kWrapped_Flag = 0x1, + /** + * This resource wraps a GPU resource given to us by the user. + * Lifetime management is left up to the user (i.e., we will not + * free it). + */ + kWrapped_FlagBit = 0x1, + + /** + * This texture should be de-refed when the deferred ref count goes + * to zero. A resource gets into this state when the resource cache + * is holding a ref-of-obligation (i.e., someone needs to own it but + * no one else wants to) but doesn't really want to keep it around. + */ + kDeferredUnref_FlagBit = 0x2, }; uint32_t fFlags; diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 3311d255b1..53458b3b32 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -511,19 +511,25 @@ void GrContext::addExistingTextureToCache(GrTexture* texture) { SkASSERT(NULL != texture->getCacheEntry()); // Conceptually, the cache entry is going to assume responsibility - // for the creation ref. + // for the creation ref. Assert refcnt == 1. SkASSERT(texture->unique()); - // Since this texture came from an AutoScratchTexture it should - // still be in the exclusive pile - fTextureCache->makeNonExclusive(texture->getCacheEntry()); - if (fGpu->caps()->reuseScratchTextures()) { + // Since this texture came from an AutoScratchTexture it should + // still be in the exclusive pile. Recycle it. + fTextureCache->makeNonExclusive(texture->getCacheEntry()); this->purgeCache(); - } else { + } else if (texture->getDeferredRefCount() <= 0) { // When we aren't reusing textures we know this scratch texture // will never be reused and would be just wasting time in the cache + fTextureCache->makeNonExclusive(texture->getCacheEntry()); fTextureCache->deleteResource(texture->getCacheEntry()); + } else { + // In this case (fDeferredRefCount > 0) but the cache is the only + // one holding a real ref. Mark the object so when the deferred + // ref count goes to 0 the texture will be deleted (remember + // in this code path scratch textures aren't getting reused). + texture->setNeedsDeferredUnref(); } } @@ -536,8 +542,25 @@ void GrContext::unlockScratchTexture(GrTexture* texture) { // while it was locked (to avoid two callers simultaneously getting // the same texture). if (texture->getCacheEntry()->key().isScratch()) { - fTextureCache->makeNonExclusive(texture->getCacheEntry()); - this->purgeCache(); + if (fGpu->caps()->reuseScratchTextures()) { + fTextureCache->makeNonExclusive(texture->getCacheEntry()); + this->purgeCache(); + } else if (texture->unique() && texture->getDeferredRefCount() <= 0) { + // Only the cache now knows about this texture. Since we're never + // reusing scratch textures (in this code path) it would just be + // wasting time sitting in the cache. + fTextureCache->makeNonExclusive(texture->getCacheEntry()); + fTextureCache->deleteResource(texture->getCacheEntry()); + } else { + // In this case (fRefCnt > 1 || defRefCnt > 0) but we don't really + // want to readd it to the cache (since it will never be reused). + // Instead, give up the cache's ref and leave the decision up to + // addExistingTextureToCache once its ref count reaches 0. For + // this to work we need to leave it in the exclusive list. + texture->setFlag((GrTextureFlags) GrTexture::kReturnToCache_FlagBit); + // Give up the cache's ref to the texture + texture->unref(); + } } } diff --git a/src/gpu/GrResource.cpp b/src/gpu/GrResource.cpp index 91ffe79ceb..8b43906164 100644 --- a/src/gpu/GrResource.cpp +++ b/src/gpu/GrResource.cpp @@ -17,7 +17,7 @@ GrResource::GrResource(GrGpu* gpu, bool isWrapped) { fCacheEntry = NULL; fDeferredRefCount = 0; if (isWrapped) { - fFlags = kWrapped_Flag; + fFlags = kWrapped_FlagBit; } else { fFlags = 0; } diff --git a/tests/ClipCacheTest.cpp b/tests/ClipCacheTest.cpp index 9407688f2d..fab1f58dd7 100644 --- a/tests/ClipCacheTest.cpp +++ b/tests/ClipCacheTest.cpp @@ -197,15 +197,12 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) { // verify that the old state is restored check_state(reporter, cache, clip1, texture1, bound1); REPORTER_ASSERT(reporter, texture1->getRefCnt()); - REPORTER_ASSERT(reporter, texture2->getRefCnt()); // manually clear the state cache.reset(); // verify it is now empty check_state(reporter, cache, emptyClip, NULL, emptyBound); - REPORTER_ASSERT(reporter, texture1->getRefCnt()); - REPORTER_ASSERT(reporter, texture2->getRefCnt()); // pop again - so there is no state cache.pop(); @@ -215,8 +212,6 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) { // only do in release since it generates asserts in debug check_state(reporter, cache, emptyClip, NULL, emptyBound); #endif - REPORTER_ASSERT(reporter, texture1->getRefCnt()); - REPORTER_ASSERT(reporter, texture2->getRefCnt()); } ////////////////////////////////////////////////////////////////////////////////