Make GrCacheable implement its own ref counting.
R=robertphillips@google.com
Author: bsalomon@google.com
Review URL: https://codereview.chromium.org/392333008
diff --git a/gm/texdata.cpp b/gm/texdata.cpp
index ca82292..8f71141 100644
--- a/gm/texdata.cpp
+++ b/gm/texdata.cpp
@@ -91,7 +91,7 @@
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 f92154b..ce38c67 100644
--- a/include/core/SkRefCnt.h
+++ b/include/core/SkRefCnt.h
@@ -113,7 +113,6 @@
// 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 344ae6b..0efc7f7 100644
--- a/include/gpu/GrCacheable.h
+++ b/include/gpu/GrCacheable.h
@@ -15,9 +15,25 @@
/**
* 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 @@
protected:
GrCacheable()
- : fCacheEntry(NULL)
+ : fRefCnt(1)
+ , fCacheEntry(NULL)
, fGenID(0) {}
bool isInCache() const { return NULL != fCacheEntry; }
@@ -64,10 +81,11 @@
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 bdea2f9..92fa881 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -1119,7 +1119,7 @@
// 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 72d2f89..859b8b0 100644
--- a/include/gpu/GrGpuObject.h
+++ b/include/gpu/GrGpuObject.h
@@ -54,24 +54,6 @@
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 047bd18..14a198f 100644
--- a/include/gpu/GrTexture.h
+++ b/include/gpu/GrTexture.h
@@ -89,7 +89,6 @@
#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 4e833a1..a815263 100755
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -528,21 +528,14 @@
// 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 @@
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 529c3a5..ac10ebc 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -373,7 +373,7 @@
}
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 fb5a8d3..63069a4 100644
--- a/src/gpu/GrTexture.cpp
+++ b/src/gpu/GrTexture.cpp
@@ -29,7 +29,7 @@
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 @@
return;
}
- SkASSERT(0 == this->getDeferredRefCount());
this->INHERITED::internal_dispose();
}
diff --git a/tests/ClipCacheTest.cpp b/tests/ClipCacheTest.cpp
index 77f0137..d9f3b57 100644
--- a/tests/ClipCacheTest.cpp
+++ b/tests/ClipCacheTest.cpp
@@ -57,7 +57,7 @@
return;
}
- SkAutoUnref au(texture);
+ SkAutoTUnref<GrTexture> au(texture);
SkIRect intScreen = SkIRect::MakeWH(kXSize, kYSize);
SkRect screen;
@@ -164,14 +164,12 @@
// 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 @@
// 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 @@
// 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 5df1ab3..06ede48 100644
--- a/tests/ReadWriteAlphaTest.cpp
+++ b/tests/ReadWriteAlphaTest.cpp
@@ -45,7 +45,7 @@
return;
}
- SkAutoUnref au(texture);
+ SkAutoTUnref<GrTexture> au(texture);
// create a distinctive texture
for (int y = 0; y < Y_SIZE; ++y) {