Revert "Reorder msaa and mipmap resolves to happen all at once"
This reverts commit fd1414608ba1174a26af27067575c65c2dc3d605.
Reason for revert: red bots
Original change's description:
> Reorder msaa and mipmap resolves to happen all at once
>
> Makes it so every renderTask has only one textureResolveTask, and
> modifies GrTextureResolveTask to perform multiple resolves
> back-to-back.
>
> Bug: skia:9406
> Change-Id: I93566cf4b23764bd846a1e0a0848642c9b3a507a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/241936
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=egdaniel@google.com,robertphillips@google.com,csmartdalton@google.com
Change-Id: I12f340da3dfec81477ceeab806ca76ce2b3c397b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:9406
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/242390
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index 42db1eb..6f3dfdd 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -626,11 +626,7 @@
for (int i = 0; i < fDAG.numRenderTasks(); ++i) {
if (fActiveOpsTask != fDAG.renderTask(i)) {
- // The resolveTask associated with the activeTask remains open for as long as the
- // activeTask does.
- bool isActiveResolveTask =
- fActiveOpsTask && fActiveOpsTask->fTextureResolveTask == fDAG.renderTask(i);
- SkASSERT(isActiveResolveTask || fDAG.renderTask(i)->isClosed());
+ SkASSERT(fDAG.renderTask(i)->isClosed());
}
}
@@ -683,9 +679,12 @@
return opsTask;
}
-GrTextureResolveRenderTask* GrDrawingManager::newTextureResolveRenderTask(const GrCaps& caps) {
+GrRenderTask* GrDrawingManager::newTextureResolveRenderTask(
+ sk_sp<GrSurfaceProxy> proxy, GrSurfaceProxy::ResolveFlags flags, const GrCaps& caps) {
+ SkDEBUGCODE(auto* previousTaskBeforeMipsResolve = proxy->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 any proxy's current
+ // 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 'proxy', they will then come to depend on the render task being
// created here.
@@ -693,8 +692,15 @@
// 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 static_cast<GrTextureResolveRenderTask*>(fDAG.addBeforeLast(
- sk_make_sp<GrTextureResolveRenderTask>()));
+ auto* resolveTask = static_cast<GrTextureResolveRenderTask*>(fDAG.addBeforeLast(
+ sk_make_sp<GrTextureResolveRenderTask>(std::move(proxy), 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::newWaitRenderTask(sk_sp<GrSurfaceProxy> proxy,
diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h
index 72f3211..6c6d669 100644
--- a/src/gpu/GrDrawingManager.h
+++ b/src/gpu/GrDrawingManager.h
@@ -27,7 +27,6 @@
class GrRenderTargetProxy;
class GrSoftwarePathRenderer;
class GrTextureContext;
-class GrTextureResolveRenderTask;
class SkDeferredDisplayList;
class GrDrawingManager {
@@ -50,10 +49,12 @@
// others). An unmanaged one is created and used by the onFlushCallback.
sk_sp<GrOpsTask> newOpsTask(sk_sp<GrRenderTargetProxy>, bool managedOpsTask);
- // Create a render task that can resolve MSAA and/or regenerate mipmap levels on proxies. This
- // method will only add the new render task to the list. It is up to the caller to call
- // addProxy() on the returned object.
- GrTextureResolveRenderTask* newTextureResolveRenderTask(const GrCaps&);
+ // Create a new, specialized, render task that will regenerate mipmap levels and/or resolve
+ // MSAA (depending on ResolveFlags). This method will add the new render task to the list of
+ // render tasks and make it depend on the target texture proxy. It is up to the caller to add
+ // any dependencies on the new render task.
+ GrRenderTask* newTextureResolveRenderTask(
+ sk_sp<GrSurfaceProxy>, GrSurfaceProxy::ResolveFlags, const GrCaps&);
// Create a new render task that will cause the gpu to wait on semaphores before executing any
// more RenderTasks that target proxy. It is possible for this wait to also block additional
diff --git a/src/gpu/GrOnFlushResourceProvider.cpp b/src/gpu/GrOnFlushResourceProvider.cpp
index 2cf4af8..781df12 100644
--- a/src/gpu/GrOnFlushResourceProvider.cpp
+++ b/src/gpu/GrOnFlushResourceProvider.cpp
@@ -49,9 +49,8 @@
renderTask->makeClosed(*this->caps());
}
auto task = static_cast<GrTextureResolveRenderTask*>(fDrawingMgr->fOnFlushRenderTasks.push_back(
- sk_make_sp<GrTextureResolveRenderTask>()).get());
- task->addProxy(textureProxy, resolveFlags, *this->caps());
- task->makeClosed(*this->caps());
+ sk_make_sp<GrTextureResolveRenderTask>(std::move(textureProxy), resolveFlags)).get());
+ task->init(*this->caps());
}
bool GrOnFlushResourceProvider::assignUniqueKeyToProxy(const GrUniqueKey& key,
diff --git a/src/gpu/GrRenderTask.cpp b/src/gpu/GrRenderTask.cpp
index 287fd5a..42906dd 100644
--- a/src/gpu/GrRenderTask.cpp
+++ b/src/gpu/GrRenderTask.cpp
@@ -10,7 +10,6 @@
#include "src/gpu/GrRenderTargetPriv.h"
#include "src/gpu/GrStencilAttachment.h"
#include "src/gpu/GrTextureProxyPriv.h"
-#include "src/gpu/GrTextureResolveRenderTask.h"
uint32_t GrRenderTask::CreateUniqueID() {
static std::atomic<uint32_t> nextID{1};
@@ -62,12 +61,6 @@
}
}
- if (fTextureResolveTask) {
- this->addDependency(fTextureResolveTask);
- fTextureResolveTask->makeClosed(caps);
- fTextureResolveTask = nullptr;
- }
-
this->setFlag(kClosed_Flag);
}
@@ -153,23 +146,18 @@
// Does this proxy have msaa to resolve and/or mipmaps to regenerate?
if (GrSurfaceProxy::ResolveFlags::kNone != resolveFlags) {
- if (!fTextureResolveTask) {
- fTextureResolveTask = textureResolveManager.newTextureResolveRenderTask(caps);
- }
- fTextureResolveTask->addProxy(sk_ref_sp(dependedOn), resolveFlags, caps);
-
- // addProxy() should have closed the texture proxy's previous task.
- SkASSERT(!dependedOnTask || dependedOnTask->isClosed());
- SkASSERT(dependedOn->getLastRenderTask() == fTextureResolveTask);
+ // Create a renderTask that resolves the texture's mipmap data.
+ GrRenderTask* textureResolveTask = textureResolveManager.newTextureResolveRenderTask(
+ sk_ref_sp(dependedOn), resolveFlags, caps);
#ifdef SK_DEBUG
- // addProxy() should have called addDependency (in this instance, recursively) on
- // fTextureResolveTask.
+ // GrTextureResolveRenderTask::init should have called addDependency (in this instance,
+ // recursively) on the textureResolveTask.
if (dependedOnTask) {
- SkASSERT(fTextureResolveTask->dependsOn(dependedOnTask));
+ SkASSERT(textureResolveTask->dependsOn(dependedOnTask));
}
if (textureProxy && textureProxy->texPriv().isDeferred()) {
- SkASSERT(fTextureResolveTask->fDeferredProxies.back() == textureProxy);
+ SkASSERT(textureResolveTask->fDeferredProxies.back() == textureProxy);
}
// The GrTextureResolveRenderTask factory should have also marked the proxy clean, set the
@@ -180,12 +168,13 @@
if (textureProxy) {
SkASSERT(!textureProxy->mipMapsAreDirty());
}
- SkASSERT(dependedOn->getLastRenderTask() == fTextureResolveTask);
+ SkASSERT(dependedOn->getLastRenderTask() == textureResolveTask);
+ SkASSERT(textureResolveTask->isClosed());
#endif
- return;
- }
- if (textureProxy && textureProxy->texPriv().isDeferred()) {
+ // Fall through and add textureResolveTask as a dependency of "this".
+ dependedOnTask = textureResolveTask;
+ } else if (textureProxy && textureProxy->texPriv().isDeferred()) {
fDeferredProxies.push_back(textureProxy);
}
diff --git a/src/gpu/GrRenderTask.h b/src/gpu/GrRenderTask.h
index 0cb006f..2ad4656 100644
--- a/src/gpu/GrRenderTask.h
+++ b/src/gpu/GrRenderTask.h
@@ -17,7 +17,6 @@
class GrOpFlushState;
class GrOpsTask;
class GrResourceAllocator;
-class GrTextureResolveRenderTask;
// This class abstracts a task that targets a single GrSurfaceProxy, participates in the
// GrDrawingManager's DAG, and implements the onExecute method to modify its target proxy's
@@ -78,9 +77,7 @@
void visitTargetAndSrcProxies_debugOnly(const VisitSurfaceProxyFunc& fn) const {
this->visitProxies_debugOnly(fn);
- if (fTarget) {
- fn(fTarget.get(), GrMipMapped::kNo);
- }
+ fn(fTarget.get(), GrMipMapped::kNo);
}
#endif
@@ -188,11 +185,6 @@
SkSTArray<1, GrRenderTask*, true> fDependencies;
// 'this' GrRenderTask's output is relied on by the GrRenderTasks in 'fDependents'
SkSTArray<1, GrRenderTask*, true> fDependents;
-
- // For performance reasons, we should perform texture resolves back-to-back as much as possible.
- // (http://skbug.com/9406). To accomplish this, we make and reuse one single resolve task for
- // each render task, then add it as a dependency during makeClosed().
- GrTextureResolveRenderTask* fTextureResolveTask = nullptr;
};
#endif
diff --git a/src/gpu/GrTextureResolveManager.h b/src/gpu/GrTextureResolveManager.h
index f610c6b..a9b0182 100644
--- a/src/gpu/GrTextureResolveManager.h
+++ b/src/gpu/GrTextureResolveManager.h
@@ -25,9 +25,11 @@
explicit GrTextureResolveManager(GrDrawingManager* drawingManager)
: fDrawingManager(drawingManager) {}
- GrTextureResolveRenderTask* newTextureResolveRenderTask(const GrCaps& caps) const {
+ GrRenderTask* newTextureResolveRenderTask(sk_sp<GrSurfaceProxy> proxy,
+ GrSurfaceProxy::ResolveFlags resolveFlags,
+ const GrCaps& caps) const {
SkASSERT(fDrawingManager);
- return fDrawingManager->newTextureResolveRenderTask(caps);
+ return fDrawingManager->newTextureResolveRenderTask(std::move(proxy), resolveFlags, caps);
}
private:
diff --git a/src/gpu/GrTextureResolveRenderTask.cpp b/src/gpu/GrTextureResolveRenderTask.cpp
index d97e5d5..0b77383 100644
--- a/src/gpu/GrTextureResolveRenderTask.cpp
+++ b/src/gpu/GrTextureResolveRenderTask.cpp
@@ -14,83 +14,58 @@
#include "src/gpu/GrResourceAllocator.h"
#include "src/gpu/GrTexturePriv.h"
-GrTextureResolveRenderTask::~GrTextureResolveRenderTask() {
- for (const auto& resolve : fResolves) {
- // Ensure the proxy doesn't keep hold of a dangling back pointer.
- resolve.fProxy->setLastRenderTask(nullptr);
- }
-}
-
-void GrTextureResolveRenderTask::addProxy(
- sk_sp<GrSurfaceProxy> proxy, GrSurfaceProxy::ResolveFlags flags, const GrCaps& caps) {
- // Ensure the last render task that operated on the proxy is closed. That's where msaa and
- // mipmaps should have been marked dirty.
- SkASSERT(!proxy->getLastRenderTask() || proxy->getLastRenderTask()->isClosed());
- SkASSERT(GrSurfaceProxy::ResolveFlags::kNone != flags);
-
- if (GrSurfaceProxy::ResolveFlags::kMSAA & flags) {
- GrRenderTargetProxy* renderTargetProxy = proxy->asRenderTargetProxy();
+void GrTextureResolveRenderTask::init(const GrCaps& caps) {
+ if (GrSurfaceProxy::ResolveFlags::kMSAA & fResolveFlags) {
+ GrRenderTargetProxy* renderTargetProxy = fTarget->asRenderTargetProxy();
SkASSERT(renderTargetProxy);
SkASSERT(renderTargetProxy->isMSAADirty());
renderTargetProxy->markMSAAResolved();
}
- if (GrSurfaceProxy::ResolveFlags::kMipMaps & flags) {
- GrTextureProxy* textureProxy = proxy->asTextureProxy();
+ if (GrSurfaceProxy::ResolveFlags::kMipMaps & fResolveFlags) {
+ GrTextureProxy* textureProxy = fTarget->asTextureProxy();
SkASSERT(GrMipMapped::kYes == textureProxy->mipMapped());
SkASSERT(textureProxy->mipMapsAreDirty());
textureProxy->markMipMapsClean();
}
- // Add the proxy as a dependency: We will read the existing contents of this texture while
+ // Add the target as a dependency: We will read the existing contents of this texture while
// generating mipmap levels and/or resolving MSAA.
- this->addDependency(proxy.get(), GrMipMapped::kNo, GrTextureResolveManager(nullptr), caps);
- proxy->setLastRenderTask(this);
+ //
+ // NOTE: This must be called before makeClosed.
+ this->addDependency(fTarget.get(), GrMipMapped::kNo, GrTextureResolveManager(nullptr), caps);
+ fTarget->setLastRenderTask(this);
- fResolves.emplace_back(std::move(proxy), flags);
+ // We only resolve the texture; nobody should try to do anything else with this opsTask.
+ this->makeClosed(caps);
}
void GrTextureResolveRenderTask::gatherProxyIntervals(GrResourceAllocator* alloc) const {
- // This renderTask doesn't have "normal" ops, however we still need to add intervals so
- // fEndOfOpsTaskOpIndices will remain in sync. We create fake op#'s to capture the fact that we
- // manipulate the resolve proxies.
- auto fakeOp = alloc->curOp();
- for (const auto& resolve : fResolves) {
- alloc->addInterval(resolve.fProxy.get(), fakeOp, fakeOp,
- GrResourceAllocator::ActualUse::kYes);
- }
+ // This renderTask doesn't have "normal" ops. In this case we still need to add an interval (so
+ // fEndOfOpsTaskOpIndices will remain in sync), so we create a fake op# to capture the fact that
+ // we manipulate fTarget.
+ alloc->addInterval(fTarget.get(), alloc->curOp(), alloc->curOp(),
+ GrResourceAllocator::ActualUse::kYes);
alloc->incOps();
}
bool GrTextureResolveRenderTask::onExecute(GrOpFlushState* flushState) {
- // Resolve all msaa back-to-back, before regenerating mipmaps.
- for (const auto& resolve : fResolves) {
- if (GrSurfaceProxy::ResolveFlags::kMSAA & resolve.fFlags) {
- GrRenderTarget* renderTarget = resolve.fProxy->peekRenderTarget();
- SkASSERT(renderTarget);
- if (renderTarget->needsResolve()) {
- flushState->gpu()->resolveRenderTarget(renderTarget);
- }
+ // Resolve msaa before regenerating mipmaps.
+ if (GrSurfaceProxy::ResolveFlags::kMSAA & fResolveFlags) {
+ GrRenderTarget* renderTarget = fTarget->peekRenderTarget();
+ SkASSERT(renderTarget);
+ if (renderTarget->needsResolve()) {
+ flushState->gpu()->resolveRenderTarget(renderTarget);
}
}
- // Regenerate all mipmaps back-to-back.
- for (const auto& resolve : fResolves) {
- if (GrSurfaceProxy::ResolveFlags::kMipMaps & resolve.fFlags) {
- GrTexture* texture = resolve.fProxy->peekTexture();
- SkASSERT(texture);
- if (texture->texturePriv().mipMapsAreDirty()) {
- flushState->gpu()->regenerateMipMapLevels(texture);
- }
+
+ if (GrSurfaceProxy::ResolveFlags::kMipMaps & fResolveFlags) {
+ GrTexture* texture = fTarget->peekTexture();
+ SkASSERT(texture);
+ if (texture->texturePriv().mipMapsAreDirty()) {
+ flushState->gpu()->regenerateMipMapLevels(texture);
}
}
return true;
}
-
-#ifdef SK_DEBUG
-void GrTextureResolveRenderTask::visitProxies_debugOnly(const VisitSurfaceProxyFunc& fn) const {
- for (const auto& resolve : fResolves) {
- fn(resolve.fProxy.get(), GrMipMapped::kNo);
- }
-}
-#endif
diff --git a/src/gpu/GrTextureResolveRenderTask.h b/src/gpu/GrTextureResolveRenderTask.h
index b175b0b..86365dc 100644
--- a/src/gpu/GrTextureResolveRenderTask.h
+++ b/src/gpu/GrTextureResolveRenderTask.h
@@ -12,10 +12,17 @@
class GrTextureResolveRenderTask final : public GrRenderTask {
public:
- GrTextureResolveRenderTask() : GrRenderTask(nullptr) {}
- ~GrTextureResolveRenderTask() override;
+ GrTextureResolveRenderTask(sk_sp<GrSurfaceProxy> proxy,
+ GrSurfaceProxy::ResolveFlags resolveFlags)
+ : GrRenderTask(std::move(proxy))
+ , fResolveFlags(resolveFlags) {
+ // Ensure the last render task that operated on the target is closed. That's where msaa and
+ // mipmaps should have been marked dirty.
+ SkASSERT(!fTarget->getLastRenderTask() || fTarget->getLastRenderTask()->isClosed());
+ SkASSERT(GrSurfaceProxy::ResolveFlags::kNone != fResolveFlags);
+ }
- void addProxy(sk_sp<GrSurfaceProxy>, GrSurfaceProxy::ResolveFlags, const GrCaps&);
+ void init(const GrCaps&);
private:
void onPrepare(GrOpFlushState*) override {}
@@ -33,17 +40,11 @@
bool onExecute(GrOpFlushState*) override;
#ifdef SK_DEBUG
- SkDEBUGCODE(void visitProxies_debugOnly(const VisitSurfaceProxyFunc&) const override;)
+ // No non-dst proxies.
+ void visitProxies_debugOnly(const VisitSurfaceProxyFunc& fn) const override {}
#endif
- struct Resolve {
- Resolve(sk_sp<GrSurfaceProxy> proxy, GrSurfaceProxy::ResolveFlags flags)
- : fProxy(std::move(proxy)), fFlags(flags) {}
- sk_sp<GrSurfaceProxy> fProxy;
- GrSurfaceProxy::ResolveFlags fFlags;
- };
-
- SkSTArray<4, Resolve> fResolves;
+ const GrSurfaceProxy::ResolveFlags fResolveFlags;
};
#endif