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>
diff --git a/include/private/GrOpList.h b/include/private/GrOpList.h
index 0d5a1a2..a4e541c 100644
--- a/include/private/GrOpList.h
+++ b/include/private/GrOpList.h
@@ -67,15 +67,7 @@
/*
* Does this opList depend on 'dependedOn'?
*/
- bool dependsOn(GrOpList* dependedOn) const {
- for (int i = 0; i < fDependencies.count(); ++i) {
- if (fDependencies[i] == dependedOn) {
- return true;
- }
- }
-
- return false;
- }
+ bool dependsOn(const GrOpList* dependedOn) const;
/*
* Safely cast this GrOpList to a GrTextureOpList (if possible).
@@ -120,6 +112,13 @@
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;
@@ -174,13 +173,13 @@
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;
};
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index 0df9b34..366f285 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 @@
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 @@
}
#endif
- if (fSortRenderTargets) {
- SkDEBUGCODE(bool result =) SkTTopoSort<GrOpList, GrOpList::TopoSortTraits>(&fOpLists);
- SkASSERT(result);
- }
-
GrOpFlushState flushState(gpu, fContext->contextPriv().resourceProvider(),
&fTokenTracker);
@@ -278,7 +273,7 @@
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,11 +426,23 @@
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()) {
- fOpLists.back()->makeClosed(*fContext->contextPriv().caps());
+ 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();
@@ -457,11 +464,23 @@
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()) {
- fOpLists.back()->makeClosed(*fContext->contextPriv().caps());
+ 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(),
diff --git a/src/gpu/GrOpList.cpp b/src/gpu/GrOpList.cpp
index b9f4255..12d15af 100644
--- a/src/gpu/GrOpList.cpp
+++ b/src/gpu/GrOpList.cpp
@@ -96,6 +96,9 @@
}
fDependencies.push_back(dependedOn);
+ dependedOn->addDependent(this);
+
+ SkDEBUGCODE(this->validate());
}
// Convert from a GrSurface-based dependency to a GrOpList one
@@ -106,11 +109,13 @@
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,22 +127,78 @@
}
}
+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("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);
}
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
diff --git a/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h b/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h
index 9850e60..6d68292 100644
--- a/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h
+++ b/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h
@@ -44,6 +44,15 @@
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 @@
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;