Make GrCacheable implement its own ref counting.

R=robertphillips@google.com

Author: bsalomon@google.com

Review URL: https://codereview.chromium.org/392333008

11 files changed, 31 insertions, 46 deletions

diff --git a/gm/texdata.cpp b/gm/texdata.cpp
index ca82292f2f..8f7114168c 100644
--- a/gm/texdata.cpp
+++ b/gm/texdata.cpp
@@ -91,7 +91,7 @@ protected:
                 if (!texture) {
                     return;
                 }
-                SkAutoUnref au(texture);
+                SkAutoTUnref<GrTexture> au(texture);
 
                 GrContext::AutoClip acs(ctx, SkRect::MakeWH(2*S, 2*S));
 
diff --git a/include/core/SkRefCnt.h b/include/core/SkRefCnt.h
index f92154bc2f..ce38c67bd5 100644
--- a/include/core/SkRefCnt.h
+++ b/include/core/SkRefCnt.h
@@ -113,7 +113,6 @@ private:
 
     // The following friends are those which override internal_dispose()
     // and conditionally call SkRefCnt::internal_dispose().
-    friend class GrTexture;
     friend class SkWeakRefCnt;
 
     mutable int32_t fRefCnt;
diff --git a/include/gpu/GrCacheable.h b/include/gpu/GrCacheable.h
index 344ae6b583..0efc7f7d61 100644
--- a/include/gpu/GrCacheable.h
+++ b/include/gpu/GrCacheable.h
@@ -15,9 +15,25 @@ class GrResourceCacheEntry;
 /**
  * Base class for objects that can be kept in the GrResourceCache.
  */
-class GrCacheable : public SkRefCnt {
+class GrCacheable : public SkNoncopyable {
 public:
-    SK_DECLARE_INST_COUNT(GrCacheable)
+    SK_DECLARE_INST_COUNT_ROOT(GrCacheable)
+
+    // These method signatures are written to mirror SkRefCnt. However, we don't require
+    // thread safety as GrCacheable objects are not intended to cross thread boundaries.
+    // internal_dispose() exists because of GrTexture's reliance on it. It will be removed
+    // soon.
+    void ref() const { ++fRefCnt; }
+    void unref() const { --fRefCnt; if (0 == fRefCnt) { this->internal_dispose(); } }
+    virtual void internal_dispose() const { SkDELETE(this); }
+    bool unique() const { return 1 == fRefCnt; }
+#ifdef SK_DEBUG
+    void validate() const {
+        SkASSERT(fRefCnt > 0);
+    }
+#endif
+
+    virtual ~GrCacheable() { SkASSERT(0 == fRefCnt); }
 
     /**
      * Retrieves the amount of GPU memory used by this resource in bytes. It is
@@ -50,7 +66,8 @@ public:
 
 protected:
     GrCacheable()
-        : fCacheEntry(NULL)
+        : fRefCnt(1)
+        , fCacheEntry(NULL)
         , fGenID(0) {}
 
     bool isInCache() const { return NULL != fCacheEntry; }
@@ -64,10 +81,11 @@ protected:
     void didChangeGpuMemorySize() const;
 
 private:
+    mutable int32_t         fRefCnt;
     GrResourceCacheEntry*   fCacheEntry;  // NULL if not in cache
     mutable uint32_t        fGenID;
 
-    typedef SkRefCnt INHERITED;
+    typedef SkNoncopyable INHERITED;
 };
 
 #endif
diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index bdea2f94ce..92fa881f7b 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -1119,7 +1119,7 @@ public:
         // The cache also has a ref which we are lending to the caller of detach(). When the caller
         // lets go of the ref and the ref count goes to 0 internal_dispose will see this flag is
         // set and re-ref the texture, thereby restoring the cache's ref.
-        SkASSERT(texture->getRefCnt() > 1);
+        SkASSERT(!texture->unique());
         texture->impl()->setFlag((GrTextureFlags) GrTextureImpl::kReturnToCache_FlagBit);
         texture->unref();
         SkASSERT(NULL != texture->getCacheEntry());
diff --git a/include/gpu/GrGpuObject.h b/include/gpu/GrGpuObject.h
index 72d2f892be..859b8b015e 100644
--- a/include/gpu/GrGpuObject.h
+++ b/include/gpu/GrGpuObject.h
@@ -54,24 +54,6 @@ public:
     const GrContext* getContext() const;
     GrContext* getContext();
 
-    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; }
-
     virtual bool isValidOnGpu() const SK_OVERRIDE { return !this->wasDestroyed(); }
 
 protected:
diff --git a/include/gpu/GrTexture.h b/include/gpu/GrTexture.h
index 047bd18186..14a198fede 100644
--- a/include/gpu/GrTexture.h
+++ b/include/gpu/GrTexture.h
@@ -89,7 +89,6 @@ public:
 #ifdef SK_DEBUG
     void validate() const {
         this->INHERITED::validate();
-
         this->validateDesc();
     }
 #endif
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 4e833a1f96..a815263d13 100755
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -528,21 +528,14 @@ void GrContext::addExistingTextureToCache(GrTexture* texture) {
         // still be in the exclusive pile. Recycle it.
         fResourceCache->makeNonExclusive(texture->getCacheEntry());
         this->purgeCache();
-    } else if (texture->getDeferredRefCount() <= 0) {
+    } else {
         // When we aren't reusing textures we know this scratch texture
         // will never be reused and would be just wasting time in the cache
         fResourceCache->makeNonExclusive(texture->getCacheEntry());
         fResourceCache->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();
     }
 }
 
-
 void GrContext::unlockScratchTexture(GrTexture* texture) {
     ASSERT_OWNED_RESOURCE(texture);
     SkASSERT(NULL != texture->getCacheEntry());
@@ -554,14 +547,14 @@ void GrContext::unlockScratchTexture(GrTexture* texture) {
         if (fGpu->caps()->reuseScratchTextures() || NULL != texture->asRenderTarget()) {
             fResourceCache->makeNonExclusive(texture->getCacheEntry());
             this->purgeCache();
-        } else if (texture->unique() && texture->getDeferredRefCount() <= 0) {
+        } else if (texture->unique()) {
             // 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.
             fResourceCache->makeNonExclusive(texture->getCacheEntry());
             fResourceCache->deleteResource(texture->getCacheEntry());
         } else {
-            // In this case (fRefCnt > 1 || defRefCnt > 0) but we don't really
+            // In this case (there is still a non-cache ref) 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
diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp
index 529c3a5d1d..ac10ebc641 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -373,7 +373,7 @@ void GrResourceCache::purgeInvalidated() {
 }
 
 void GrResourceCache::deleteResource(GrResourceCacheEntry* entry) {
-    SkASSERT(1 == entry->fResource->getRefCnt());
+    SkASSERT(entry->fResource->unique());
 
     // remove from our cache
     fCache.remove(entry->key(), entry);
diff --git a/src/gpu/GrTexture.cpp b/src/gpu/GrTexture.cpp
index fb5a8d350e..63069a452b 100644
--- a/src/gpu/GrTexture.cpp
+++ b/src/gpu/GrTexture.cpp
@@ -29,7 +29,7 @@ void GrTexture::internal_dispose() const {
     if (this->impl()->isSetFlag((GrTextureFlags) GrTextureImpl::kReturnToCache_FlagBit) &&
         NULL != this->INHERITED::getContext()) {
         GrTexture* nonConstThis = const_cast<GrTexture *>(this);
-        this->fRefCnt = 1;      // restore ref count to initial setting
+        this->ref(); // restore ref count to initial setting
 
         nonConstThis->impl()->resetFlag((GrTextureFlags) GrTextureImpl::kReturnToCache_FlagBit);
         nonConstThis->INHERITED::getContext()->addExistingTextureToCache(nonConstThis);
@@ -39,7 +39,6 @@ void GrTexture::internal_dispose() const {
         return;
     }
 
-    SkASSERT(0 == this->getDeferredRefCount());
     this->INHERITED::internal_dispose();
 }
 
diff --git a/tests/ClipCacheTest.cpp b/tests/ClipCacheTest.cpp
index 77f0137d5a..d9f3b578b8 100644
--- a/tests/ClipCacheTest.cpp
+++ b/tests/ClipCacheTest.cpp
@@ -57,7 +57,7 @@ static void test_clip_bounds(skiatest::Reporter* reporter, GrContext* context) {
         return;
     }
 
-    SkAutoUnref au(texture);
+    SkAutoTUnref<GrTexture> au(texture);
 
     SkIRect intScreen = SkIRect::MakeWH(kXSize, kYSize);
     SkRect screen;
@@ -164,14 +164,12 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
 
     // check that the set took
     check_state(reporter, cache, clip1, texture1, bound1);
-    REPORTER_ASSERT(reporter, texture1->getRefCnt());
 
     // push the state
     cache.push();
 
     // verify that the pushed state is initially empty
     check_empty_state(reporter, cache);
-    REPORTER_ASSERT(reporter, texture1->getRefCnt());
 
     // modify the new state
     SkIRect bound2;
@@ -189,8 +187,6 @@ static void test_cache(skiatest::Reporter* reporter, GrContext* context) {
 
     // check that the changes took
     check_state(reporter, cache, clip2, texture2, bound2);
-    REPORTER_ASSERT(reporter, texture1->getRefCnt());
-    REPORTER_ASSERT(reporter, texture2->getRefCnt());
 
     // check to make sure canReuse works
     REPORTER_ASSERT(reporter, cache.canReuse(clip2.getTopmostGenID(), bound2));
@@ -201,7 +197,6 @@ 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());
 
     // manually clear the state
     cache.reset();
diff --git a/tests/ReadWriteAlphaTest.cpp b/tests/ReadWriteAlphaTest.cpp
index 5df1ab395f..06ede48afd 100644
--- a/tests/ReadWriteAlphaTest.cpp
+++ b/tests/ReadWriteAlphaTest.cpp
@@ -45,7 +45,7 @@ DEF_GPUTEST(ReadWriteAlpha, reporter, factory) {
             return;
         }
 
-        SkAutoUnref au(texture);
+        SkAutoTUnref<GrTexture> au(texture);
 
         // create a distinctive texture
         for (int y = 0; y < Y_SIZE; ++y) {