commit 65888b81cf35503b148f7866224a74d1fef1508a
author Adlai Holler <adlai@google.com> Mon Jun 22 14:06:55 2020 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Mon Jun 22 18:08:50 2020 +0000
tree d96930e0966e10ed190798f908693138bb3493f1
parent abe2375dfb06afcd1967aa89c8b36405e693a7bf

Revert "Consolidate GrDrawingManager <-> GrRenderTask lifecycle"

This reverts commit 6f1487fe80c19c7a18a41d5a738d7ba2301929a9.

Reason for revert: http://crbug.com/1097620

Original change's description:
> Consolidate GrDrawingManager <-> GrRenderTask lifecycle
>
> This creates a funnel in the drawing manager (removeRenderTasks) that
> opens the door for tighter integration between the two classes. Also we
> add some assertions about the relationship and cut out duplicated code.
>
> Bug: skia:10372
> Change-Id: I0781ba7d45ac090cf7f6d430f0d56afe0f98b7e0
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/297195
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Adlai Holler <adlai@google.com>

TBR=robertphillips@google.com,adlai@google.com

Change-Id: I5d34ada1838d206d8a33294427d459c36ad6b740
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10372
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298137
Auto-Submit: Adlai Holler <adlai@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

src/gpu/GrDrawingManager.cpp

diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index df9bf30..5ae2684 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -58,7 +58,7 @@
     fRenderTasks.reset();
 }
 
-void GrDrawingManager::RenderTaskDAG::rawRemoveRenderTasks(int startIndex, int stopIndex) {
+void GrDrawingManager::RenderTaskDAG::removeRenderTasks(int startIndex, int stopIndex) {
     for (int i = startIndex; i < stopIndex; ++i) {
         fRenderTasks[i] = nullptr;
     }
@@ -133,13 +133,37 @@
 }
 
 void GrDrawingManager::RenderTaskDAG::closeAll(const GrCaps* caps) {
-    for (auto& task : fRenderTasks) {
-        if (task) {
-            task->makeClosed(*caps);
+    for (int i = 0; i < fRenderTasks.count(); ++i) {
+        if (fRenderTasks[i]) {
+            fRenderTasks[i]->makeClosed(*caps);
         }
     }
 }
 
+void GrDrawingManager::RenderTaskDAG::cleanup(GrDrawingManager* drawingMgr, const GrCaps* caps) {
+    for (int i = 0; i < fRenderTasks.count(); ++i) {
+        if (!fRenderTasks[i]) {
+            continue;
+        }
+
+        // no renderTask should receive a dependency
+        fRenderTasks[i]->makeClosed(*caps);
+
+        fRenderTasks[i]->disown(drawingMgr);
+
+        // We shouldn't need to do this, but it turns out some clients still hold onto opsTasks
+        // after a cleanup.
+        // MDB TODO: is this still true?
+        if (!fRenderTasks[i]->unique()) {
+            // TODO: Eventually this should be guaranteed unique.
+            // https://bugs.chromium.org/p/skia/issues/detail?id=7111
+            fRenderTasks[i]->endFlush(drawingMgr);
+        }
+    }
+
+    fRenderTasks.reset();
+}
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 GrDrawingManager::GrDrawingManager(GrRecordingContext* context,
                                    const GrPathRendererChain::Options& optionsForPathRendererChain,
@@ -153,9 +177,17 @@
         , fFlushing(false)
         , fReduceOpsTaskSplitting(reduceOpsTaskSplitting) { }
 
+void GrDrawingManager::cleanup() {
+    fDAG.cleanup(this, fContext->priv().caps());
+
+    fPathRendererChain = nullptr;
+    fSoftwarePathRenderer = nullptr;
+
+    fOnFlushCBObjects.reset();
+}
+
 GrDrawingManager::~GrDrawingManager() {
-    fDAG.closeAll(fContext->priv().caps());
-    this->removeRenderTasks(0, fDAG.numRenderTasks());
+    this->cleanup();
 }
 
 bool GrDrawingManager::wasAbandoned() const {
@@ -328,7 +360,6 @@
                     }
                     renderTask->handleInternalAllocationFailure();
                 }
-                this->removeRenderTasks(startIndex, stopIndex);
             }
 
             if (this->executeRenderTasks(
@@ -340,9 +371,10 @@
 
 #ifdef SK_DEBUG
     for (int i = 0; i < fDAG.numRenderTasks(); ++i) {
-        // All render tasks should have been cleared out by now – we only reset the array below to
-        // reclaim storage.
-        SkASSERT(!fDAG.renderTask(i));
+        // If there are any remaining opsTaskss at this point, make sure they will not survive the
+        // flush. Otherwise we need to call endFlush() on them.
+        // http://skbug.com/7111
+        SkASSERT(!fDAG.renderTask(i) || fDAG.renderTask(i)->unique());
     }
 #endif
     fLastRenderTasks.reset();
@@ -487,7 +519,7 @@
         }
         task->disown(this);
     }
-    fDAG.rawRemoveRenderTasks(startIndex, stopIndex);
+    fDAG.removeRenderTasks(startIndex, stopIndex);
 }
 
 static void resolve_and_mipmap(GrGpu* gpu, GrSurfaceProxy* proxy) {