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>

5 files changed, 87 insertions, 97 deletions

diff --git a/src/gpu/ccpr/GrCCDrawPathsOp.cpp b/src/gpu/ccpr/GrCCDrawPathsOp.cpp
index 96c1134536..b3fd711a69 100644
--- a/src/gpu/ccpr/GrCCDrawPathsOp.cpp
+++ b/src/gpu/ccpr/GrCCDrawPathsOp.cpp
@@ -22,16 +22,13 @@ static bool has_coord_transforms(const GrPaint& paint) {
     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(GrCoverageCountingPathRenderer* ccpr, GrPaint&&
 }
 
 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 @@ GrDrawOp::RequiresDstTexture GrCCDrawPathsOp::finalize(const GrCaps& caps,
 
 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 @@ bool GrCCDrawPathsOp::onCombineIfPossible(GrOp* op, const GrCaps& caps) {
         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 @@ bool GrCCDrawPathsOp::onCombineIfPossible(GrOp* op, const GrCaps& caps) {
     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::setupResources(GrCCPerFlushResources* resources,
 }
 
 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 b523442393..5406d893f7 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 @@ public:
     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 @@ public:
     }
     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 @@ private:
         fAtlasBatches.push_back() = {atlas, endInstanceIdx};
     }
 
-    GrCoverageCountingPathRenderer* const fCCPR;
     const uint32_t fSRGBFlags;
     const SkMatrix fViewMatrixIfUsingLocalCoords;
 
@@ -82,7 +80,7 @@ private:
     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 7585a7a286..d1a6d8ff21 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 0c1b493471..ee0dbde799 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 @@ sk_sp<GrCoverageCountingPathRenderer> GrCoverageCountingPathRenderer::CreateIfSu
     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 @@ bool GrCoverageCountingPathRenderer::onDrawPath(const DrawPathArgs& args) {
         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 @@ std::unique_ptr<GrFragmentProcessor> GrCoverageCountingPathRenderer::makeClipPro
     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 @@ void GrCoverageCountingPathRenderer::preFlush(GrOnFlushResourceProvider* onFlush
                                               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 @@ void GrCoverageCountingPathRenderer::preFlush(GrOnFlushResourceProvider* onFlush
         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 ca52fedada..dd0b190170 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 @@ struct GrCCRTPendingPaths {
     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 @@ public:
     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 @@ public:
                   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