Remove SkGpuDevice::fTexture, use new pixel ref class name

Review URL: https://codereview.appspot.com/6474068/


git-svn-id: http://skia.googlecode.com/svn/trunk@5307 2bbb7eff-a529-9590-31e7-b0007b416f81

include/core/SkTDLinkedList.h

@@ -95,6 +95,9 @@ public:
         return NULL == fHead && NULL == fTail;
     }
 
+    T* head() { return fHead; }
+    T* tail() { return fTail; }
+
     class Iter {
     public:
         enum IterStart {

include/gpu/GrResource.h

@@ -76,6 +76,8 @@ protected:
     virtual void onRelease() = 0;
     virtual void onAbandon() = 0;
 
+    bool isInCache() const { return NULL != fCacheEntry; }
+
 private:
 
     friend class GrGpu; // GrGpu manages list of resources.

include/gpu/SkGpuDevice.h

@@ -132,9 +132,8 @@ private:
     GrClipData      fClipData;
 
     // state for our offscreen render-target
-    // TODO: remove 'fCached' and let fTexture automatically return to the cache
+    // TODO: remove 'fCached' and automatically return to the cache
-    bool                fCached;        // is fTexture in the cache
+    bool                fCached;        // is fRenderTarget->asTexture() in the cache
-    GrTexture*          fTexture;
     GrRenderTarget*     fRenderTarget;
     bool                fNeedClear;
     bool                fNeedPrepareRenderTarget;

src/gpu/GrContext.cpp

@@ -136,7 +136,7 @@ void GrContext::contextDestroyed() {
 
     fAARectRenderer->reset();
 
-    fTextureCache->removeAll();
+    fTextureCache->purgeAllUnlocked();
     fFontCache->freeAll();
     fGpu->markContextDirty();
 }
@@ -152,7 +152,7 @@ void GrContext::freeGpuResources() {
 
     fAARectRenderer->reset();
 
-    fTextureCache->removeAll();
+    fTextureCache->purgeAllUnlocked();
     fFontCache->freeAll();
     // a path renderer may be holding onto resources
     GrSafeSetNull(fPathRendererChain);

src/gpu/GrResourceCache.cpp

@@ -35,6 +35,36 @@ void GrResourceEntry::validate() const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+class GrResourceCache::Key {
+    typedef GrResourceEntry T;
+
+    const GrResourceKey& fKey;
+public:
+    Key(const GrResourceKey& key) : fKey(key) {}
+
+    uint32_t getHash() const { return fKey.hashIndex(); }
+
+    static bool LT(const T& entry, const Key& key) {
+        return entry.key() < key.fKey;
+    }
+    static bool EQ(const T& entry, const Key& key) {
+        return entry.key() == key.fKey;
+    }
+#if GR_DEBUG
+    static uint32_t GetHash(const T& entry) {
+        return entry.key().hashIndex();
+    }
+    static bool LT(const T& a, const T& b) {
+        return a.key() < b.key();
+    }
+    static bool EQ(const T& a, const T& b) {
+        return a.key() == b.key();
+    }
+#endif
+};
+
+///////////////////////////////////////////////////////////////////////////////
+
 GrResourceCache::GrResourceCache(int maxCount, size_t maxBytes) :
         fMaxCount(maxCount),
         fMaxBytes(maxBytes) {
@@ -58,7 +88,20 @@ GrResourceCache::GrResourceCache(int maxCount, size_t maxBytes) :
 GrResourceCache::~GrResourceCache() {
     GrAutoResourceCacheValidate atcv(this);
 
-    this->removeAll();
+    EntryList::Iter iter;
+
+    // Unlike the removeAll, here we really remove everything, including locked resources.
+    while (GrResourceEntry* entry = fList.head()) {
+        GrAutoResourceCacheValidate atcv(this);
+
+        // remove from our cache
+        fCache.remove(entry->fKey, entry);
+
+        // remove from our llist
+        this->internalDetach(entry, false);
+
+        delete entry;
+    }
 }
 
 void GrResourceCache::getLimits(int* maxResources, size_t* maxResourceBytes) const{
@@ -141,34 +184,6 @@ void GrResourceCache::attachToHead(GrResourceEntry* entry,
     }
 }
 
-class GrResourceCache::Key {
-    typedef GrResourceEntry T;
-
-    const GrResourceKey& fKey;
-public:
-    Key(const GrResourceKey& key) : fKey(key) {}
-
-    uint32_t getHash() const { return fKey.hashIndex(); }
-
-    static bool LT(const T& entry, const Key& key) {
-        return entry.key() < key.fKey;
-    }
-    static bool EQ(const T& entry, const Key& key) {
-        return entry.key() == key.fKey;
-    }
-#if GR_DEBUG
-    static uint32_t GetHash(const T& entry) {
-        return entry.key().hashIndex();
-    }
-    static bool LT(const T& a, const T& b) {
-        return a.key() < b.key();
-    }
-    static bool EQ(const T& a, const T& b) {
-        return a.key() == b.key();
-    }
-#endif
-};
-
 GrResource* GrResourceCache::findAndLock(const GrResourceKey& key,
                                          LockType type) {
     GrAutoResourceCacheValidate atcv(this);
@@ -310,14 +325,13 @@ void GrResourceCache::purgeAsNeeded() {
         fPurging = true;
         bool withinBudget = false;
         do {
-            SkTDLinkedList<GrResourceEntry>::Iter iter;
+            EntryList::Iter iter;
 
             // Note: the following code relies on the fact that the
             // doubly linked list doesn't invalidate its data/pointers
             // outside of the specific area where a deletion occurs (e.g.,
             // in internalDetach)
-            GrResourceEntry* entry = iter.init(fList,
+            GrResourceEntry* entry = iter.init(fList, EntryList::Iter::kTail_IterStart);
-                    SkTDLinkedList<GrResourceEntry>::Iter::kTail_IterStart);
 
             while (entry && fUnlockedEntryCount) {
                 GrAutoResourceCacheValidate atcv(this);
@@ -350,7 +364,7 @@ void GrResourceCache::purgeAsNeeded() {
     }
 }
 
-void GrResourceCache::removeAll() {
+void GrResourceCache::purgeAllUnlocked() {
     GrAutoResourceCacheValidate atcv(this);
 
     // we can have one GrResource holding a lock on another
@@ -384,14 +398,13 @@ void GrResourceCache::removeAll() {
 ///////////////////////////////////////////////////////////////////////////////
 
 #if GR_DEBUG
-size_t GrResourceCache::countBytes(const SkTDLinkedList<GrResourceEntry>& list) {
+size_t GrResourceCache::countBytes(const EntryList& list) {
     size_t bytes = 0;
 
-    SkTDLinkedList<GrResourceEntry>::Iter iter;
+    EntryList::Iter iter;
 
-    const GrResourceEntry* entry = iter.init(
+    const GrResourceEntry* entry = iter.init(const_cast<EntryList&>(list),
-                  const_cast<SkTDLinkedList<GrResourceEntry>&>(list),
+                                             EntryList::Iter::kTail_IterStart);
-                  SkTDLinkedList<GrResourceEntry>::Iter::kTail_IterStart);
 
     for ( ; NULL != entry; entry = iter.prev()) {
         bytes += entry->resource()->sizeInBytes();
@@ -417,11 +430,10 @@ void GrResourceCache::validate() const {
     int count = 0;
     int unlockCount = 0;
 
-    SkTDLinkedList<GrResourceEntry>::Iter iter;
+    EntryList::Iter iter;
 
-    const GrResourceEntry* entry = iter.init(
+    const GrResourceEntry* entry = iter.init(const_cast<EntryList&>(fList),
-                  const_cast<SkTDLinkedList<GrResourceEntry>&>(fList),
+                                             EntryList::Iter::kHead_IterStart);
-                  SkTDLinkedList<GrResourceEntry>::Iter::kHead_IterStart);
 
     for ( ; NULL != entry; entry = iter.next()) {
         entry->validate();

src/gpu/GrResourceCache.h

@@ -269,7 +269,10 @@ public:
      */
     void unlock(GrResourceEntry*);
 
-    void removeAll();
+    /**
+     * Removes every resource in the cache that isn't locked.
+     */
+    void purgeAllUnlocked();
 
 #if GR_DEBUG
     void validate() const;
@@ -289,11 +292,12 @@ private:
     GrTHashTable<GrResourceEntry, Key, 8> fCache;
 
     // manage the dlink list
-    SkTDLinkedList<GrResourceEntry>    fList;
+    typedef SkTDLinkedList<GrResourceEntry> EntryList;
+    EntryList    fList;
 
 #if GR_DEBUG
     // These objects cannot be returned by a search
-    SkTDLinkedList<GrResourceEntry>    fExclusiveList;
+    EntryList    fExclusiveList;
 #endif
 
     // our budget, used in purgeAsNeeded()

src/gpu/GrStencilBuffer.cpp

@@ -41,7 +41,6 @@ void GrStencilBuffer::onRelease() {
         this->unlockInCache();
         // we shouldn't be deleted here because some RT still has a ref on us.
     }
-    fHoldingLock = false;
 }
 
 void GrStencilBuffer::onAbandon() {
@@ -50,12 +49,13 @@ void GrStencilBuffer::onAbandon() {
 }
 
 void GrStencilBuffer::unlockInCache() {
-    if (fHoldingLock) {
+    if (fHoldingLock && this->isInCache()) {
         GrGpu* gpu = this->getGpu();
         if (NULL != gpu) {
             GrAssert(NULL != gpu->getContext());
             gpu->getContext()->unlockStencilBuffer(this);
         }
+        fHoldingLock = false;
     }
 }
 

src/gpu/SkGpuDevice.cpp

@@ -189,28 +189,23 @@ void SkGpuDevice::initFromRenderTarget(GrContext* context,
     fContext->ref();
 
     fCached = false;
-    fTexture = NULL;
     fRenderTarget = NULL;
     fNeedClear = false;
 
     GrAssert(NULL != renderTarget);
     fRenderTarget = renderTarget;
     fRenderTarget->ref();
-    // if this RT is also a texture, hold a ref on it
-    fTexture = fRenderTarget->asTexture();
-    SkSafeRef(fTexture);
 
-    // Create a pixel ref for the underlying SkBitmap. We prefer a texture pixel
+    // Hold onto to the texture in the pixel ref (if there is one) because the texture holds a ref
-    // ref to a render target pixel reft. The pixel ref may get ref'ed outside
+    // on the RT but not vice-versa.
-    // the device via accessBitmap. This external ref may outlive the device.
+    // TODO: Remove this trickery once we figure out how to make SkGrPixelRef do this without
-    // Since textures own their render targets (but not vice-versa) we
+    // busting chrome (for a currently unknown reason).
-    // are ensuring that both objects will live as long as the pixel ref.
+    GrSurface* surface = fRenderTarget->asTexture();
-    SkPixelRef* pr;
+    if (NULL == surface) {
-    if (fTexture) {
+        surface = fRenderTarget;
-        pr = SkNEW_ARGS(SkGrTexturePixelRef, (fTexture));
-    } else {
-        pr = SkNEW_ARGS(SkGrRenderTargetPixelRef, (fRenderTarget));
     }
+    SkPixelRef* pr = SkNEW_ARGS(SkGrPixelRef, (surface));
+
     this->setPixelRef(pr, 0)->unref();
 }
 
@@ -227,7 +222,6 @@ SkGpuDevice::SkGpuDevice(GrContext* context,
     fContext->ref();
 
     fCached = false;
-    fTexture = NULL;
     fRenderTarget = NULL;
     fNeedClear = false;
 
@@ -243,16 +237,16 @@ SkGpuDevice::SkGpuDevice(GrContext* context,
     desc.fHeight = height;
     desc.fConfig = SkBitmapConfig2GrPixelConfig(bm.config());
 
-    fTexture = fContext->createUncachedTexture(desc, NULL, 0);
+    SkAutoTUnref<GrTexture> texture(fContext->createUncachedTexture(desc, NULL, 0));
 
-    if (NULL != fTexture) {
+    if (NULL != texture) {
-        fRenderTarget = fTexture->asRenderTarget();
+        fRenderTarget = texture->asRenderTarget();
         fRenderTarget->ref();
 
         GrAssert(NULL != fRenderTarget);
 
         // wrap the bitmap with a pixelref to expose our texture
-        SkGrTexturePixelRef* pr = SkNEW_ARGS(SkGrTexturePixelRef, (fTexture));
+        SkGrPixelRef* pr = SkNEW_ARGS(SkGrPixelRef, (texture));
         this->setPixelRef(pr, 0)->unref();
     } else {
         GrPrintf("--- failed to create gpu-offscreen [%d %d]\n",
@@ -270,12 +264,11 @@ SkGpuDevice::~SkGpuDevice() {
     // This call gives the context a chance to relinquish it
     fContext->setRenderTarget(NULL);
 
-    SkSafeUnref(fTexture);
+    GrTexture* texture = fRenderTarget->asTexture();
-    SkSafeUnref(fRenderTarget);
+    if (NULL != texture && fCached) {
-    if (NULL != fTexture && fCached) {
+        fContext->unlockTexture(texture);
-        GrAssert(fRenderTarget == fTexture->asRenderTarget());
-        fContext->unlockTexture(fTexture);
     }
+    SkSafeUnref(fRenderTarget);
     fContext->unref();
 }
 
@@ -485,9 +478,10 @@ SkGpuRenderTarget* SkGpuDevice::accessRenderTarget() {
 }
 
 bool SkGpuDevice::bindDeviceAsTexture(GrPaint* paint) {
-    if (NULL != fTexture) {
+    GrTexture* texture = fRenderTarget->asTexture();
+    if (NULL != texture) {
         paint->textureSampler(kBitmapTextureIdx)->setCustomStage(
-            SkNEW_ARGS(GrSingleTextureEffect, (fTexture)))->unref();
+            SkNEW_ARGS(GrSingleTextureEffect, (texture)))->unref();
         return true;
     }
     return false;