Pull non-substantive changes out of "Reduce arbitrary opList splitting when sorting"

Change-Id: I044d7cde5ed4f1dc60bb55dfd534680b4cd055bf
Reviewed-on: https://skia-review.googlesource.com/142167
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

3 files changed, 54 insertions, 28 deletions

diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index 0df9b34468..8b0fb22f6b 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -32,11 +32,6 @@
 #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,
@@ -132,6 +127,11 @@ 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,11 +149,6 @@ GrSemaphoresSubmitted GrDrawingManager::internalFlush(GrSurfaceProxy*,
     }
 #endif
 
-    if (fSortRenderTargets) {
-        SkDEBUGCODE(bool result =) SkTTopoSort<GrOpList, GrOpList::TopoSortTraits>(&fOpLists);
-        SkASSERT(result);
-    }
-
     GrOpFlushState flushState(gpu, fContext->contextPriv().resourceProvider(),
                               &fTokenTracker);
 
@@ -278,7 +273,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(false);
+        fOpLists[i]->dump(true);
     }
 #endif
 
@@ -431,10 +426,11 @@ 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()) {
+        // 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());
     }
 
@@ -457,10 +453,11 @@ 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()) {
+        // 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());
     }
 
diff --git a/src/gpu/GrOpList.cpp b/src/gpu/GrOpList.cpp
index b9f425576e..313c38f14c 100644
--- a/src/gpu/GrOpList.cpp
+++ b/src/gpu/GrOpList.cpp
@@ -106,11 +106,13 @@ void GrOpList::addDependency(GrSurfaceProxy* dependedOn, const GrCaps& caps) {
 
         GrOpList* opList = dependedOn->getLastOpList();
         if (opList == this) {
-            // self-read - presumably for dst reads
+            // self-read - presumably for dst reads. We can't make it closed in the self-read case.
         } else {
             this->addDependency(opList);
 
-            // Can't make it closed in the self-read case
+            // We are closing 'opList' here bc the current contents of it are what 'this' opList
+            // depends on. We need a break in 'opList' so that the usage of that state has a
+            // chance to execute.
             opList->makeClosed(caps);
         }
     }
@@ -122,18 +124,36 @@ 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;
+}
+
 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("opList: %d -> RT: %d\n", fUniqueID, fTarget.get() ? fTarget.get()->uniqueID().asUInt()
-                                                                : -1);
+    SkDebugf("opListID: %d -> proxyID: %d\n", fUniqueID,
+             fTarget.get() ? fTarget.get()->uniqueID().asUInt() : -1);
+    SkDebugf("ColorLoadOp: %s %x StencilLoadOp: %s\n",
+             op_to_name(fColorLoadOp),
+             GrLoadOp::kClear == fColorLoadOp ? fLoadClearColor : 0x0,
+             op_to_name(fStencilLoadOp));
 
     if (printDependencies) {
-        SkDebugf("relies On (%d): ", fDependencies.count());
+        SkDebugf("I rely On (%d): ", fDependencies.count());
         for (int i = 0; i < fDependencies.count(); ++i) {
             SkDebugf("%d, ", fDependencies[i]->fUniqueID);
         }
diff --git a/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h b/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h
index 9850e605fb..6d68292616 100644
--- a/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h
+++ b/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h
@@ -44,6 +44,15 @@ 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));
@@ -74,14 +83,14 @@ private:
 
     GR_DECLARE_FRAGMENT_PROCESSOR_TEST
 
-    GrCoordTransform fCoordTransform;
-    TextureSampler fTextureSampler;
+    GrCoordTransform      fCoordTransform;
+    TextureSampler        fTextureSampler;
     // TODO: Inline the kernel constants into the generated shader code. This may involve pulling
     // some of the logic from SkGpuBlurUtils into this class related to radius/sigma calculations.
-    float fKernel[kMaxKernelWidth];
-    int   fBounds[2];
-    int fRadius;
-    Direction fDirection;
+    float                 fKernel[kMaxKernelWidth];
+    int                   fBounds[2];
+    int                   fRadius;
+    Direction             fDirection;
     GrTextureDomain::Mode fMode;
 
     typedef GrFragmentProcessor INHERITED;