ccpr: Remove GrCCDrawPathsOp's back-pointer into CCPR
Bug: skia:7988
Change-Id: Ia05173e90fa2cda28de6ae2a9aaee577eaf4bc65
Reviewed-on: https://skia-review.googlesource.com/129621
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/ccpr/GrCCDrawPathsOp.cpp b/src/gpu/ccpr/GrCCDrawPathsOp.cpp
index 96c1134..b3fd711 100644
--- a/src/gpu/ccpr/GrCCDrawPathsOp.cpp
+++ b/src/gpu/ccpr/GrCCDrawPathsOp.cpp
@@ -22,16 +22,13 @@
return false;
}
-GrCCDrawPathsOp::GrCCDrawPathsOp(GrCoverageCountingPathRenderer* ccpr, GrPaint&& paint,
- const SkIRect& clipIBounds, const SkMatrix& viewMatrix,
+GrCCDrawPathsOp::GrCCDrawPathsOp(GrPaint&& paint, const SkIRect& clipIBounds, const SkMatrix& m,
const SkPath& path, const SkRect& devBounds)
: GrDrawOp(ClassID())
- , fCCPR(ccpr)
, fSRGBFlags(GrPipeline::SRGBFlagsFromPaint(paint))
- , fViewMatrixIfUsingLocalCoords(has_coord_transforms(paint) ? viewMatrix : SkMatrix::I())
- , fDraws({clipIBounds, viewMatrix, path, paint.getColor(), nullptr})
+ , fViewMatrixIfUsingLocalCoords(has_coord_transforms(paint) ? m : SkMatrix::I())
+ , fDraws({clipIBounds, m, path, paint.getColor(), nullptr})
, fProcessors(std::move(paint)) {
- SkDEBUGCODE(fCCPR->incrDrawOpCount_debugOnly());
SkDEBUGCODE(fBaseInstance = -1);
// FIXME: intersect with clip bounds to (hopefully) improve batching.
// (This is nontrivial due to assumptions in generating the octagon cover geometry.)
@@ -39,17 +36,15 @@
}
GrCCDrawPathsOp::~GrCCDrawPathsOp() {
- if (fOwningRTPendingPaths) {
+ if (fOwningPerOpListPaths) {
// Remove CCPR's dangling pointer to this Op before deleting it.
- fOwningRTPendingPaths->fDrawOps.remove(this);
+ fOwningPerOpListPaths->fDrawOps.remove(this);
}
- SkDEBUGCODE(fCCPR->decrDrawOpCount_debugOnly());
}
GrDrawOp::RequiresDstTexture GrCCDrawPathsOp::finalize(const GrCaps& caps,
const GrAppliedClip* clip,
GrPixelConfigIsClamped dstIsClamped) {
- SkASSERT(!fCCPR->isFlushing_debugOnly());
// There should only be one single path draw in this Op right now.
SkASSERT(1 == fNumDraws);
GrProcessorSet::Analysis analysis =
@@ -60,11 +55,9 @@
bool GrCCDrawPathsOp::onCombineIfPossible(GrOp* op, const GrCaps& caps) {
GrCCDrawPathsOp* that = op->cast<GrCCDrawPathsOp>();
- SkASSERT(fCCPR == that->fCCPR);
- SkASSERT(!fCCPR->isFlushing_debugOnly());
- SkASSERT(fOwningRTPendingPaths);
+ SkASSERT(fOwningPerOpListPaths);
SkASSERT(fNumDraws);
- SkASSERT(!that->fOwningRTPendingPaths || that->fOwningRTPendingPaths == fOwningRTPendingPaths);
+ SkASSERT(!that->fOwningPerOpListPaths || that->fOwningPerOpListPaths == fOwningPerOpListPaths);
SkASSERT(that->fNumDraws);
if (this->getFillType() != that->getFillType() || fSRGBFlags != that->fSRGBFlags ||
@@ -73,7 +66,7 @@
return false;
}
- fDraws.append(std::move(that->fDraws), &fOwningRTPendingPaths->fAllocator);
+ fDraws.append(std::move(that->fDraws), &fOwningPerOpListPaths->fAllocator);
this->joinBounds(*that);
SkDEBUGCODE(fNumDraws += that->fNumDraws);
@@ -81,11 +74,11 @@
return true;
}
-void GrCCDrawPathsOp::wasRecorded(GrCCRTPendingPaths* owningRTPendingPaths) {
+void GrCCDrawPathsOp::wasRecorded(GrCCPerOpListPaths* owningPerOpListPaths) {
SkASSERT(1 == fNumDraws);
- SkASSERT(!fOwningRTPendingPaths);
- owningRTPendingPaths->fDrawOps.addToTail(this);
- fOwningRTPendingPaths = owningRTPendingPaths;
+ SkASSERT(!fOwningPerOpListPaths);
+ owningPerOpListPaths->fDrawOps.addToTail(this);
+ fOwningPerOpListPaths = owningPerOpListPaths;
}
int GrCCDrawPathsOp::countPaths(GrCCPathParser::PathStats* stats) const {
@@ -135,9 +128,9 @@
}
void GrCCDrawPathsOp::onExecute(GrOpFlushState* flushState) {
- SkASSERT(fOwningRTPendingPaths);
+ SkASSERT(fOwningPerOpListPaths);
- const GrCCPerFlushResources* resources = fCCPR->getPerFlushResources();
+ const GrCCPerFlushResources* resources = fOwningPerOpListPaths->fFlushResources.get();
if (!resources) {
return; // Setup failed.
}
diff --git a/src/gpu/ccpr/GrCCDrawPathsOp.h b/src/gpu/ccpr/GrCCDrawPathsOp.h
index b523442..5406d89 100644
--- a/src/gpu/ccpr/GrCCDrawPathsOp.h
+++ b/src/gpu/ccpr/GrCCDrawPathsOp.h
@@ -16,8 +16,7 @@
class GrCCAtlas;
class GrCCPerFlushResources;
-struct GrCCRTPendingPaths;
-class GrCoverageCountingPathRenderer;
+struct GrCCPerOpListPaths;
/**
* This is the Op that draws paths to the actual canvas, using atlases generated by CCPR.
@@ -27,8 +26,8 @@
DEFINE_OP_CLASS_ID
SK_DECLARE_INTERNAL_LLIST_INTERFACE(GrCCDrawPathsOp);
- GrCCDrawPathsOp(GrCoverageCountingPathRenderer*, GrPaint&&, const SkIRect& clipIBounds,
- const SkMatrix&, const SkPath&, const SkRect& devBounds);
+ GrCCDrawPathsOp(GrPaint&&, const SkIRect& clipIBounds, const SkMatrix&, const SkPath&,
+ const SkRect& devBounds);
~GrCCDrawPathsOp() override;
const char* name() const override { return "GrCCDrawOp"; }
@@ -41,7 +40,7 @@
}
void onPrepare(GrOpFlushState*) override {}
- void wasRecorded(GrCCRTPendingPaths* owningRTPendingPaths);
+ void wasRecorded(GrCCPerOpListPaths* owningPerOpListPaths);
int countPaths(GrCCPathParser::PathStats*) const;
void setupResources(GrCCPerFlushResources*, GrOnFlushResourceProvider*);
SkDEBUGCODE(int numSkippedInstances_debugOnly() const { return fNumSkippedInstances; })
@@ -66,7 +65,6 @@
fAtlasBatches.push_back() = {atlas, endInstanceIdx};
}
- GrCoverageCountingPathRenderer* const fCCPR;
const uint32_t fSRGBFlags;
const SkMatrix fViewMatrixIfUsingLocalCoords;
@@ -82,7 +80,7 @@
SkDEBUGCODE(int fNumDraws = 1);
GrProcessorSet fProcessors;
- GrCCRTPendingPaths* fOwningRTPendingPaths = nullptr;
+ GrCCPerOpListPaths* fOwningPerOpListPaths = nullptr;
int fBaseInstance;
SkSTArray<1, AtlasBatch, true> fAtlasBatches;
diff --git a/src/gpu/ccpr/GrCCPerFlushResources.h b/src/gpu/ccpr/GrCCPerFlushResources.h
index 7585a7a..d1a6d8f 100644
--- a/src/gpu/ccpr/GrCCPerFlushResources.h
+++ b/src/gpu/ccpr/GrCCPerFlushResources.h
@@ -9,14 +9,17 @@
#define GrCCPerFlushResources_DEFINED
#include "GrAllocator.h"
+#include "GrNonAtomicRef.h"
#include "ccpr/GrCCAtlas.h"
#include "ccpr/GrCCPathParser.h"
#include "ccpr/GrCCPathProcessor.h"
/**
- * This class wraps all the GPU resources that CCPR builds at flush time.
+ * This class wraps all the GPU resources that CCPR builds at flush time. It is allocated in CCPR's
+ * preFlush() method, and referenced by all the GrCCPerOpListPaths objects that are being flushed.
+ * It is deleted in postFlush() once all the flushing GrCCPerOpListPaths objects are deleted.
*/
-class GrCCPerFlushResources {
+class GrCCPerFlushResources : public GrNonAtomicRef<GrCCPerFlushResources> {
public:
GrCCPerFlushResources(GrOnFlushResourceProvider*, int numPathDraws, int numClipPaths,
const GrCCPathParser::PathStats&);
diff --git a/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp b/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp
index 0c1b493..ee0dbde 100644
--- a/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp
+++ b/src/gpu/ccpr/GrCoverageCountingPathRenderer.cpp
@@ -13,8 +13,8 @@
#include "SkMakeUnique.h"
#include "SkPathOps.h"
#include "ccpr/GrCCClipProcessor.h"
+#include "ccpr/GrCCDrawPathsOp.h"
#include "ccpr/GrCCPathParser.h"
-#include "ccpr/GrCCPerFlushResources.h"
using PathInstance = GrCCPathProcessor::Instance;
@@ -47,14 +47,22 @@
return sk_sp<GrCoverageCountingPathRenderer>(ccpr);
}
-GrCoverageCountingPathRenderer::GrCoverageCountingPathRenderer(bool drawCachablePaths)
- : fDrawCachablePaths(drawCachablePaths) {
+GrCCPerOpListPaths* GrCoverageCountingPathRenderer::lookupPendingPaths(uint32_t opListID) {
+ auto it = fPendingPaths.find(opListID);
+ if (fPendingPaths.end() == it) {
+ auto paths = skstd::make_unique<GrCCPerOpListPaths>();
+ it = fPendingPaths.insert(std::make_pair(opListID, std::move(paths))).first;
+ }
+ return it->second.get();
}
-GrCoverageCountingPathRenderer::~GrCoverageCountingPathRenderer() {
- // Ensure no Ops exist that could have a dangling pointer back into this class.
- SkASSERT(fRTPendingPathsMap.empty());
- SkASSERT(0 == fNumOutstandingDrawOps);
+void GrCoverageCountingPathRenderer::adoptAndRecordOp(GrCCDrawPathsOp* op,
+ const DrawPathArgs& args) {
+ GrRenderTargetContext* rtc = args.fRenderTargetContext;
+ if (uint32_t opListID = rtc->addDrawOp(*args.fClip, std::unique_ptr<GrDrawOp>(op))) {
+ // If the Op wasn't dropped or combined, give it a pointer to its owning GrCCPerOpListPaths.
+ op->wasRecorded(this->lookupPendingPaths(opListID));
+ }
}
GrPathRenderer::CanDrawPath GrCoverageCountingPathRenderer::onCanDrawPath(
@@ -112,25 +120,17 @@
SkPath croppedPath;
path.transform(*args.fViewMatrix, &croppedPath);
crop_path(croppedPath, clipIBounds, &croppedPath);
- this->adoptAndRecordOp(new GrCCDrawPathsOp(this, std::move(args.fPaint), clipIBounds,
+ this->adoptAndRecordOp(new GrCCDrawPathsOp(std::move(args.fPaint), clipIBounds,
SkMatrix::I(), croppedPath,
croppedPath.getBounds()), args);
return true;
}
- this->adoptAndRecordOp(new GrCCDrawPathsOp(this, std::move(args.fPaint), clipIBounds,
+ this->adoptAndRecordOp(new GrCCDrawPathsOp(std::move(args.fPaint), clipIBounds,
*args.fViewMatrix, path, devBounds), args);
return true;
}
-void GrCoverageCountingPathRenderer::adoptAndRecordOp(GrCCDrawPathsOp* op,
- const DrawPathArgs& args) {
- GrRenderTargetContext* rtc = args.fRenderTargetContext;
- if (uint32_t opListID = rtc->addDrawOp(*args.fClip, std::unique_ptr<GrDrawOp>(op))) {
- op->wasRecorded(&fRTPendingPathsMap[opListID]);
- }
-}
-
std::unique_ptr<GrFragmentProcessor> GrCoverageCountingPathRenderer::makeClipProcessor(
GrProxyProvider* proxyProvider,
uint32_t opListID, const SkPath& deviceSpacePath, const SkIRect& accessRect,
@@ -140,7 +140,7 @@
SkASSERT(!fFlushing);
GrCCClipPath& clipPath =
- fRTPendingPathsMap[opListID].fClipPaths[deviceSpacePath.getGenerationID()];
+ this->lookupPendingPaths(opListID)->fClipPaths[deviceSpacePath.getGenerationID()];
if (!clipPath.isInitialized()) {
// This ClipPath was just created during lookup. Initialize it.
const SkRect& pathDevBounds = deviceSpacePath.getBounds();
@@ -166,56 +166,55 @@
const uint32_t* opListIDs, int numOpListIDs,
SkTArray<sk_sp<GrRenderTargetContext>>* atlasDraws) {
SkASSERT(!fFlushing);
- SkASSERT(!fPerFlushResources);
+ SkASSERT(fFlushingPaths.empty());
SkDEBUGCODE(fFlushing = true);
- if (fRTPendingPathsMap.empty()) {
+ if (fPendingPaths.empty()) {
return; // Nothing to draw.
}
- // Count up the paths about to be flushed so we can preallocate buffers.
+ // Move the per-opList paths that are about to be flushed from fPendingPaths to fFlushingPaths,
+ // and count up the paths about to be flushed so we can preallocate buffers.
int numPathDraws = 0;
int numClipPaths = 0;
GrCCPathParser::PathStats flushingPathStats;
- fFlushingRTPathIters.reserve(numOpListIDs);
+ fFlushingPaths.reserve(numOpListIDs);
for (int i = 0; i < numOpListIDs; ++i) {
- auto iter = fRTPendingPathsMap.find(opListIDs[i]);
- if (fRTPendingPathsMap.end() == iter) {
- continue;
+ auto iter = fPendingPaths.find(opListIDs[i]);
+ if (fPendingPaths.end() == iter) {
+ continue; // No paths on this opList.
}
- const GrCCRTPendingPaths& rtPendingPaths = iter->second;
- for (const GrCCDrawPathsOp* op : rtPendingPaths.fDrawOps) {
+ fFlushingPaths.push_back(std::move(iter->second)).get();
+ fPendingPaths.erase(iter);
+
+ for (const GrCCDrawPathsOp* op : fFlushingPaths.back()->fDrawOps) {
numPathDraws += op->countPaths(&flushingPathStats);
}
- for (const auto& clipsIter : rtPendingPaths.fClipPaths) {
+ for (const auto& clipsIter : fFlushingPaths.back()->fClipPaths) {
flushingPathStats.statPath(clipsIter.second.deviceSpacePath());
}
- numClipPaths += rtPendingPaths.fClipPaths.size();
-
- fFlushingRTPathIters.push_back(std::move(iter));
+ numClipPaths += fFlushingPaths.back()->fClipPaths.size();
}
if (0 == numPathDraws + numClipPaths) {
return; // Nothing to draw.
}
- auto resources = skstd::make_unique<GrCCPerFlushResources>(onFlushRP, numPathDraws,
- numClipPaths, flushingPathStats);
+ auto resources = sk_make_sp<GrCCPerFlushResources>(onFlushRP, numPathDraws, numClipPaths,
+ flushingPathStats);
if (!resources->isMapped()) {
return; // Some allocation failed.
}
// Layout atlas(es) and parse paths.
SkDEBUGCODE(int numSkippedPaths = 0);
- for (const auto& iter : fFlushingRTPathIters) {
- GrCCRTPendingPaths* rtPendingPaths = &iter->second;
-
- for (GrCCDrawPathsOp* op : rtPendingPaths->fDrawOps) {
+ for (const auto& flushingPaths : fFlushingPaths) {
+ for (GrCCDrawPathsOp* op : flushingPaths->fDrawOps) {
op->setupResources(resources.get(), onFlushRP);
SkDEBUGCODE(numSkippedPaths += op->numSkippedInstances_debugOnly());
}
- for (auto& clipsIter : rtPendingPaths->fClipPaths) {
+ for (auto& clipsIter : flushingPaths->fClipPaths) {
clipsIter.second.placePathInAtlas(resources.get(), onFlushRP);
}
}
@@ -226,17 +225,16 @@
return;
}
- fPerFlushResources = std::move(resources);
+ // Commit flushing paths to the resources once they are successfully completed.
+ for (auto& flushingPaths : fFlushingPaths) {
+ flushingPaths->fFlushResources = resources;
+ }
}
void GrCoverageCountingPathRenderer::postFlush(GrDeferredUploadToken, const uint32_t* opListIDs,
int numOpListIDs) {
SkASSERT(fFlushing);
- fPerFlushResources.reset();
// We wait to erase these until after flush, once Ops and FPs are done accessing their data.
- for (const auto& iter : fFlushingRTPathIters) {
- fRTPendingPathsMap.erase(iter);
- }
- fFlushingRTPathIters.reset();
+ fFlushingPaths.reset();
SkDEBUGCODE(fFlushing = false);
}
diff --git a/src/gpu/ccpr/GrCoverageCountingPathRenderer.h b/src/gpu/ccpr/GrCoverageCountingPathRenderer.h
index ca52fed..dd0b190 100644
--- a/src/gpu/ccpr/GrCoverageCountingPathRenderer.h
+++ b/src/gpu/ccpr/GrCoverageCountingPathRenderer.h
@@ -13,16 +13,16 @@
#include "SkArenaAlloc.h"
#include "SkTInternalLList.h"
#include "ccpr/GrCCClipPath.h"
-#include "ccpr/GrCCDrawPathsOp.h"
+#include "ccpr/GrCCPerFlushResources.h"
#include <map>
-class GrCCPerFlushResources;
+class GrCCDrawPathsOp;
/**
- * Tracks all the paths in a single render target that will be drawn at next flush.
+ * Tracks all the paths in a given opList that will be drawn when it flushes.
*/
-struct GrCCRTPendingPaths {
- ~GrCCRTPendingPaths() {
+struct GrCCPerOpListPaths {
+ ~GrCCPerOpListPaths() {
// Ensure there are no surviving DrawPathsOps with a dangling pointer into this class.
if (!fDrawOps.isEmpty()) {
SK_ABORT("GrCCDrawPathsOp(s) not deleted during flush");
@@ -36,6 +36,7 @@
SkTInternalLList<GrCCDrawPathsOp> fDrawOps;
std::map<uint32_t, GrCCClipPath> fClipPaths;
SkSTArenaAlloc<10 * 1024> fAllocator{10 * 1024 * 2};
+ sk_sp<const GrCCPerFlushResources> fFlushResources;
};
/**
@@ -50,7 +51,11 @@
static bool IsSupported(const GrCaps&);
static sk_sp<GrCoverageCountingPathRenderer> CreateIfSupported(const GrCaps&,
bool drawCachablePaths);
- ~GrCoverageCountingPathRenderer() override;
+ ~GrCoverageCountingPathRenderer() override {
+ // Ensure callers are actually flushing paths they record, not causing us to leak memory.
+ SkASSERT(fPendingPaths.empty());
+ SkASSERT(!fFlushing);
+ }
// GrPathRenderer overrides.
StencilSupport onGetStencilSupport(const GrShape&) const override {
@@ -69,31 +74,24 @@
SkTArray<sk_sp<GrRenderTargetContext>>* atlasDraws) override;
void postFlush(GrDeferredUploadToken, const uint32_t* opListIDs, int numOpListIDs) override;
-#ifdef SK_DEBUG
- bool isFlushing_debugOnly() const { return fFlushing; }
- void incrDrawOpCount_debugOnly() { ++fNumOutstandingDrawOps; }
- void decrDrawOpCount_debugOnly() { --fNumOutstandingDrawOps; }
-#endif
-
private:
- GrCoverageCountingPathRenderer(bool drawCachablePaths);
+ GrCoverageCountingPathRenderer(bool drawCachablePaths)
+ : fDrawCachablePaths(drawCachablePaths) {}
+ GrCCPerOpListPaths* lookupPendingPaths(uint32_t opListID);
void adoptAndRecordOp(GrCCDrawPathsOp*, const DrawPathArgs&);
- const GrCCPerFlushResources* getPerFlushResources() const {
- SkASSERT(fFlushing);
- return fPerFlushResources.get();
- }
+ // fPendingPaths holds the GrCCPerOpListPaths objects that have already been created, but not
+ // flushed, and those that are still being created. All GrCCPerOpListPaths objects will first
+ // reside in fPendingPaths, then be moved to fFlushingPaths during preFlush().
+ std::map<uint32_t, std::unique_ptr<GrCCPerOpListPaths>> fPendingPaths;
- std::map<uint32_t, GrCCRTPendingPaths> fRTPendingPathsMap;
- SkSTArray<4, std::map<uint32_t, GrCCRTPendingPaths>::iterator> fFlushingRTPathIters;
- std::unique_ptr<GrCCPerFlushResources> fPerFlushResources;
+ // fFlushingPaths holds the GrCCPerOpListPaths objects that are currently being flushed.
+ // (It will only contain elements when fFlushing is true.)
+ SkSTArray<4, std::unique_ptr<GrCCPerOpListPaths>> fFlushingPaths;
SkDEBUGCODE(bool fFlushing = false);
- SkDEBUGCODE(int fNumOutstandingDrawOps = 0);
const bool fDrawCachablePaths;
-
- friend void GrCCDrawPathsOp::onExecute(GrOpFlushState*); // For getPerFlushResources.
};
#endif