Revert "Reduce arbitrary opList splitting when sorting"

This reverts commit 94fee93c9b.

Reason for revert: Android (and Chromecast) woes

Original change's description:
> Reduce arbitrary opList splitting when sorting
> 
> Change-Id: I49a47672600f72dc46f27462a2c344e77a06a659
> Reviewed-on: https://skia-review.googlesource.com/141243
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>

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

Change-Id: Ic4fd4ab17bb15bef35dcbf852e0f8ad99ee45e8f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/141760
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

include/private/GrOpList.h

@@ -67,7 +67,15 @@ public:
     /*
      * Does this opList depend on 'dependedOn'?
      */
-    bool dependsOn(const GrOpList* dependedOn) const;
+    bool dependsOn(GrOpList* dependedOn) const {
+        for (int i = 0; i < fDependencies.count(); ++i) {
+            if (fDependencies[i] == dependedOn) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 
     /*
      * Safely cast this GrOpList to a GrTextureOpList (if possible).
@@ -112,13 +120,6 @@ protected:
 private:
     friend class GrDrawingManager; // for resetFlag, TopoSortTraits & gatherProxyIntervals
 
-    void addDependency(GrOpList* dependedOn);
-    void addDependent(GrOpList* dependent);
-    SkDEBUGCODE(bool isDependedent(const GrOpList* dependent) const);
-    SkDEBUGCODE(void validate() const);
-
-    void closeThoseWhoDependOnMe(const GrCaps&);
-
     // Remove all Ops which reference proxies that have not been instantiated.
     virtual void purgeOpsWithUninstantiatedProxies() = 0;
 
@@ -173,13 +174,13 @@ private:
     virtual void onPrepare(GrOpFlushState* flushState) = 0;
     virtual bool onExecute(GrOpFlushState* flushState) = 0;
 
+    void addDependency(GrOpList* dependedOn);
+
     uint32_t               fUniqueID;
     uint32_t               fFlags;
 
     // 'this' GrOpList relies on the output of the GrOpLists in 'fDependencies'
     SkSTArray<1, GrOpList*, true> fDependencies;
-    // 'this' GrOpList's output is relied on by the GrOpLists in 'fDependents'
-    SkSTArray<1, GrOpList*, true> fDependents;
 
     typedef SkRefCnt INHERITED;
 };

src/gpu/GrDrawingManager.cpp

@@ -32,6 +32,11 @@
 #include "ccpr/GrCoverageCountingPathRenderer.h"
 #include "text/GrTextContext.h"
 
+// Turn on/off the sorting of opLists at flush time
+#ifndef SK_DISABLE_RENDER_TARGET_SORTING
+   #define SK_DISABLE_RENDER_TARGET_SORTING
+#endif
+
 GrDrawingManager::GrDrawingManager(GrContext* context,
                                    const GrPathRendererChain::Options& optionsForPathRendererChain,
                                    const GrTextContext::Options& optionsForTextContext,
@@ -127,11 +132,6 @@ GrSemaphoresSubmitted GrDrawingManager::internalFlush(GrSurfaceProxy*,
         fOpLists[i]->makeClosed(*fContext->contextPriv().caps());
     }
 
-    if (fSortRenderTargets) {
-        SkDEBUGCODE(bool result =) SkTTopoSort<GrOpList, GrOpList::TopoSortTraits>(&fOpLists);
-        SkASSERT(result);
-    }
-
 #ifdef SK_DEBUG
     // This block checks for any unnecessary splits in the opLists. If two sequential opLists
     // share the same backing GrSurfaceProxy it means the opList was artificially split.
@@ -149,6 +149,11 @@ GrSemaphoresSubmitted GrDrawingManager::internalFlush(GrSurfaceProxy*,
     }
 #endif
 
+    if (fSortRenderTargets) {
+        SkDEBUGCODE(bool result =) SkTTopoSort<GrOpList, GrOpList::TopoSortTraits>(&fOpLists);
+        SkASSERT(result);
+    }
+
     GrOpFlushState flushState(gpu, fContext->contextPriv().resourceProvider(),
                               &fTokenTracker);
 
@@ -273,7 +278,7 @@ bool GrDrawingManager::executeOpLists(int startIndex, int stopIndex, GrOpFlushSt
     SkDebugf("Flushing opLists: %d to %d out of [%d, %d]\n",
                             startIndex, stopIndex, 0, fOpLists.count());
     for (int i = startIndex; i < stopIndex; ++i) {
-        fOpLists[i]->dump(true);
+        fOpLists[i]->dump(false);
     }
 #endif
 
@@ -426,24 +431,12 @@ sk_sp<GrRenderTargetOpList> GrDrawingManager::newRTOpList(GrRenderTargetProxy* r
                                                           bool managedOpList) {
     SkASSERT(fContext);
 
+    // This is  a temporary fix for the partial-MDB world. In that world we're not reordering
+    // so ops that (in the single opList world) would've just glommed onto the end of the single
+    // opList but referred to a far earlier RT need to appear in their own opList.
     if (!fOpLists.empty()) {
-        if (fSortRenderTargets) {
-            // In this case we need to close all the opLists that rely on the current contents of
-            // 'rtp'. That is bc we're going to update the content of the proxy so they need to be
-            // split in case they use both the old and new content. (This is a bit of an overkill:
-            // they really only need to be split if they ever reference proxy's contents again but
-            // that is hard to predict/handle).
-            if (GrOpList* lastOpList = rtp->getLastOpList()) {
-                lastOpList->closeThoseWhoDependOnMe(*fContext->contextPriv().caps());
-            }
-        } else {
-            // This is  a temporary fix for the partial-MDB world. In that world we're not
-            // reordering so ops that (in the single opList world) would've just glommed onto the
-            // end of the single opList but referred to a far earlier RT need to appear in their
-            // own opList.
         fOpLists.back()->makeClosed(*fContext->contextPriv().caps());
     }
-    }
 
     auto resourceProvider = fContext->contextPriv().resourceProvider();
 
@@ -464,24 +457,12 @@ sk_sp<GrRenderTargetOpList> GrDrawingManager::newRTOpList(GrRenderTargetProxy* r
 sk_sp<GrTextureOpList> GrDrawingManager::newTextureOpList(GrTextureProxy* textureProxy) {
     SkASSERT(fContext);
 
+    // This is  a temporary fix for the partial-MDB world. In that world we're not reordering
+    // so ops that (in the single opList world) would've just glommed onto the end of the single
+    // opList but referred to a far earlier RT need to appear in their own opList.
     if (!fOpLists.empty()) {
-        if (fSortRenderTargets) {
-            // In this case we need to close all the opLists that rely on the current contents of
-            // 'rtp'. That is bc we're going to update the content of the proxy so they need to be
-            // split in case they use both the old and new content. (This is a bit of an overkill:
-            // they really only need to be split if they ever reference proxy's contents again but
-            // that is hard to predict/handle).
-            if (GrOpList* lastOpList = textureProxy->getLastOpList()) {
-                lastOpList->closeThoseWhoDependOnMe(*fContext->contextPriv().caps());
-            }
-        } else {
-            // This is  a temporary fix for the partial-MDB world. In that world we're not
-            // reordering so ops that (in the single opList world) would've just glommed onto the
-            // end of the single opList but referred to a far earlier RT need to appear in their
-            // own opList.
         fOpLists.back()->makeClosed(*fContext->contextPriv().caps());
     }
-    }
 
     sk_sp<GrTextureOpList> opList(new GrTextureOpList(fContext->contextPriv().resourceProvider(),
                                                       fContext->contextPriv().refOpMemoryPool(),

src/gpu/GrOpList.cpp

@@ -96,9 +96,6 @@ void GrOpList::addDependency(GrOpList* dependedOn) {
     }
 
     fDependencies.push_back(dependedOn);
-    dependedOn->addDependent(this);
-
-    SkDEBUGCODE(this->validate());
 }
 
 // Convert from a GrSurface-based dependency to a GrOpList one
@@ -109,13 +106,11 @@ void GrOpList::addDependency(GrSurfaceProxy* dependedOn, const GrCaps& caps) {
 
         GrOpList* opList = dependedOn->getLastOpList();
         if (opList == this) {
-            // self-read - presumably for dst reads. We can't make it closed in the self-read case.
+            // self-read - presumably for dst reads
         } else {
             this->addDependency(opList);
 
-            // We are closing 'opList' here bc the current contents of it are what 'this' opList
+            // Can't make it closed in the self-read case
-            // depends on. We need a break in 'opList' so that the usage of that state has a
-            // chance to execute.
             opList->makeClosed(caps);
         }
     }
@@ -127,78 +122,22 @@ void GrOpList::addDependency(GrSurfaceProxy* dependedOn, const GrCaps& caps) {
     }
 }
 
-bool GrOpList::dependsOn(const GrOpList* dependedOn) const {
-    for (int i = 0; i < fDependencies.count(); ++i) {
-        if (fDependencies[i] == dependedOn) {
-            return true;
-        }
-    }
-
-    return false;
-}
-
-void GrOpList::addDependent(GrOpList* dependent) {
-    fDependents.push_back(dependent);
-}
-
-#ifdef SK_DEBUG
-bool GrOpList::isDependedent(const GrOpList* dependent) const {
-    for (int i = 0; i < fDependents.count(); ++i) {
-        if (fDependents[i] == dependent) {
-            return true;
-        }
-    }
-
-    return false;
-}
-
-void GrOpList::validate() const {
-    // TODO: check for loops and duplicates
-
-    for (int i = 0; i < fDependencies.count(); ++i) {
-        SkASSERT(fDependencies[i]->isDependedent(this));
-    }
-}
-#endif
-
-void GrOpList::closeThoseWhoDependOnMe(const GrCaps& caps) {
-    for (int i = 0; i < fDependents.count(); ++i) {
-        if (!fDependents[i]->isClosed()) {
-            fDependents[i]->makeClosed(caps);
-        }
-    }
-}
-
 bool GrOpList::isInstantiated() const {
     return fTarget.get()->priv().isInstantiated();
 }
 
 #ifdef SK_DEBUG
-static const char* op_to_name(GrLoadOp op) {
-    return GrLoadOp::kLoad == op ? "load" : GrLoadOp::kClear == op ? "clear" : "discard";
-}
-
 void GrOpList::dump(bool printDependencies) const {
     SkDebugf("--------------------------------------------------------------\n");
-    SkDebugf("opListID: %d -> proxyID: %d\n", fUniqueID,
+    SkDebugf("opList: %d -> RT: %d\n", fUniqueID, fTarget.get() ? fTarget.get()->uniqueID().asUInt()
-             fTarget.get() ? fTarget.get()->uniqueID().asUInt() : -1);
+                                                                : -1);
-    SkDebugf("ColorLoadOp: %s %x StencilLoadOp: %s\n",
-             op_to_name(fColorLoadOp),
-             GrLoadOp::kClear == fColorLoadOp ? fLoadClearColor : 0x0,
-             op_to_name(fStencilLoadOp));
 
     if (printDependencies) {
-        SkDebugf("I rely On (%d): ", fDependencies.count());
+        SkDebugf("relies On (%d): ", fDependencies.count());
         for (int i = 0; i < fDependencies.count(); ++i) {
             SkDebugf("%d, ", fDependencies[i]->fUniqueID);
         }
         SkDebugf("\n");
-
-        SkDebugf("(%d) Rely On Me: ", fDependents.count());
-        for (int i = 0; i < fDependents.count(); ++i) {
-            SkDebugf("%d, ", fDependents[i]->fUniqueID);
-        }
-        SkDebugf("\n");
     }
 }
 #endif

src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h

@@ -44,15 +44,6 @@ public:
 
     const char* name() const override { return "GaussianConvolution"; }
 
-    SkString dumpInfo() const override {
-        SkString str;
-        str.appendf("dir: %s radius: %d bounds: [%d %d]",
-                    Direction::kX == fDirection ? "X" : "Y",
-                    fRadius,
-                    fBounds[0], fBounds[1]);
-        return str;
-    }
-
     std::unique_ptr<GrFragmentProcessor> clone() const override {
         return std::unique_ptr<GrFragmentProcessor>(
                 new GrGaussianConvolutionFragmentProcessor(*this));