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

diff --git a/include/gpu/GrResource.h b/include/gpu/GrResource.h
index a3291e0..3fb2e77 100644 (file)
--- 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 3311d25..53458b3 100644 (file)
--- 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 91ffe79..8b43906 100644 (file)
--- 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 9407688..fab1f58 100644 (file)
--- 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());
 }
 
 ////////////////////////////////////////////////////////////////////////////////