Reland "Yank out old reduceOpsTaskSplitting code"

This reverts commit 7689319271.

Reason for revert: Sticking with this despite regression

Original change's description:
> Revert "Yank out old reduceOpsTaskSplitting code"
>
> This reverts commit dfc880bd9b.
>
> Reason for revert: chromium:1146701
>
> Original change's description:
> > Yank out old reduceOpsTaskSplitting code
> >
> > The behavior previously triggered by this flag is reimplemented
> > in review.skia.org/345168 . The current implementation isn't used
> > and can't really be used safely because it may go over the
> > GPU memory budget.
> >
> > Bug: skia:10877
> > Change-Id: I2759c688aa60a618ef76dfec0a49ef5e5f0a9afc
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345477
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
> > Auto-Submit: Adlai Holler <adlai@google.com>
>
> TBR=bsalomon@google.com,robertphillips@google.com,adlai@google.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: skia:10877 chromium:1146701
> Change-Id: I90cdd16eaf033b816f2d1830fd0ee72fdc0ec74b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346777
> Reviewed-by: Adlai Holler <adlai@google.com>
> Commit-Queue: Adlai Holler <adlai@google.com>

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

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: skia:10877 chromium:1146701
Change-Id: Idb734bebe4d0baac9d47539791b0e6240d6be2da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348883
Reviewed-by: Adlai Holler <adlai@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
This commit is contained in:
Adlai Holler 2020-12-30 16:21:13 +00:00 committed by Skia Commit-Bot
parent 245c59a3ff
commit 4ef108e6d1
3 changed files with 53 additions and 101 deletions

View File

@ -174,8 +174,9 @@ struct SK_API GrContextOptions {
Enable fUseDrawInsteadOfClear = Enable::kDefault;
/**
* Allow Ganesh to more aggressively reorder operations.
* Experimental: Allow Ganesh to more aggressively reorder operations.
* Eventually this will just be what is done and will not be optional.
* Note: This option currently has no effect while we update its implementation.
*/
Enable fReduceOpsTaskSplitting = Enable::kDefault;

View File

@ -629,44 +629,31 @@ void GrDrawingManager::createDDLTask(sk_sp<const SkDeferredDisplayList> ddl,
#ifdef SK_DEBUG
void GrDrawingManager::validate() const {
if (fReduceOpsTaskSplitting) {
SkASSERT(!fActiveOpsTask);
} else {
if (fActiveOpsTask) {
SkASSERT(!fDAG.empty());
SkASSERT(!fActiveOpsTask->isClosed());
SkASSERT(fActiveOpsTask == fDAG.back().get());
}
if (fActiveOpsTask) {
SkASSERT(!fDAG.empty());
SkASSERT(!fActiveOpsTask->isClosed());
SkASSERT(fActiveOpsTask == fDAG.back().get());
}
for (int i = 0; i < fDAG.count(); ++i) {
if (fActiveOpsTask != fDAG[i].get()) {
// The resolveTask associated with the activeTask remains open for as long as the
// activeTask does.
bool isActiveResolveTask =
fActiveOpsTask && fActiveOpsTask->fTextureResolveTask == fDAG[i].get();
SkASSERT(isActiveResolveTask || fDAG[i]->isClosed());
}
for (int i = 0; i < fDAG.count(); ++i) {
if (fActiveOpsTask != fDAG[i].get()) {
// The resolveTask associated with the activeTask remains open for as long as the
// activeTask does.
bool isActiveResolveTask =
fActiveOpsTask && fActiveOpsTask->fTextureResolveTask == fDAG[i].get();
SkASSERT(isActiveResolveTask || fDAG[i]->isClosed());
}
}
if (!fDAG.empty() && !fDAG.back()->isClosed()) {
SkASSERT(fActiveOpsTask == fDAG.back().get());
}
if (!fDAG.empty() && !fDAG.back()->isClosed()) {
SkASSERT(fActiveOpsTask == fDAG.back().get());
}
}
#endif
void GrDrawingManager::closeRenderTasksForNewRenderTask(GrSurfaceProxy* target) {
if (target && fReduceOpsTaskSplitting) {
// In this case we need to close all the renderTasks that rely on the current contents of
// 'target'. 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 (GrRenderTask* lastRenderTask = this->getLastRenderTask(target)) {
lastRenderTask->closeThoseWhoDependOnMe(*fContext->priv().caps());
}
} else if (fActiveOpsTask) {
// This is a temporary fix for the partial-MDB world. In that world we're not
void GrDrawingManager::closeActiveOpsTask() {
if (fActiveOpsTask) {
// This is a temporary fix for the partial-MDB world. In that world we're not
// reordering so ops that (in the single opsTask world) would've just glommed onto the
// end of the single opsTask but referred to a far earlier RT need to appear in their
// own opsTask.
@ -680,22 +667,19 @@ sk_sp<GrOpsTask> GrDrawingManager::newOpsTask(GrSurfaceProxyView surfaceView,
SkDEBUGCODE(this->validate());
SkASSERT(fContext);
GrSurfaceProxy* proxy = surfaceView.proxy();
this->closeRenderTasksForNewRenderTask(proxy);
this->closeActiveOpsTask();
sk_sp<GrOpsTask> opsTask(new GrOpsTask(this, fContext->priv().arenas(),
std::move(surfaceView),
fContext->priv().auditTrail()));
SkASSERT(this->getLastRenderTask(proxy) == opsTask.get());
SkASSERT(this->getLastRenderTask(opsTask->target(0).proxy()) == opsTask.get());
if (flushTimeOpsTask) {
fOnFlushRenderTasks.push_back(opsTask);
} else {
this->appendTask(opsTask);
if (!fReduceOpsTaskSplitting) {
fActiveOpsTask = opsTask.get();
}
fActiveOpsTask = opsTask.get();
}
SkDEBUGCODE(this->validate());
@ -727,65 +711,36 @@ void GrDrawingManager::newWaitRenderTask(sk_sp<GrSurfaceProxy> proxy,
sk_sp<GrWaitRenderTask> waitTask = sk_make_sp<GrWaitRenderTask>(GrSurfaceProxyView(proxy),
std::move(semaphores),
numSemaphores);
if (fReduceOpsTaskSplitting) {
GrRenderTask* lastTask = this->getLastRenderTask(proxy.get());
if (lastTask && !lastTask->isClosed()) {
// We directly make the currently open renderTask depend on waitTask instead of using
// the proxy version of addDependency. The waitTask will never need to trigger any
// resolves or mip map generation which is the main advantage of going through the proxy
// version. Additionally we would've had to temporarily set the wait task as the
// lastRenderTask on the proxy, add the dependency, and then reset the lastRenderTask to
// lastTask. Additionally we add all dependencies of lastTask to waitTask so that the
// waitTask doesn't get reordered before them and unnecessarily block those tasks.
// Note: Any previous Ops already in lastTask will get blocked by the wait semaphore
// even though they don't need to be for correctness.
// Make sure we add the dependencies of lastTask to waitTask first or else we'll get a
// circular self dependency of waitTask on waitTask.
waitTask->addDependenciesFromOtherTask(lastTask);
lastTask->addDependency(waitTask.get());
} else {
// If there is a last task we set the waitTask to depend on it so that it doesn't get
// reordered in front of the lastTask causing the lastTask to be blocked by the
// semaphore. Again we directly just go through adding the dependency to the task and
// not the proxy since we don't need to worry about resolving anything.
if (lastTask) {
waitTask->addDependency(lastTask);
}
this->setLastRenderTask(proxy.get(), waitTask.get());
}
this->appendTask(waitTask);
if (fActiveOpsTask && (fActiveOpsTask->target(0).proxy() == proxy.get())) {
SkASSERT(this->getLastRenderTask(proxy.get()) == fActiveOpsTask);
this->insertTaskBeforeLast(waitTask);
// In this case we keep the current renderTask open but just insert the new waitTask
// before it in the list. The waitTask will never need to trigger any resolves or mip
// map generation which is the main advantage of going through the proxy version.
// Additionally we would've had to temporarily set the wait task as the lastRenderTask
// on the proxy, add the dependency, and then reset the lastRenderTask to
// fActiveOpsTask. Additionally we make the waitTask depend on all of fActiveOpsTask
// dependencies so that we don't unnecessarily reorder the waitTask before them.
// Note: Any previous Ops already in fActiveOpsTask will get blocked by the wait
// semaphore even though they don't need to be for correctness.
// Make sure we add the dependencies of fActiveOpsTask to waitTask first or else we'll
// get a circular self dependency of waitTask on waitTask.
waitTask->addDependenciesFromOtherTask(fActiveOpsTask);
fActiveOpsTask->addDependency(waitTask.get());
} else {
if (fActiveOpsTask && (fActiveOpsTask->target(0).proxy() == proxy.get())) {
SkASSERT(this->getLastRenderTask(proxy.get()) == fActiveOpsTask);
this->insertTaskBeforeLast(waitTask);
// In this case we keep the current renderTask open but just insert the new waitTask
// before it in the list. The waitTask will never need to trigger any resolves or mip
// map generation which is the main advantage of going through the proxy version.
// Additionally we would've had to temporarily set the wait task as the lastRenderTask
// on the proxy, add the dependency, and then reset the lastRenderTask to
// fActiveOpsTask. Additionally we make the waitTask depend on all of fActiveOpsTask
// dependencies so that we don't unnecessarily reorder the waitTask before them.
// Note: Any previous Ops already in fActiveOpsTask will get blocked by the wait
// semaphore even though they don't need to be for correctness.
// Make sure we add the dependencies of fActiveOpsTask to waitTask first or else we'll
// get a circular self dependency of waitTask on waitTask.
waitTask->addDependenciesFromOtherTask(fActiveOpsTask);
fActiveOpsTask->addDependency(waitTask.get());
} else {
// In this case we just close the previous RenderTask and start and append the waitTask
// to the DAG. Since it is the last task now we call setLastRenderTask on the proxy. If
// there is a lastTask on the proxy we make waitTask depend on that task. This
// dependency isn't strictly needed but it does keep the DAG from reordering the
// waitTask earlier and blocking more tasks.
if (GrRenderTask* lastTask = this->getLastRenderTask(proxy.get())) {
waitTask->addDependency(lastTask);
}
this->setLastRenderTask(proxy.get(), waitTask.get());
this->closeRenderTasksForNewRenderTask(proxy.get());
this->appendTask(waitTask);
// In this case we just close the previous RenderTask and start and append the waitTask
// to the DAG. Since it is the last task now we call setLastRenderTask on the proxy. If
// there is a lastTask on the proxy we make waitTask depend on that task. This
// dependency isn't strictly needed but it does keep the DAG from reordering the
// waitTask earlier and blocking more tasks.
if (GrRenderTask* lastTask = this->getLastRenderTask(proxy.get())) {
waitTask->addDependency(lastTask);
}
this->setLastRenderTask(proxy.get(), waitTask.get());
this->closeActiveOpsTask();
this->appendTask(waitTask);
}
waitTask->makeClosed(caps);
@ -800,8 +755,7 @@ void GrDrawingManager::newTransferFromRenderTask(sk_sp<GrSurfaceProxy> srcProxy,
size_t dstOffset) {
SkDEBUGCODE(this->validate());
SkASSERT(fContext);
// This copies from srcProxy to dstBuffer so it doesn't have a real target.
this->closeRenderTasksForNewRenderTask(nullptr);
this->closeActiveOpsTask();
GrRenderTask* task = this->appendTask(sk_make_sp<GrTransferFromRenderTask>(
srcProxy, srcRect, surfaceColorType, dstColorType,
@ -828,7 +782,7 @@ bool GrDrawingManager::newCopyRenderTask(GrSurfaceProxyView srcView,
SkDEBUGCODE(this->validate());
SkASSERT(fContext);
this->closeRenderTasksForNewRenderTask(dstView.proxy());
this->closeActiveOpsTask();
const GrCaps& caps = *fContext->priv().caps();
GrSurfaceProxy* srcProxy = srcView.proxy();

View File

@ -126,10 +126,7 @@ private:
bool wasAbandoned() const;
// Closes the target's dependent render tasks (or, if not in sorting/opsTask-splitting-reduction
// mode, closes fActiveOpsTask) in preparation for us opening a new opsTask that will write to
// 'target'.
void closeRenderTasksForNewRenderTask(GrSurfaceProxy* target);
void closeActiveOpsTask();
// return true if any GrRenderTasks were actually executed; false otherwise
bool executeRenderTasks(int startIndex, int stopIndex, GrOpFlushState*,