Add renderTasks to the DAG before calling addDependency
This makes sure the dependent task is already in the DAG before a
textureResolveRenderTask calls "addBeforeLast".
Bug: skia:
Change-Id: Ib276d41c386fd3d5a237212d60d7bf67a662e419
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/237257
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index a32240c..7e05f0b 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -84,15 +84,21 @@
}
GrRenderTask* GrDrawingManager::RenderTaskDAG::add(sk_sp<GrRenderTask> renderTask) {
- return fRenderTasks.emplace_back(std::move(renderTask)).get();
+ if (renderTask) {
+ return fRenderTasks.emplace_back(std::move(renderTask)).get();
+ }
+ return nullptr;
}
GrRenderTask* GrDrawingManager::RenderTaskDAG::addBeforeLast(sk_sp<GrRenderTask> renderTask) {
SkASSERT(!fRenderTasks.empty());
- // Release 'fRenderTasks.back()' and grab the raw pointer. If we pass in a reference to an array
- // element, and the array grows during emplace_back, then the reference will become invalidated.
- fRenderTasks.emplace_back(fRenderTasks.back().release());
- return (fRenderTasks[fRenderTasks.count() - 2] = std::move(renderTask)).get();
+ if (renderTask) {
+ // Release 'fRenderTasks.back()' and grab the raw pointer, in case the SkTArray grows
+ // and reallocates during emplace_back.
+ fRenderTasks.emplace_back(fRenderTasks.back().release());
+ return (fRenderTasks[fRenderTasks.count() - 2] = std::move(renderTask)).get();
+ }
+ return nullptr;
}
void GrDrawingManager::RenderTaskDAG::add(const SkTArray<sk_sp<GrRenderTask>>& renderTasks) {
@@ -661,23 +667,26 @@
GrRenderTask* GrDrawingManager::newTextureResolveRenderTask(
sk_sp<GrTextureProxy> textureProxy, GrTextureResolveFlags flags, const GrCaps& caps) {
- // Unlike in the "new opsTask" cases, we do not want to close the active opsTask, nor (if we are
+ SkDEBUGCODE(auto* previousTaskBeforeMipsResolve = textureProxy->getLastRenderTask();)
+
+ // Unlike in the "new opsTask" case, we do not want to close the active opsTask, nor (if we are
// in sorting and opsTask reduction mode) the render tasks that depend on the proxy's current
// state. This is because those opsTasks can still receive new ops and because if they refer to
// the mipmapped version of 'textureProxy', they will then come to depend on the render task
// being created here.
- // NOTE: In either case, 'textureProxy' should already be closed at this point (i.e., its state
- // is finished))
- SkDEBUGCODE(auto* previousTaskBeforeMipsResolve = textureProxy->getLastRenderTask();)
- auto textureResolveTask = GrTextureResolveRenderTask::Make(textureProxy, flags, caps);
- // GrTextureResolveRenderTask::Make should have closed the texture proxy's previous task.
- SkASSERT(!previousTaskBeforeMipsResolve || previousTaskBeforeMipsResolve->isClosed());
- SkASSERT(textureProxy->getLastRenderTask() == textureResolveTask.get());
-
+ //
// Add the new textureResolveTask before the fActiveOpsTask (if not in
// sorting/opsTask-splitting-reduction mode) because it will depend upon this resolve task.
// NOTE: Putting it here will also reduce the amount of work required by the topological sort.
- return fDAG.addBeforeLast(std::move(textureResolveTask));
+ auto* resolveTask = static_cast<GrTextureResolveRenderTask*>(fDAG.addBeforeLast(
+ sk_make_sp<GrTextureResolveRenderTask>(std::move(textureProxy), flags)));
+ resolveTask->init(caps);
+
+ // GrTextureResolveRenderTask::init should have closed the texture proxy's previous task.
+ SkASSERT(!previousTaskBeforeMipsResolve || previousTaskBeforeMipsResolve->isClosed());
+ SkASSERT(resolveTask->fTarget->getLastRenderTask() == resolveTask);
+
+ return resolveTask;
}
void GrDrawingManager::newTransferFromRenderTask(sk_sp<GrSurfaceProxy> srcProxy,
@@ -691,9 +700,8 @@
// This copies from srcProxy to dstBuffer so it doesn't have a real target.
this->closeRenderTasksForNewRenderTask(nullptr);
- sk_sp<GrRenderTask> task(new GrTransferFromRenderTask(srcProxy, srcRect, surfaceColorType,
- dstColorType, std::move(dstBuffer),
- dstOffset));
+ GrRenderTask* task = fDAG.add(sk_make_sp<GrTransferFromRenderTask>(
+ srcProxy, srcRect, surfaceColorType, dstColorType, std::move(dstBuffer), dstOffset));
const GrCaps& caps = *fContext->priv().caps();
@@ -702,7 +710,6 @@
task->addDependency(srcProxy.get(), GrMipMapped::kNo, GrTextureResolveManager(this), caps);
task->makeClosed(caps);
- fDAG.add(std::move(task));
// We have closed the previous active oplist but since a new oplist isn't being added there
// shouldn't be an active one.
SkASSERT(!fActiveOpsTask);
@@ -717,7 +724,7 @@
SkASSERT(fContext);
this->closeRenderTasksForNewRenderTask(dstProxy.get());
- sk_sp<GrRenderTask> task = GrCopyRenderTask::Make(srcProxy, srcRect, dstProxy, dstPoint);
+ GrRenderTask* task = fDAG.add(GrCopyRenderTask::Make(srcProxy, srcRect, dstProxy, dstPoint));
if (!task) {
return false;
}
@@ -729,7 +736,6 @@
task->addDependency(srcProxy.get(), GrMipMapped::kNo, GrTextureResolveManager(this), caps);
task->makeClosed(caps);
- fDAG.add(std::move(task));
// We have closed the previous active oplist but since a new oplist isn't being added there
// shouldn't be an active one.
SkASSERT(!fActiveOpsTask);
diff --git a/src/gpu/GrRenderTask.cpp b/src/gpu/GrRenderTask.cpp
index 639f4cf..9541408 100644
--- a/src/gpu/GrRenderTask.cpp
+++ b/src/gpu/GrRenderTask.cpp
@@ -123,8 +123,8 @@
GrRenderTask* textureResolveTask = textureResolveManager.newTextureResolveRenderTask(
sk_ref_sp(textureProxy), GrTextureResolveFlags::kMipMaps, caps);
- // The GrTextureResolveRenderTask factory should have called addDependency (in this
- // instance, recursively) on the textureProxy.
+ // GrTextureResolveRenderTask::init should have called addDependency (in this instance,
+ // recursively) on the textureProxy.
SkASSERT(!dependedOnTask || textureResolveTask->dependsOn(dependedOnTask));
SkASSERT(!textureProxy->texPriv().isDeferred() ||
textureResolveTask->fDeferredProxies.back() == textureProxy);
diff --git a/src/gpu/GrTextureResolveRenderTask.cpp b/src/gpu/GrTextureResolveRenderTask.cpp
index 1e52b87..0ae9356 100644
--- a/src/gpu/GrTextureResolveRenderTask.cpp
+++ b/src/gpu/GrTextureResolveRenderTask.cpp
@@ -13,30 +13,23 @@
#include "src/gpu/GrResourceAllocator.h"
#include "src/gpu/GrTexturePriv.h"
-sk_sp<GrRenderTask> GrTextureResolveRenderTask::Make(
- sk_sp<GrTextureProxy> textureProxy, GrTextureResolveFlags flags, const GrCaps& caps) {
- GrTextureProxy* textureProxyPtr = textureProxy.get();
- sk_sp<GrTextureResolveRenderTask> resolveTask(
- new GrTextureResolveRenderTask(std::move(textureProxy), flags));
-
+void GrTextureResolveRenderTask::init(const GrCaps& caps) {
// Add the target as a dependency: We will read the existing contents of this texture while
// generating mipmap levels and/or resolving MSAA.
//
// NOTE: This must be called before makeClosed.
- resolveTask->addDependency(
- textureProxyPtr, GrMipMapped::kNo, GrTextureResolveManager(nullptr), caps);
- textureProxyPtr->setLastRenderTask(resolveTask.get());
+ this->addDependency(fTarget.get(), GrMipMapped::kNo, GrTextureResolveManager(nullptr), caps);
+ fTarget->setLastRenderTask(this);
// We only resolve the texture; nobody should try to do anything else with this opsTask.
- resolveTask->makeClosed(caps);
+ this->makeClosed(caps);
- if (GrTextureResolveFlags::kMipMaps & flags) {
- SkASSERT(GrMipMapped::kYes == textureProxyPtr->mipMapped());
- SkASSERT(textureProxyPtr->mipMapsAreDirty());
- textureProxyPtr->markMipMapsClean();
+ if (GrTextureResolveFlags::kMipMaps & fResolveFlags) {
+ GrTextureProxy* textureProxy = fTarget->asTextureProxy();
+ SkASSERT(GrMipMapped::kYes == textureProxy->mipMapped());
+ SkASSERT(textureProxy->mipMapsAreDirty());
+ textureProxy->markMipMapsClean();
}
-
- return resolveTask;
}
void GrTextureResolveRenderTask::gatherProxyIntervals(GrResourceAllocator* alloc) const {
diff --git a/src/gpu/GrTextureResolveRenderTask.h b/src/gpu/GrTextureResolveRenderTask.h
index 969f368..d3c95d6 100644
--- a/src/gpu/GrTextureResolveRenderTask.h
+++ b/src/gpu/GrTextureResolveRenderTask.h
@@ -12,16 +12,15 @@
class GrTextureResolveRenderTask final : public GrRenderTask {
public:
- static sk_sp<GrRenderTask> Make(
- sk_sp<GrTextureProxy>, GrTextureResolveFlags, const GrCaps&);
-
-private:
GrTextureResolveRenderTask(sk_sp<GrTextureProxy> textureProxy, GrTextureResolveFlags flags)
: GrRenderTask(std::move(textureProxy))
, fResolveFlags(flags) {
SkASSERT(GrTextureResolveFlags::kNone != fResolveFlags);
}
+ void init(const GrCaps&);
+
+private:
void onPrepare(GrOpFlushState*) override {}
bool onIsUsed(GrSurfaceProxy* proxy) const override {
SkASSERT(proxy != fTarget.get()); // This case should be handled by GrRenderTask.