Revert "Yank out old reduceOpsTaskSplitting code"
This reverts commit dfc880bd9ba05c5097355469c328c6d64208bc0f.
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>
diff --git a/include/gpu/GrContextOptions.h b/include/gpu/GrContextOptions.h
index 25fbf4f..1f1c2c2 100644
--- a/include/gpu/GrContextOptions.h
+++ b/include/gpu/GrContextOptions.h
@@ -174,9 +174,8 @@
Enable fUseDrawInsteadOfClear = Enable::kDefault;
/**
- * Experimental: Allow Ganesh to more aggressively reorder operations.
+ * 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;
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index 4f8180b..4902026 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -629,31 +629,44 @@
#ifdef SK_DEBUG
void GrDrawingManager::validate() const {
- 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());
+ if (fReduceOpsTaskSplitting) {
+ SkASSERT(!fActiveOpsTask);
+ } else {
+ if (fActiveOpsTask) {
+ SkASSERT(!fDAG.empty());
+ SkASSERT(!fActiveOpsTask->isClosed());
+ SkASSERT(fActiveOpsTask == fDAG.back().get());
}
- }
- if (!fDAG.empty() && !fDAG.back()->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());
+ }
+ }
+
+ if (!fDAG.empty() && !fDAG.back()->isClosed()) {
+ SkASSERT(fActiveOpsTask == fDAG.back().get());
+ }
}
}
#endif
-void GrDrawingManager::closeActiveOpsTask() {
- if (fActiveOpsTask) {
- // This is a temporary fix for the partial-MDB world. In that world we're not
+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
// 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.
@@ -667,19 +680,22 @@
SkDEBUGCODE(this->validate());
SkASSERT(fContext);
- this->closeActiveOpsTask();
+ GrSurfaceProxy* proxy = surfaceView.proxy();
+ this->closeRenderTasksForNewRenderTask(proxy);
sk_sp<GrOpsTask> opsTask(new GrOpsTask(this, fContext->priv().arenas(),
std::move(surfaceView),
fContext->priv().auditTrail()));
- SkASSERT(this->getLastRenderTask(opsTask->target(0).proxy()) == opsTask.get());
+ SkASSERT(this->getLastRenderTask(proxy) == opsTask.get());
if (flushTimeOpsTask) {
fOnFlushRenderTasks.push_back(opsTask);
} else {
this->appendTask(opsTask);
- fActiveOpsTask = opsTask.get();
+ if (!fReduceOpsTaskSplitting) {
+ fActiveOpsTask = opsTask.get();
+ }
}
SkDEBUGCODE(this->validate());
@@ -711,36 +727,65 @@
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.
- 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);
+ // 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->setLastRenderTask(proxy.get(), waitTask.get());
- this->closeActiveOpsTask();
this->appendTask(waitTask);
+ } 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);
+ }
}
waitTask->makeClosed(caps);
@@ -755,7 +800,8 @@
size_t dstOffset) {
SkDEBUGCODE(this->validate());
SkASSERT(fContext);
- this->closeActiveOpsTask();
+ // This copies from srcProxy to dstBuffer so it doesn't have a real target.
+ this->closeRenderTasksForNewRenderTask(nullptr);
GrRenderTask* task = this->appendTask(sk_make_sp<GrTransferFromRenderTask>(
srcProxy, srcRect, surfaceColorType, dstColorType,
@@ -782,7 +828,7 @@
SkDEBUGCODE(this->validate());
SkASSERT(fContext);
- this->closeActiveOpsTask();
+ this->closeRenderTasksForNewRenderTask(dstView.proxy());
const GrCaps& caps = *fContext->priv().caps();
GrSurfaceProxy* srcProxy = srcView.proxy();
diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h
index d199ee0..a4cfd9d 100644
--- a/src/gpu/GrDrawingManager.h
+++ b/src/gpu/GrDrawingManager.h
@@ -126,7 +126,10 @@
bool wasAbandoned() const;
- void closeActiveOpsTask();
+ // 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);
// return true if any GrRenderTasks were actually executed; false otherwise
bool executeRenderTasks(int startIndex, int stopIndex, GrOpFlushState*,