Don't pass around renderTargetContexts from onFlush callbacks
The drawing manager was just grabbing an opsTask off of these contexts
anyway. Instead, the onFlushResourceProvider can just snag an opsTask
off the renderTargetContext and populate the drawing manager's list of
onFlushRenderTasks.
Bug: skia:
Change-Id: I3bdb48176364bbd6e5a34fab437c45ed77d6687f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/236760
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/GrCopyRenderTask.h b/src/gpu/GrCopyRenderTask.h
index a04c89f..319d5da 100644
--- a/src/gpu/GrCopyRenderTask.h
+++ b/src/gpu/GrCopyRenderTask.h
@@ -36,6 +36,12 @@
}
bool onExecute(GrOpFlushState*) override;
+#ifdef SK_DEBUG
+ void visitProxies_debugOnly(const GrOp::VisitProxyFunc& fn) const override {
+ fn(fSrcProxy.get(), GrMipMapped::kNo);
+ }
+#endif
+
sk_sp<GrSurfaceProxy> fSrcProxy;
SkIRect fSrcRect;
SkIPoint fDstPoint;
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index 75abb7f..987b956 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -281,33 +281,36 @@
if (!fOnFlushCBObjects.empty()) {
fDAG.gatherIDs(&fFlushingRenderTaskIDs);
- SkSTArray<4, std::unique_ptr<GrRenderTargetContext>> renderTargetContexts;
for (GrOnFlushCallbackObject* onFlushCBObject : fOnFlushCBObjects) {
onFlushCBObject->preFlush(&onFlushProvider, fFlushingRenderTaskIDs.begin(),
- fFlushingRenderTaskIDs.count(), &renderTargetContexts);
- for (const auto& rtc : renderTargetContexts) {
- sk_sp<GrOpsTask> onFlushOpsTask = sk_ref_sp(rtc->getOpsTask());
- if (!onFlushOpsTask) {
- continue; // Odd - but not a big deal
- }
+ fFlushingRenderTaskIDs.count());
+ }
+ for (const auto& onFlushRenderTask : fOnFlushRenderTasks) {
+ onFlushRenderTask->makeClosed(*fContext->priv().caps());
#ifdef SK_DEBUG
- // OnFlush callbacks are already invoked during flush, and are therefore expected to
- // handle resource allocation & usage on their own. (No deferred or lazy proxies!)
- onFlushOpsTask->visitProxies_debugOnly([](GrSurfaceProxy* p, GrMipMapped) {
- SkASSERT(!p->asTextureProxy() || !p->asTextureProxy()->texPriv().isDeferred());
- SkASSERT(GrSurfaceProxy::LazyState::kNot == p->lazyInstantiationState());
- });
+ // OnFlush callbacks are invoked during flush, and are therefore expected to handle
+ // resource allocation & usage on their own. (No deferred or lazy proxies!)
+ onFlushRenderTask->visitTargetAndSrcProxies_debugOnly(
+ [](GrSurfaceProxy* p, GrMipMapped mipMapped) {
+ SkASSERT(!p->asTextureProxy() || !p->asTextureProxy()->texPriv().isDeferred());
+ SkASSERT(GrSurfaceProxy::LazyState::kNot == p->lazyInstantiationState());
+ if (GrMipMapped::kYes == mipMapped) {
+ // The onFlush callback is responsible for regenerating mips if needed.
+ SkASSERT(p->asTextureProxy() && !p->asTextureProxy()->mipMapsAreDirty());
+ }
+ });
#endif
- onFlushOpsTask->makeClosed(*fContext->priv().caps());
- onFlushOpsTask->prepare(&flushState);
- fOnFlushCBOpsTasks.push_back(std::move(onFlushOpsTask));
- }
- renderTargetContexts.reset();
+ onFlushRenderTask->prepare(&flushState);
}
}
#if 0
// Enable this to print out verbose GrOp information
+ SkDEBUGCODE(SkDebugf("onFlush renderTasks:"));
+ for (const auto& onFlushRenderTask : fOnFlushRenderTasks) {
+ SkDEBUGCODE(onFlushRenderTask->dump();)
+ }
+ SkDEBUGCODE(SkDebugf("Normal renderTasks:"));
for (int i = 0; i < fRenderTasks.count(); ++i) {
SkDEBUGCODE(fRenderTasks[i]->dump();)
}
@@ -434,13 +437,13 @@
// memory pressure.
static constexpr int kMaxRenderTasksBeforeFlush = 100;
- // Execute the onFlush op lists first, if any.
- for (sk_sp<GrOpsTask>& onFlushOpsTask : fOnFlushCBOpsTasks) {
- if (!onFlushOpsTask->execute(flushState)) {
- SkDebugf("WARNING: onFlushOpsTask failed to execute.\n");
+ // Execute the onFlush renderTasks first, if any.
+ for (sk_sp<GrRenderTask>& onFlushRenderTask : fOnFlushRenderTasks) {
+ if (!onFlushRenderTask->execute(flushState)) {
+ SkDebugf("WARNING: onFlushRenderTask failed to execute.\n");
}
- SkASSERT(onFlushOpsTask->unique());
- onFlushOpsTask = nullptr;
+ SkASSERT(onFlushRenderTask->unique());
+ onFlushRenderTask = nullptr;
(*numRenderTasksExecuted)++;
if (*numRenderTasksExecuted >= kMaxRenderTasksBeforeFlush) {
flushState->gpu()->finishFlush(nullptr, 0, SkSurface::BackendSurfaceAccess::kNoAccess,
@@ -448,7 +451,7 @@
*numRenderTasksExecuted = 0;
}
}
- fOnFlushCBOpsTasks.reset();
+ fOnFlushRenderTasks.reset();
// Execute the normal op lists.
for (int i = startIndex; i < stopIndex; ++i) {
diff --git a/src/gpu/GrDrawingManager.h b/src/gpu/GrDrawingManager.h
index 25b0343..240c546 100644
--- a/src/gpu/GrDrawingManager.h
+++ b/src/gpu/GrDrawingManager.h
@@ -211,8 +211,8 @@
GrOpsTask* fActiveOpsTask = nullptr;
// These are the IDs of the opsTask currently being flushed (in internalFlush)
SkSTArray<8, uint32_t, true> fFlushingRenderTaskIDs;
- // These are the new opsTask generated by the onFlush CBs
- SkSTArray<8, sk_sp<GrOpsTask>> fOnFlushCBOpsTasks;
+ // These are the new renderTasks generated by the onFlush CBs
+ SkSTArray<4, sk_sp<GrRenderTask>> fOnFlushRenderTasks;
std::unique_ptr<GrTextContext> fTextContext;
diff --git a/src/gpu/GrOnFlushResourceProvider.cpp b/src/gpu/GrOnFlushResourceProvider.cpp
index 32c6585..8d7c95c 100644
--- a/src/gpu/GrOnFlushResourceProvider.cpp
+++ b/src/gpu/GrOnFlushResourceProvider.cpp
@@ -16,9 +16,7 @@
#include "src/gpu/GrSurfaceProxy.h"
std::unique_ptr<GrRenderTargetContext> GrOnFlushResourceProvider::makeRenderTargetContext(
- sk_sp<GrSurfaceProxy> proxy,
- GrColorType colorType,
- sk_sp<SkColorSpace> colorSpace,
+ sk_sp<GrSurfaceProxy> proxy, GrColorType colorType, sk_sp<SkColorSpace> colorSpace,
const SkSurfaceProps* props) {
// Since this is at flush time and these won't be allocated for us by the GrResourceAllocator
// we have to manually ensure it is allocated here. The proxy had best have been created
@@ -36,6 +34,9 @@
renderTargetContext->discard();
+ // FIXME: http://skbug.com/9357: This breaks if the renderTargetContext splits its opsTask.
+ fDrawingMgr->fOnFlushRenderTasks.push_back(sk_ref_sp(renderTargetContext->getOpsTask()));
+
return renderTargetContext;
}
diff --git a/src/gpu/GrOnFlushResourceProvider.h b/src/gpu/GrOnFlushResourceProvider.h
index f26f8d3..e4fe6f1 100644
--- a/src/gpu/GrOnFlushResourceProvider.h
+++ b/src/gpu/GrOnFlushResourceProvider.h
@@ -35,8 +35,8 @@
* callback. The callback should return the render target contexts used to render the atlases
* in 'results'.
*/
- virtual void preFlush(GrOnFlushResourceProvider*, const uint32_t* opsTaskIDs, int numOpsTaskIDs,
- SkTArray<std::unique_ptr<GrRenderTargetContext>>* results) = 0;
+ virtual void preFlush(GrOnFlushResourceProvider*, const uint32_t* opsTaskIDs,
+ int numOpsTaskIDs) = 0;
/**
* Called once flushing is complete and all ops indicated by preFlush have been executed and
diff --git a/src/gpu/GrOpsTask.h b/src/gpu/GrOpsTask.h
index a89b22c..a90b7e6 100644
--- a/src/gpu/GrOpsTask.h
+++ b/src/gpu/GrOpsTask.h
@@ -97,7 +97,7 @@
SkDEBUGCODE(void dump(bool printDependencies) const override;)
SkDEBUGCODE(int numClips() const override { return fNumClips; })
- SkDEBUGCODE(void visitProxies_debugOnly(const GrOp::VisitProxyFunc&) const;)
+ SkDEBUGCODE(void visitProxies_debugOnly(const GrOp::VisitProxyFunc&) const override;)
private:
bool isNoOp() const {
diff --git a/src/gpu/GrRenderTargetContext.h b/src/gpu/GrRenderTargetContext.h
index 10094ec..83c20f7 100644
--- a/src/gpu/GrRenderTargetContext.h
+++ b/src/gpu/GrRenderTargetContext.h
@@ -519,6 +519,7 @@
friend class GrAtlasTextBlob; // for access to add[Mesh]DrawOp
friend class GrClipStackClip; // for access to getOpsTask
+ friend class GrOnFlushResourceProvider; // for access to getOpsTask (http://skbug.com/9357)
friend class GrDrawingManager; // for ctor
friend class GrRenderTargetContextPriv;
diff --git a/src/gpu/GrRenderTask.h b/src/gpu/GrRenderTask.h
index f13a0de..3f66d90 100644
--- a/src/gpu/GrRenderTask.h
+++ b/src/gpu/GrRenderTask.h
@@ -57,12 +57,21 @@
*/
virtual GrOpsTask* asOpsTask() { return nullptr; }
+#ifdef SK_DEBUG
/*
* Dump out the GrRenderTask dependency DAG
*/
- SkDEBUGCODE(virtual void dump(bool printDependencies) const;)
+ virtual void dump(bool printDependencies) const;
- SkDEBUGCODE(virtual int numClips() const { return 0; })
+ virtual int numClips() const { return 0; }
+
+ virtual void visitProxies_debugOnly(const GrOp::VisitProxyFunc&) const = 0;
+
+ void visitTargetAndSrcProxies_debugOnly(const GrOp::VisitProxyFunc& fn) const {
+ this->visitProxies_debugOnly(fn);
+ fn(fTarget.get(), GrMipMapped::kNo);
+ }
+#endif
protected:
// In addition to just the GrSurface being allocated, has the stencil buffer been allocated (if
diff --git a/src/gpu/GrTextureResolveRenderTask.h b/src/gpu/GrTextureResolveRenderTask.h
index 00d835c..969f368 100644
--- a/src/gpu/GrTextureResolveRenderTask.h
+++ b/src/gpu/GrTextureResolveRenderTask.h
@@ -36,6 +36,11 @@
bool onExecute(GrOpFlushState*) override;
+#ifdef SK_DEBUG
+ // No non-dst proxies.
+ void visitProxies_debugOnly(const GrOp::VisitProxyFunc& fn) const override {}
+#endif
+
const GrTextureResolveFlags fResolveFlags;
};
diff --git a/src/gpu/GrTransferFromRenderTask.h b/src/gpu/GrTransferFromRenderTask.h
index d30ecc4..08ad678 100644
--- a/src/gpu/GrTransferFromRenderTask.h
+++ b/src/gpu/GrTransferFromRenderTask.h
@@ -42,6 +42,12 @@
bool onExecute(GrOpFlushState*) override;
+#ifdef SK_DEBUG
+ void visitProxies_debugOnly(const GrOp::VisitProxyFunc& fn) const override {
+ fn(fSrcProxy.get(), GrMipMapped::kNo);
+ }
+#endif
+
sk_sp<GrSurfaceProxy> fSrcProxy;
SkIRect fSrcRect;
GrColorType fSurfaceColorType;
diff --git a/src/gpu/ccpr/GrCCPerFlushResources.cpp b/src/gpu/ccpr/GrCCPerFlushResources.cpp
index 223d9ad..72ff29c 100644
--- a/src/gpu/ccpr/GrCCPerFlushResources.cpp
+++ b/src/gpu/ccpr/GrCCPerFlushResources.cpp
@@ -499,8 +499,7 @@
(int16_t)atlasIBounds.right(), (int16_t)atlasIBounds.bottom()};
}
-bool GrCCPerFlushResources::finalize(GrOnFlushResourceProvider* onFlushRP,
- SkTArray<std::unique_ptr<GrRenderTargetContext>>* out) {
+bool GrCCPerFlushResources::finalize(GrOnFlushResourceProvider* onFlushRP) {
SkASSERT(this->isMapped());
SkASSERT(fNextPathInstanceIdx == fEndPathInstance);
SkASSERT(fNextCopyInstanceIdx == fEndCopyInstance);
@@ -553,7 +552,6 @@
}
baseCopyInstance = endCopyInstance;
}
- out->push_back(std::move(rtc));
}
SkASSERT(fCopyPathRanges.count() == copyRangeIdx);
SkASSERT(fNextCopyInstanceIdx == baseCopyInstance);
@@ -590,7 +588,6 @@
atlas->getStrokeBatchID(), atlas->drawBounds());
}
rtc->addDrawOp(GrNoClip(), std::move(op));
- out->push_back(std::move(rtc));
}
SkASSERT(atlas->getEndStencilResolveInstance() >= baseStencilResolveInstance);
diff --git a/src/gpu/ccpr/GrCCPerFlushResources.h b/src/gpu/ccpr/GrCCPerFlushResources.h
index f2504e6..d446da7 100644
--- a/src/gpu/ccpr/GrCCPerFlushResources.h
+++ b/src/gpu/ccpr/GrCCPerFlushResources.h
@@ -107,8 +107,7 @@
}
// Finishes off the GPU buffers and renders the atlas(es).
- bool finalize(GrOnFlushResourceProvider*,
- SkTArray<std::unique_ptr<GrRenderTargetContext>>* out);
+ bool finalize(GrOnFlushResourceProvider*);
// Accessors used by draw calls, once the resources have been finalized.
const GrCCFiller& filler() const { SkASSERT(!this->isMapped()); return fFiller; }
diff --git a/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp b/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp
index 4798b6f..4e86d7e 100644
--- a/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp
+++ b/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp
@@ -228,10 +228,7 @@
}
void GrCoverageCountingPathRenderer::preFlush(
- GrOnFlushResourceProvider* onFlushRP,
- const uint32_t* opsTaskIDs,
- int numOpsTaskIDs,
- SkTArray<std::unique_ptr<GrRenderTargetContext>>* out) {
+ GrOnFlushResourceProvider* onFlushRP, const uint32_t* opsTaskIDs, int numOpsTaskIDs) {
using DoCopiesToA8Coverage = GrCCDrawPathsOp::DoCopiesToA8Coverage;
SkASSERT(!fFlushing);
SkASSERT(fFlushingPaths.empty());
@@ -308,7 +305,7 @@
}
// Allocate resources and then render the atlas(es).
- if (!resources->finalize(onFlushRP, out)) {
+ if (!resources->finalize(onFlushRP)) {
return;
}
diff --git a/src/gpu/ccpr/GrCoverageCountingPathRenderer.h b/src/gpu/ccpr/GrCoverageCountingPathRenderer.h
index 4a7ca18..e70a15a 100644
--- a/src/gpu/ccpr/GrCoverageCountingPathRenderer.h
+++ b/src/gpu/ccpr/GrCoverageCountingPathRenderer.h
@@ -65,8 +65,8 @@
const GrCaps&);
// GrOnFlushCallbackObject overrides.
- void preFlush(GrOnFlushResourceProvider*, const uint32_t* opsTaskIDs, int numOpsTaskIDs,
- SkTArray<std::unique_ptr<GrRenderTargetContext>>* out) override;
+ void preFlush(GrOnFlushResourceProvider*, const uint32_t* opsTaskIDs,
+ int numOpsTaskIDs) override;
void postFlush(GrDeferredUploadToken, const uint32_t* opsTaskIDs, int numOpsTaskIDs) override;
void purgeCacheEntriesOlderThan(GrProxyProvider*, const GrStdSteadyClock::time_point&);
diff --git a/src/gpu/ops/GrSmallPathRenderer.h b/src/gpu/ops/GrSmallPathRenderer.h
index e7017b1..a916fc1 100644
--- a/src/gpu/ops/GrSmallPathRenderer.h
+++ b/src/gpu/ops/GrSmallPathRenderer.h
@@ -33,10 +33,9 @@
// the list of active OnFlushBackkbackObjects in an freeGpuResources call (i.e., we accept the
// default retainOnFreeGpuResources implementation).
- void preFlush(GrOnFlushResourceProvider* onFlushResourceProvider, const uint32_t*, int,
- SkTArray<std::unique_ptr<GrRenderTargetContext>>*) override {
+ void preFlush(GrOnFlushResourceProvider* onFlushRP, const uint32_t*, int) override {
if (fAtlas) {
- fAtlas->instantiate(onFlushResourceProvider);
+ fAtlas->instantiate(onFlushRP);
}
}
diff --git a/src/gpu/text/GrAtlasManager.h b/src/gpu/text/GrAtlasManager.h
index 650b905..1799d8d 100644
--- a/src/gpu/text/GrAtlasManager.h
+++ b/src/gpu/text/GrAtlasManager.h
@@ -88,11 +88,10 @@
// GrOnFlushCallbackObject overrides
- void preFlush(GrOnFlushResourceProvider* onFlushResourceProvider, const uint32_t*, int,
- SkTArray<std::unique_ptr<GrRenderTargetContext>>*) override {
+ void preFlush(GrOnFlushResourceProvider* onFlushRP, const uint32_t*, int) override {
for (int i = 0; i < kMaskFormatCount; ++i) {
if (fAtlases[i]) {
- fAtlases[i]->instantiate(onFlushResourceProvider);
+ fAtlases[i]->instantiate(onFlushRP);
}
}
}