diff options
author | robertphillips <robertphillips@google.com> | 2014-10-09 12:30:10 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-10-09 12:30:10 -0700 |
commit | 5c481666c9678f43e039ad605457be3854cf8de3 (patch) | |
tree | 9336f9c76012a80594353871865e38e5f4367048 /src/gpu | |
parent | 27415b71bd529456165945e19b5b7efbebf6fb51 (diff) |
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
Diffstat (limited to 'src/gpu')
-rw-r--r-- | src/gpu/GrLayerCache.cpp | 24 | ||||
-rw-r--r-- | src/gpu/GrLayerCache.h | 13 |
2 files changed, 17 insertions, 20 deletions
diff --git a/src/gpu/GrLayerCache.cpp b/src/gpu/GrLayerCache.cpp index 0481d144e5..50f7abe830 100644 --- a/src/gpu/GrLayerCache.cpp +++ b/src/gpu/GrLayerCache.cpp @@ -153,14 +153,12 @@ bool GrLayerCache::lock(GrCachedLayer* layer, const GrTextureDesc& desc, bool do 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 @@ bool GrLayerCache::lock(GrCachedLayer* layer, const GrTextureDesc& desc, bool do // 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 @@ bool GrLayerCache::lock(GrCachedLayer* layer, const GrTextureDesc& desc, bool do 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 @@ bool GrLayerCache::lock(GrCachedLayer* layer, const GrTextureDesc& desc, bool do 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 @@ void GrLayerCache::unlock(GrCachedLayer* layer) { 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 @@ void GrLayerCache::unlock(GrCachedLayer* layer) { #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 @@ void GrLayerCache::validate() const { 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 @@ void GrLayerCache::validate() const { 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 { diff --git a/src/gpu/GrLayerCache.h b/src/gpu/GrLayerCache.h index 790b82a8f3..7bc98b0216 100644 --- a/src/gpu/GrLayerCache.h +++ b/src/gpu/GrLayerCache.h @@ -231,9 +231,10 @@ private: // This implements a plot-centric locking mechanism (since the atlas // backing texture is always locked). Each layer that is locked (i.e., // needed for the current rendering) in a plot increments the plot lock - // count for that plot. Similarly, once a rendering is complete all the - // layers used in it decrement the lock count for the used plots. - // Plots with a 0 lock count are open for recycling/purging. + // count for that plot for each time it is used. Similarly, once a + // rendering is complete all the layers used in it decrement the lock + // count for the used plots. Plots with a 0 lock count are open for + // recycling/purging. int fPlotLocks[kNumPlotsX * kNumPlotsY]; void initAtlas(); @@ -255,6 +256,12 @@ private: // was purged; false otherwise. bool purgePlot(); + void incPlotLock(int plotIdx) { ++fPlotLocks[plotIdx]; } + void decPlotLock(int plotIdx) { + SkASSERT(fPlotLocks[plotIdx] > 0); + --fPlotLocks[plotIdx]; + } + // for testing friend class TestingAccess; int numLayers() const { return fLayerHash.count(); } |