diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp index 48540e00df..f1e116f473 100644 --- a/src/gpu/GrDrawingManager.cpp +++ b/src/gpu/GrDrawingManager.cpp @@ -36,7 +36,11 @@ void GrDrawingManager::cleanup() { // We shouldn't need to do this, but it turns out some clients still hold onto opLists // after a cleanup. // MDB TODO: is this still true? - fOpLists[i]->reset(); + if (!fOpLists[i]->unique()) { + // TODO: Eventually this should be guaranteed unique. + // https://bugs.chromium.org/p/skia/issues/detail?id=7111 + fOpLists[i]->endFlush(); + } } fOpLists.reset(); @@ -78,13 +82,6 @@ void GrDrawingManager::freeGpuResources() { } -void GrDrawingManager::reset() { - for (int i = 0; i < fOpLists.count(); ++i) { - fOpLists[i]->reset(); - } - fFlushState.reset(); -} - gr_instanced::OpAllocator* GrDrawingManager::instancingAllocator() { if (fInstancingAllocator) { return fInstancingAllocator.get(); @@ -162,6 +159,7 @@ GrSemaphoresSubmitted GrDrawingManager::internalFlush(GrSurfaceProxy*, if (!opList) { continue; // Odd - but not a big deal } + SkASSERT(opList->unique()); opList->makeClosed(*fContext->caps()); opList->prepare(&fFlushState); if (!opList->execute(&fFlushState)) { @@ -224,7 +222,11 @@ GrSemaphoresSubmitted GrDrawingManager::internalFlush(GrSurfaceProxy*, if (!fOpLists[i]) { continue; } - fOpLists[i]->reset(); + if (!fOpLists[i]->unique()) { + // TODO: Eventually this should be guaranteed unique. + // https://bugs.chromium.org/p/skia/issues/detail?id=7111 + fOpLists[i]->endFlush(); + } } fOpLists.reset(); diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h index 0143783708..38d234900a 100644 --- a/src/gpu/GrDrawingManager.h +++ b/src/gpu/GrDrawingManager.h @@ -94,7 +94,6 @@ private: void abandon(); void cleanup(); - void reset(); GrSemaphoresSubmitted flush(GrSurfaceProxy* proxy, int numSemaphores = 0, GrBackendSemaphore backendSemaphores[] = nullptr) { diff --git a/src/gpu/GrOpList.cpp b/src/gpu/GrOpList.cpp index 59e1cd0f23..902ab89b7c 100644 --- a/src/gpu/GrOpList.cpp +++ b/src/gpu/GrOpList.cpp @@ -45,14 +45,17 @@ GrOpList::GrOpList(GrResourceProvider* resourceProvider, } GrOpList::~GrOpList() { - this->reset(); + if (fTarget.get() && this == fTarget.get()->getLastOpList()) { + // Ensure the target proxy doesn't keep hold of a dangling back pointer. + fTarget.get()->setLastOpList(nullptr); + } } bool GrOpList::instantiate(GrResourceProvider* resourceProvider) { return SkToBool(fTarget.get()->instantiate(resourceProvider)); } -void GrOpList::reset() { +void GrOpList::endFlush() { if (fTarget.get() && this == fTarget.get()->getLastOpList()) { fTarget.get()->setLastOpList(nullptr); } diff --git a/src/gpu/GrOpList.h b/src/gpu/GrOpList.h index 1f98ddc41b..cd0b50035e 100644 --- a/src/gpu/GrOpList.h +++ b/src/gpu/GrOpList.h @@ -58,7 +58,10 @@ public: } } - virtual void reset(); + // Called when this class will survive a flush and needs to truncate its ops and start over. + // TODO: ultimately it should be invalid for an op list to survive a flush. + // https://bugs.chromium.org/p/skia/issues/detail?id=7111 + virtual void endFlush(); // TODO: in an MDB world, where the OpLists don't allocate GPU resources, it seems like // these could go away diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp index bc293bb1e3..33240ea3f1 100644 --- a/src/gpu/GrRenderTargetOpList.cpp +++ b/src/gpu/GrRenderTargetOpList.cpp @@ -180,7 +180,7 @@ bool GrRenderTargetOpList::onExecute(GrOpFlushState* flushState) { return true; } -void GrRenderTargetOpList::reset() { +void GrRenderTargetOpList::endFlush() { fLastClipStackGenID = SK_InvalidUniqueID; fRecordedOps.reset(); if (fInstancedRendering) { @@ -188,7 +188,7 @@ void GrRenderTargetOpList::reset() { fInstancedRendering = nullptr; } - INHERITED::reset(); + INHERITED::endFlush(); } void GrRenderTargetOpList::abandonGpuResources() { diff --git a/src/gpu/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h index cf513b7e4d..24125acba6 100644 --- a/src/gpu/GrRenderTargetOpList.h +++ b/src/gpu/GrRenderTargetOpList.h @@ -55,7 +55,7 @@ public: /** * Empties the draw buffer of any queued up draws. */ - void reset() override; + void endFlush() override; void abandonGpuResources() override; void freeGpuResources() override; diff --git a/src/gpu/GrTextureOpList.cpp b/src/gpu/GrTextureOpList.cpp index 97371d598f..852258fa71 100644 --- a/src/gpu/GrTextureOpList.cpp +++ b/src/gpu/GrTextureOpList.cpp @@ -79,9 +79,9 @@ bool GrTextureOpList::onExecute(GrOpFlushState* flushState) { return true; } -void GrTextureOpList::reset() { +void GrTextureOpList::endFlush() { fRecordedOps.reset(); - INHERITED::reset(); + INHERITED::endFlush(); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/GrTextureOpList.h b/src/gpu/GrTextureOpList.h index 0d00c9479a..1b37ff27da 100644 --- a/src/gpu/GrTextureOpList.h +++ b/src/gpu/GrTextureOpList.h @@ -29,7 +29,7 @@ public: /** * Empties the draw buffer of any queued ops. */ - void reset() override; + void endFlush() override; void abandonGpuResources() override {} void freeGpuResources() override {} diff --git a/src/gpu/instanced/InstancedRendering.cpp b/src/gpu/instanced/InstancedRendering.cpp index cb4a1de63d..52daaec248 100644 --- a/src/gpu/instanced/InstancedRendering.cpp +++ b/src/gpu/instanced/InstancedRendering.cpp @@ -25,7 +25,8 @@ InstancedRendering::InstancedRendering(GrGpu* gpu) } InstancedRendering::~InstancedRendering() { - SkASSERT(State::kRecordingDraws == fState); + // Make sure there isn't anything with a dangling pointer to this instance. + SkASSERT(fTrackedOps.isEmpty()); } void InstancedRendering::beginFlush(GrResourceProvider* rp) {