commit 5c481666c9678f43e039ad605457be3854cf8de3
author robertphillips <robertphillips@google.com> Thu Oct 09 12:30:10 2014 -0700
committer Commit bot <commit-bot@chromium.org> Thu Oct 09 12:30:10 2014 -0700
tree 9336f9c76012a80594353871865e38e5f4367048
parent 27415b71bd529456165945e19b5b7efbebf6fb51

Fix bug in plot locking system

In the new MultiPictureDraw tests a single hoisted layer is reused multiple times. The previous plot locking scheme allowed GrCachedLayer objects to be aggressively deleted prematurely leaving the reusing GrHoistedLayer objects with dangling pointers.

This CL changes the plot locking system to add a pseudo-ref for each GrHoistedLayer.

NOTRY=true

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

src/gpu/GrLayerCache.cpp

diff --git a/src/gpu/GrLayerCache.cpp b/src/gpu/GrLayerCache.cpp
index 0481d14..50f7abe 100644
--- a/src/gpu/GrLayerCache.cpp
+++ b/src/gpu/GrLayerCache.cpp
@@ -153,14 +153,12 @@
 
     if (layer->locked()) {
         // This layer is already locked
-#ifdef SK_DEBUG
         if (layer->isAtlased()) {
-            // It claims to be atlased
+            this->incPlotLock(layer->plot()->id());
             SkASSERT(!dontAtlas);
             SkASSERT(layer->rect().width() == desc.fWidth);
             SkASSERT(layer->rect().height() == desc.fHeight);
         }
-#endif
         return false;
     }
 
@@ -168,7 +166,7 @@
         // Hooray it is still in the atlas - make sure it stays there
         SkASSERT(!dontAtlas);
         layer->setLocked(true);
-        fPlotLocks[layer->plot()->id()]++;
+        this->incPlotLock(layer->plot()->id());
         return false;
     } else if (!dontAtlas && PlausiblyAtlasable(desc.fWidth, desc.fHeight)) {
         // Not in the atlas - will it fit?
@@ -193,7 +191,7 @@
                 layer->setTexture(fAtlas->getTexture(), bounds);
                 layer->setPlot(plot);
                 layer->setLocked(true);
-                fPlotLocks[layer->plot()->id()]++;
+                this->incPlotLock(layer->plot()->id());
                 return true;
             }
 
@@ -219,7 +217,7 @@
 void GrLayerCache::unlock(GrCachedLayer* layer) {
     SkDEBUGCODE(GrAutoValidateLayer avl(fAtlas->getTexture(), layer);)
 
-    if (NULL == layer || !layer->locked()) {
+    if (NULL == layer) {
         // invalid or not locked
         return;
     }
@@ -227,8 +225,7 @@
     if (layer->isAtlased()) {
         const int plotID = layer->plot()->id();
 
-        SkASSERT(fPlotLocks[plotID] > 0);
-        fPlotLocks[plotID]--;
+        this->decPlotLock(plotID);
         // At this point we could aggressively clear out un-locked plots but
         // by delaying we may be able to reuse some of the atlased layers later.
 #if DISABLE_CACHING
@@ -253,9 +250,6 @@
 
 #ifdef SK_DEBUG
 void GrLayerCache::validate() const {
-    int plotLocks[kNumPlotsX * kNumPlotsY];
-    memset(plotLocks, 0, sizeof(plotLocks));
-
     SkTDynamicHash<GrCachedLayer, GrCachedLayer::Key>::ConstIter iter(&fLayerHash);
     for (; !iter.done(); ++iter) {
         const GrCachedLayer* layer = &(*iter);
@@ -270,7 +264,7 @@
             SkASSERT(!pictInfo->fPlotUsage.isEmpty());
 #endif
         } else {
-            // If there is no picture info for this layer then all of its
+            // If there is no picture info for this picture then all of its
             // layers should be non-atlased.
             SkASSERT(!layer->isAtlased());
         }
@@ -282,14 +276,10 @@
             SkASSERT(pictInfo->fPlotUsage.contains(layer->plot()));
 
             if (layer->locked()) {
-                plotLocks[layer->plot()->id()]++;
+                SkASSERT(fPlotLocks[layer->plot()->id()] > 0);
             }
         } 
     }
-
-    for (int i = 0; i < kNumPlotsX*kNumPlotsY; ++i) {
-        SkASSERT(plotLocks[i] == fPlotLocks[i]);
-    }
 }
 
 class GrAutoValidateCache : ::SkNoncopyable {