Scratch textures are no longer removed from the cache in Debug

http://codereview.appspot.com/6465079/



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

include/gpu/GrContext.h

@@ -178,17 +178,6 @@ public:
      */
     void unlockTexture(GrTexture* texture);
 
-    /**
-     * Free any data associated with the provided entry in the texture cache.
-     * Currently this entry point is only used when a scratch texture is 
-     * detached from the cache. In this case the GrResourceEntry* associated
-     * with the texture needs to be freed since it will be re-allocated when
-     * the texture is re-added. This entry point will be removed soon since the
-     * texture can now carry around a pointer to its GrResourceEntry* (and
-     * will eventually take over its functionality).
-     */
-    void freeEntry(GrTexture* texture);
-
     /**
      * Creates a texture that is outside the cache. Does not count against
      * cache's budget.
@@ -858,9 +847,10 @@ public:
     GrTexture* detach() {
         GrTexture* temp = fTexture;
 
-        // freeEntry will remove the texture cache's ref
-        temp->ref();
-        fContext->freeEntry(fTexture);
+        // Conceptually the texture's cache entry loses its ref to the
+        // texture while the caller of this method gets a ref.
+        GrAssert(NULL != temp->getCacheEntry());
+
         fTexture = NULL;
 
         temp->setFlag((GrTextureFlags) GrTexture::kReturnToCache_FlagBit);

src/gpu/GrContext.cpp

@@ -470,10 +470,11 @@ GrTexture* GrContext::lockScratchTexture(const GrTextureDesc& inDesc,
 
     // If the caller gives us the same desc/sampler twice we don't want
     // to return the same texture the second time (unless it was previously
-    // released). So we detach the entry from the cache and reattach at release.
+    // released). So make it exclusive to hide it from future searches.
     if (NULL != resource) {
-        fTextureCache->detach(resource->getCacheEntry());
+        fTextureCache->makeExclusive(resource->getCacheEntry());
     }
+
     return static_cast<GrTexture*>(resource);
 }
 
@@ -483,14 +484,20 @@ void GrContext::addExistingTextureToCache(GrTexture* texture) {
         return;
     }
 
-    // 'texture' is a scratch texture returning to the fold
-    GrCacheData cacheData(GrCacheData::kScratch_CacheID);
+    // This texture should already have a cache entry since it was once
+    // attached
+    GrAssert(NULL != texture->getCacheEntry());
 
-    GrResourceKey key = GrTexture::ComputeKey(fGpu, NULL,
-                                              texture->desc(),
-                                              cacheData,
-                                              true);
-    fTextureCache->attach(key, texture);
+    // Conceptually, the cache entry is going to assume responsibility
+    // for the creation ref.
+    GrAssert(1 == texture->getRefCnt());
+
+    // Since this texture came from an AutoScratchTexture it should
+    // still be in the exclusive pile
+    fTextureCache->makeNonExclusive(texture->getCacheEntry());
+
+    // and it should still be locked
+    fTextureCache->unlock(texture->getCacheEntry());
 }
 
 void GrContext::unlockTexture(GrTexture* texture) {
@@ -501,18 +508,10 @@ void GrContext::unlockTexture(GrTexture* texture) {
     // while it was locked (to avoid two callers simultaneously getting
     // the same texture).
     if (GrTexture::IsScratchTexture(texture->getCacheEntry()->key())) {
-        fTextureCache->reattachAndUnlock(texture->getCacheEntry());
-    } else {
-        fTextureCache->unlock(texture->getCacheEntry());
+        fTextureCache->makeNonExclusive(texture->getCacheEntry());
     }
-}
 
-void GrContext::freeEntry(GrTexture* texture) {
-    ASSERT_OWNED_RESOURCE(texture);
-    GrAssert(NULL != texture->getCacheEntry());
-
-    fTextureCache->freeEntry(texture->getCacheEntry());
-    texture->setCacheEntry(NULL);
+    fTextureCache->unlock(texture->getCacheEntry());
 }
 
 GrTexture* GrContext::createUncachedTexture(const GrTextureDesc& descIn,

src/gpu/GrResourceCache.cpp

@@ -21,6 +21,7 @@ GrResourceEntry::GrResourceEntry(const GrResourceKey& key, GrResource* resource)
 }
 
 GrResourceEntry::~GrResourceEntry() {
+    fResource->setCacheEntry(NULL);
     fResource->unref();
 }
 
@@ -52,7 +53,6 @@ GrResourceCache::GrResourceCache(int maxCount, size_t maxBytes) :
     fClientDetachedCount          = 0;
     fClientDetachedBytes          = 0;
 
-    fHead = fTail = NULL;
     fPurging = false;
 }
 
@@ -84,26 +84,12 @@ void GrResourceCache::setLimits(int maxResources, size_t maxResourceBytes) {
 
 void GrResourceCache::internalDetach(GrResourceEntry* entry,
                                     bool clientDetach) {
-    GrResourceEntry* prev = entry->fPrev;
-    GrResourceEntry* next = entry->fNext;
+    fList.remove(entry);
 
-    if (prev) {
-        prev->fNext = next;
-    } else {
-        fHead = next;
-    }
-    if (next) {
-        next->fPrev = prev;
-    } else {
-        fTail = prev;
-    }
     if (!entry->isLocked()) {
         --fUnlockedEntryCount;
     }
 
-    entry->fPrev = NULL;
-    entry->fNext = NULL;
-
     // update our stats
     if (clientDetach) {
         fClientDetachedCount += 1;
@@ -126,15 +112,8 @@ void GrResourceCache::internalDetach(GrResourceEntry* entry,
 
 void GrResourceCache::attachToHead(GrResourceEntry* entry,
                                   bool clientReattach) {
-    entry->fPrev = NULL;
-    entry->fNext = fHead;
-    if (fHead) {
-        fHead->fPrev = entry;
-    }
-    fHead = entry;
-    if (NULL == fTail) {
-        fTail = entry;
-    }
+    fList.addToHead(entry);
+
     if (!entry->isLocked()) {
         ++fUnlockedEntryCount;
 #if GR_DEBUG
@@ -227,7 +206,6 @@ GrResourceEntry* GrResourceCache::create(const GrResourceKey& key,
     GrAutoResourceCacheValidate atcv(this);
 
     GrResourceEntry* entry = SkNEW_ARGS(GrResourceEntry, (key, resource));
-
     resource->setCacheEntry(entry);
 
     if (lock) {
@@ -260,35 +238,43 @@ void GrResourceCache::attach(const GrResourceKey& key,
     this->create(key, resource, false, true);
 }
 
-void GrResourceCache::detach(GrResourceEntry* entry) {
+void GrResourceCache::makeExclusive(GrResourceEntry* entry) {
     GrAutoResourceCacheValidate atcv(this);
+
     this->internalDetach(entry, true);
     fCache.remove(entry->fKey, entry);
+
+#if GR_DEBUG
+    fExclusiveList.addToHead(entry);
+#endif
 }
 
-void GrResourceCache::freeEntry(GrResourceEntry* entry) {
-    GrAssert(NULL == entry->fNext && NULL == entry->fPrev);
-    delete entry;
+void GrResourceCache::removeInvalidResource(GrResourceEntry* entry) {
+    // If the resource went invalid while it was detached then purge it
+    // This can happen when a 3D context was lost,
+    // the client called GrContext::contextDestroyed() to notify Gr,
+    // and then later an SkGpuDevice's destructor releases its backing
+    // texture (which was invalidated at contextDestroyed time).
+    fClientDetachedCount -= 1;
+    fEntryCount -= 1;
+    size_t size = entry->resource()->sizeInBytes();
+    fClientDetachedBytes -= size;
+    fEntryBytes -= size;
 }
 
-void GrResourceCache::reattachAndUnlock(GrResourceEntry* entry) {
+void GrResourceCache::makeNonExclusive(GrResourceEntry* entry) {
     GrAutoResourceCacheValidate atcv(this);
+
+#if GR_DEBUG
+    fExclusiveList.remove(entry);
+#endif
+
     if (entry->resource()->isValid()) {
         attachToHead(entry, true);
         fCache.insert(entry->key(), entry);
     } else {
-        // If the resource went invalid while it was detached then purge it
-        // This can happen when a 3D context was lost,
-        // the client called GrContext::contextDestroyed() to notify Gr,
-        // and then later an SkGpuDevice's destructor releases its backing
-        // texture (which was invalidated at contextDestroyed time).
-        fClientDetachedCount -= 1;
-        fEntryCount -= 1;
-        size_t size = entry->resource()->sizeInBytes();
-        fClientDetachedBytes -= size;
-        fEntryBytes -= size;
+        this->removeInvalidResource(entry);
     }
-    this->unlock(entry);
 }
 
 void GrResourceCache::unlock(GrResourceEntry* entry) {
@@ -325,7 +311,7 @@ void GrResourceCache::purgeAsNeeded() {
         fPurging = true;
         bool withinBudget = false;
         do {
-            GrResourceEntry* entry = fTail;
+            GrResourceEntry* entry = fList.fTail;
             while (entry && fUnlockedEntryCount) {
                 GrAutoResourceCacheValidate atcv(this);
                 if (fEntryCount <= fMaxCount && fEntryBytes <= fMaxBytes) {
@@ -347,6 +333,7 @@ void GrResourceCache::purgeAsNeeded() {
                              entry->resource()->width(),
                              entry->resource()->height());
         #endif
+
                     delete entry;
                 }
                 entry = prev;
@@ -370,6 +357,8 @@ void GrResourceCache::removeAll() {
     this->purgeAsNeeded();
 
 #if GR_DEBUG
+    GrAssert(fExclusiveList.countEntries() == fClientDetachedCount);
+    GrAssert(fExclusiveList.countBytes() == fClientDetachedBytes);
     GrAssert(!fUnlockedEntryCount);
     if (!fCache.count()) {
         // Items may have been detached from the cache (such as the backing
@@ -377,8 +366,7 @@ void GrResourceCache::removeAll() {
         // them.
         GrAssert(fEntryCount == fClientDetachedCount);
         GrAssert(fEntryBytes == fClientDetachedBytes);
-        GrAssert(NULL == fHead);
-        GrAssert(NULL == fTail);
+        GrAssert(fList.isEmpty());
     }
 #endif
 
@@ -401,14 +389,12 @@ static int countMatches(const GrResourceEntry* head, const GrResourceEntry* targ
     return count;
 }
 
-#if GR_DEBUG
 static bool both_zero_or_nonzero(int count, size_t bytes) {
     return (count == 0 && bytes == 0) || (count > 0 && bytes > 0);
 }
-#endif
 
 void GrResourceCache::validate() const {
-    GrAssert(!fHead == !fTail);
+    GrAssert(!fList.fHead == !fList.fTail);
     GrAssert(both_zero_or_nonzero(fEntryCount, fEntryBytes));
     GrAssert(both_zero_or_nonzero(fClientDetachedCount, fClientDetachedBytes));
     GrAssert(fClientDetachedBytes <= fEntryBytes);
@@ -417,32 +403,34 @@ void GrResourceCache::validate() const {
 
     fCache.validate();
 
-    GrResourceEntry* entry = fHead;
+    GrResourceEntry* entry = fList.fHead;
     int count = 0;
     int unlockCount = 0;
-    size_t bytes = 0;
     while (entry) {
         entry->validate();
         GrAssert(fCache.find(entry->key()));
         count += 1;
-        bytes += entry->resource()->sizeInBytes();
         if (!entry->isLocked()) {
             unlockCount += 1;
         }
         entry = entry->fNext;
     }
     GrAssert(count == fEntryCount - fClientDetachedCount);
+
+    size_t bytes = fList.countBytes();
     GrAssert(bytes == fEntryBytes  - fClientDetachedBytes);
+
+    bytes = fExclusiveList.countBytes();
+    GrAssert(bytes == fClientDetachedBytes);
+
     GrAssert(unlockCount == fUnlockedEntryCount);
 
-    count = 0;
-    for (entry = fTail; entry; entry = entry->fPrev) {
-        count += 1;
-    }
-    GrAssert(count == fEntryCount - fClientDetachedCount);
+    GrAssert(fList.countEntries() == fEntryCount - fClientDetachedCount);
+
+    GrAssert(fExclusiveList.countEntries() == fClientDetachedCount);
 
     for (int i = 0; i < count; i++) {
-        int matches = countMatches(fHead, fCache.getArray()[i]);
+        int matches = countMatches(fList.fHead, fCache.getArray()[i]);
         GrAssert(1 == matches);
     }
 }
@@ -462,3 +450,70 @@ void GrResourceCache::printStats() const {
 }
 
 #endif
+
+///////////////////////////////////////////////////////////////////////////////
+void GrDLinkedList::remove(GrResourceEntry* entry) {
+    GrAssert(this->isInList(entry));
+
+    GrResourceEntry* prev = entry->fPrev;
+    GrResourceEntry* next = entry->fNext;
+
+    if (prev) {
+        prev->fNext = next;
+    } else {
+        fHead = next;
+    }
+    if (next) {
+        next->fPrev = prev;
+    } else {
+        fTail = prev;
+    }
+
+    entry->fPrev = NULL;
+    entry->fNext = NULL;
+}
+
+void GrDLinkedList::addToHead(GrResourceEntry* entry) {
+    GrAssert(NULL == entry->fPrev && NULL == entry->fNext);
+
+    entry->fPrev = NULL;
+    entry->fNext = fHead;
+    if (fHead) {
+        fHead->fPrev = entry;
+    }
+    fHead = entry;
+    if (NULL == fTail) {
+        fTail = entry;
+    }
+}
+
+#if GR_DEBUG
+
+bool GrDLinkedList::isInList(const GrResourceEntry* entry) const {
+
+    for (GrResourceEntry* cur = fHead; NULL != cur; cur = cur->fNext) {
+        if (entry == cur) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+int GrDLinkedList::countEntries() const {
+    int count = 0;
+    for (GrResourceEntry* entry = fTail; NULL != entry; entry = entry->fPrev) {
+        ++count;
+    }
+    return count;
+}
+
+size_t GrDLinkedList::countBytes() const {
+    size_t bytes = 0;
+    for (GrResourceEntry* entry = fTail; NULL != entry; entry = entry->fPrev) {
+        bytes += entry->resource()->sizeInBytes();
+    }
+    return bytes;
+}
+
+#endif // GR_DEBUG

src/gpu/GrResourceCache.h

@@ -157,12 +157,34 @@ private:
     GrResourceEntry* fNext;
 
     friend class GrResourceCache;
+    friend class GrDLinkedList;
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "GrTHashCache.h"
 
+class GrDLinkedList {
+public:
+    GrDLinkedList()
+        : fHead(NULL)
+        , fTail(NULL) {
+    }
+
+    void remove(GrResourceEntry* entry);
+    void addToHead(GrResourceEntry* entry);
+
+#ifdef GR_DEBUG
+    bool isInList(const GrResourceEntry* entry) const;
+    bool isEmpty() const { return NULL == fHead && NULL == fTail; }
+    int countEntries() const;
+    size_t countBytes() const;
+#endif
+
+    GrResourceEntry* fHead;
+    GrResourceEntry* fTail;
+};
+
 /**
  *  Cache of GrResource objects.
  *
@@ -254,21 +276,18 @@ public:
     bool hasKey(const GrResourceKey& key) const;
 
     /**
-     * Detach removes an entry from the cache. This prevents the entry from
-     * being found by a subsequent findAndLock() until it is reattached. The
-     * entry still counts against the cache's budget and should be reattached
-     * when exclusive access is no longer needed.
+     * Hide 'entry' so that future searches will not find it. Such
+     * hidden entries will not be purged. The entry still counts against
+     * the cache's budget and should be made non-exclusive when exclusive access
+     * is no longer needed.
      */
-    void detach(GrResourceEntry*);
-
-    void freeEntry(GrResourceEntry* entry);
+    void makeExclusive(GrResourceEntry* entry);
 
     /**
-     * Reattaches a resource to the cache and unlocks it. Allows it to be found
-     * by a subsequent findAndLock or be purged (provided its lock count is
-     * now 0.)
+     * Restore 'entry' so that it can be found by future searches. 'entry'
+     * will also be purgeable (provided its lock count is now 0.)
      */
-    void reattachAndUnlock(GrResourceEntry*);
+    void makeNonExclusive(GrResourceEntry* entry);
 
     /**
      *  When done with an entry, call unlock(entry) on it, which returns it to
@@ -290,12 +309,18 @@ private:
     void attachToHead(GrResourceEntry*, bool);
     void purgeAsNeeded();
 
+    void removeInvalidResource(GrResourceEntry* entry);
+
     class Key;
     GrTHashTable<GrResourceEntry, Key, 8> fCache;
 
     // manage the dlink list
-    GrResourceEntry* fHead;
-    GrResourceEntry* fTail;
+    GrDLinkedList    fList;
+
+#if GR_DEBUG
+    // These objects cannot be returned by a search
+    GrDLinkedList    fExclusiveList;
+#endif
 
     // our budget, used in purgeAsNeeded()
     int fMaxCount;