From 20d4e546d8cb34f8944ef00d249416a047c95524 Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Tue, 24 Jul 2018 11:04:18 -0400 Subject: Reduce arbitrary opList splitting when sorting (take 2) The original CL (https://skia-review.googlesource.com/c/skia/+/141243 (Reduce arbitrary opList splitting when sorting)) was reverted due to Gold image diffs and perf regressions on Android. The image diffs should be fixed by: https://skia-review.googlesource.com/c/skia/+/142163 (Fix explicit allocation bug) Change-Id: I4bcc59820daf440de81f48e8970b47a6c8ec2bbb Reviewed-on: https://skia-review.googlesource.com/142102 Commit-Queue: Robert Phillips Reviewed-by: Greg Daniel --- include/private/GrOpList.h | 7 +++++++ src/gpu/GrDrawingManager.cpp | 42 ++++++++++++++++++++++++++++++++---------- src/gpu/GrOpList.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ src/gpu/GrRenderTargetOpList.h | 3 +++ 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/include/private/GrOpList.h b/include/private/GrOpList.h index 9241cd78f5..a4e541c81d 100644 --- a/include/private/GrOpList.h +++ b/include/private/GrOpList.h @@ -113,6 +113,11 @@ 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,6 +178,8 @@ private: // '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 8b0fb22f6b..366f285cf3 100644 --- a/src/gpu/GrDrawingManager.cpp +++ b/src/gpu/GrDrawingManager.cpp @@ -427,11 +427,22 @@ sk_sp GrDrawingManager::newRTOpList(GrRenderTargetProxy* r SkASSERT(fContext); 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()); + 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(); @@ -454,11 +465,22 @@ sk_sp GrDrawingManager::newTextureOpList(GrTextureProxy* textur SkASSERT(fContext); 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()); + 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 opList(new GrTextureOpList(fContext->contextPriv().resourceProvider(), diff --git a/src/gpu/GrOpList.cpp b/src/gpu/GrOpList.cpp index 313c38f14c..12d15afe6a 100644 --- a/src/gpu/GrOpList.cpp +++ b/src/gpu/GrOpList.cpp @@ -96,6 +96,9 @@ 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 @@ -134,6 +137,38 @@ bool GrOpList::dependsOn(const GrOpList* dependedOn) const { 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(); } @@ -158,6 +193,12 @@ void GrOpList::dump(bool printDependencies) const { 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/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h index 8629a7cb1f..81bd6f16e4 100644 --- a/src/gpu/GrRenderTargetOpList.h +++ b/src/gpu/GrRenderTargetOpList.h @@ -87,6 +87,9 @@ public: op->visitProxies(addDependency); clip.visitProxies(addDependency); + if (dstProxy.proxy()) { + addDependency(dstProxy.proxy()); + } return this->recordOp(std::move(op), caps, clip.doesClip() ? &clip : nullptr, &dstProxy); } -- cgit v1.2.3