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
This commit is contained in:
robertphillips 2014-10-09 12:30:10 -07:00 committed by Commit bot
parent 27415b71bd
commit 5c481666c9
2 changed files with 17 additions and 20 deletions

View File

@ -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 {

View File

@ -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(); }