Fix for GrTextureStripAtlas memory leak

https://codereview.appspot.com/6549050/



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

4 files changed, 94 insertions, 21 deletions

diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index 281be8d749..3c38fad9e2 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -63,6 +63,24 @@ public:
     void resetContext();
 
     /**
+     * Callback function to allow classes to cleanup on GrContext destruction.
+     * The 'info' field is filled in with the 'info' passed to addCleanUp.
+     */
+    typedef void (*PFCleanUpFunc)(const GrContext* context, void* info);
+
+    /**
+     * Add a function to be called from within GrContext's destructor.
+     * This gives classes a chance to free resources held on a per context basis.
+     * The 'info' parameter will be stored and passed to the callback function.
+     */
+    void addCleanUp(PFCleanUpFunc cleanUp, void* info) {
+        CleanUpData* entry = fCleanUpData.push();
+
+        entry->fFunc = cleanUp;
+        entry->fInfo = info;
+    }
+
+    /**
      * Abandons all gpu resources, assumes 3D API state is unknown. Call this
      * if you have lost the associated GPU context, and thus internal texture,
      * buffer, etc. references/IDs are now invalid. Should be called even when
@@ -806,6 +824,13 @@ private:
     int                         fPMToUPMConversion;
     int                         fUPMToPMConversion;
 
+    struct CleanUpData {
+        PFCleanUpFunc fFunc;
+        void*         fInfo;
+    };
+
+    SkTDArray<CleanUpData>      fCleanUpData;
+
     GrContext(GrGpu* gpu);
 
     void setupDrawBuffer();
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 05ca687713..f35e729e24 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -88,6 +88,10 @@ int GrContext::GetThreadInstanceCount() {
 }
 
 GrContext::~GrContext() {
+    for (int i = 0; i < fCleanUpData.count(); ++i) {
+        (*fCleanUpData[i].fFunc)(this, fCleanUpData[i].fInfo);
+    }
+
     this->flush();
 
     // Since the gpu can hold scratch textures, give it a chance to let go
diff --git a/src/gpu/effects/GrTextureStripAtlas.cpp b/src/gpu/effects/GrTextureStripAtlas.cpp
index 3a2942b375..f39e20bccd 100644
--- a/src/gpu/effects/GrTextureStripAtlas.cpp
+++ b/src/gpu/effects/GrTextureStripAtlas.cpp
@@ -9,7 +9,6 @@
 #include "GrTextureStripAtlas.h"
 #include "SkPixelRef.h"
 #include "SkTSearch.h"
-#include "GrBinHashKey.h"
 #include "GrTexture.h"
 
 #ifdef SK_DEBUG
@@ -20,35 +19,57 @@
 
 GR_DEFINE_RESOURCE_CACHE_DOMAIN(GrTextureStripAtlas, GetTextureStripAtlasDomain)
 
+
 int32_t GrTextureStripAtlas::gCacheCount = 0;
 
-// Hash table entry for atlases
-class AtlasEntry;
-typedef GrTBinHashKey<AtlasEntry, sizeof(GrTextureStripAtlas::Desc)> AtlasHashKey;
-class AtlasEntry : public ::GrNoncopyable {
-public:
-    AtlasEntry() : fAtlas(NULL) {}
-    ~AtlasEntry() { SkDELETE(fAtlas); }
-    int compare(const AtlasHashKey& key) const { return fKey.compare(key); }
-    AtlasHashKey fKey;
-    GrTextureStripAtlas* fAtlas;
-};
+GrTHashTable<GrTextureStripAtlas::AtlasEntry, 
+                GrTextureStripAtlas::AtlasHashKey, 8>* 
+                            GrTextureStripAtlas::gAtlasCache = NULL;
+
+GrTHashTable<GrTextureStripAtlas::AtlasEntry, GrTextureStripAtlas::AtlasHashKey, 8>*
+GrTextureStripAtlas::GetCache() {
+
+    if (NULL == gAtlasCache) {
+        gAtlasCache = SkNEW((GrTHashTable<AtlasEntry, AtlasHashKey, 8>));
+    }
+
+    return gAtlasCache;
+}
+
+// Remove the specified atlas from the cache
+void GrTextureStripAtlas::CleanUp(const GrContext* context, void* info) {
+    GrAssert(NULL != info);
+
+    AtlasEntry* entry = static_cast<AtlasEntry*>(info);
+
+    // remove the cache entry
+    GetCache()->remove(entry->fKey, entry);
+
+    // remove the actual entry
+    SkDELETE(entry);
+
+    if (0 == GetCache()->count()) {
+        SkDELETE(gAtlasCache);
+        gAtlasCache = NULL;
+    }
+}
 
 GrTextureStripAtlas* GrTextureStripAtlas::GetAtlas(const GrTextureStripAtlas::Desc& desc) {
-    static SkTDArray<AtlasEntry> gAtlasEntries;
-    static GrTHashTable<AtlasEntry, AtlasHashKey, 8> gAtlasCache;
     AtlasHashKey key;
     key.setKeyData(desc.asKey());
-    AtlasEntry* entry = gAtlasCache.find(key);
-    if (NULL != entry) {
-        return entry->fAtlas;
-    } else {
-        entry = gAtlasEntries.push();
+    AtlasEntry* entry = GetCache()->find(key);
+    if (NULL == entry) {
+        entry = SkNEW(AtlasEntry);
+
         entry->fAtlas = SkNEW_ARGS(GrTextureStripAtlas, (desc));
         entry->fKey = key;
-        gAtlasCache.insert(key, entry);
-        return entry->fAtlas;
+
+        desc.fContext->addCleanUp(CleanUp, entry);
+
+        GetCache()->insert(key, entry);
     }
+
+    return entry->fAtlas;
 }
 
 GrTextureStripAtlas::GrTextureStripAtlas(GrTextureStripAtlas::Desc desc)
diff --git a/src/gpu/effects/GrTextureStripAtlas.h b/src/gpu/effects/GrTextureStripAtlas.h
index 1a4c371b69..a6833e0201 100644
--- a/src/gpu/effects/GrTextureStripAtlas.h
+++ b/src/gpu/effects/GrTextureStripAtlas.h
@@ -13,6 +13,7 @@
 #include "GrTHashCache.h"
 #include "SkGr.h"
 #include "SkTDArray.h"
+#include "GrBinHashKey.h"
 
 /**
  * Maintains a single large texture whose rows store many textures of a small fixed height,
@@ -129,6 +130,28 @@ private:
     void validate();
 #endif
 
+    /**
+     * Clean up callback registered with GrContext. Allows this class to
+     * free up any allocated AtlasEntry and GrTextureStripAtlas objects
+     */
+    static void CleanUp(const GrContext* context, void* info);
+
+    // Hash table entry for atlases
+    class AtlasEntry;
+    typedef GrTBinHashKey<AtlasEntry, sizeof(GrTextureStripAtlas::Desc)> AtlasHashKey;
+    class AtlasEntry : public ::GrNoncopyable {
+    public:
+        AtlasEntry() : fAtlas(NULL) {}
+        ~AtlasEntry() { SkDELETE(fAtlas); }
+        int compare(const AtlasHashKey& key) const { return fKey.compare(key); }
+        AtlasHashKey fKey;
+        GrTextureStripAtlas* fAtlas;
+    };
+
+    static GrTHashTable<AtlasEntry, AtlasHashKey, 8>* gAtlasCache;
+
+    static GrTHashTable<AtlasEntry, AtlasHashKey, 8>* GetCache();
+
     // We increment gCacheCount for each atlas
     static int32_t gCacheCount;