Reland "Notify RTC when OpsTask is closed so it can drop ownership"

This reverts commit 777f239a65.

Assigns observer in GrRenderTargetContext constructor.

Change-Id: Ie1d4a425eb959f7b822b5fc4988b2bb6ca2ed6a5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/284733
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

src/gpu/GrOpsTask.cpp

@@ -10,6 +10,7 @@
 #include "include/private/GrRecordingContext.h"
 #include "src/core/SkExchange.h"
 #include "src/core/SkRectPriv.h"
+#include "src/core/SkScopeExit.h"
 #include "src/core/SkTraceEvent.h"
 #include "src/gpu/GrAuditTrail.h"
 #include "src/gpu/GrCaps.h"
@@ -39,6 +40,10 @@ using DstProxyView = GrXferProcessor::DstProxyView;
 
 ////////////////////////////////////////////////////////////////////////////////
 
+GrOpsTaskClosedObserver::~GrOpsTaskClosedObserver() = default;
+
+////////////////////////////////////////////////////////////////////////////////
+
 static inline bool can_reorder(const SkRect& a, const SkRect& b) { return !GrRectsOverlap(a, b); }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -359,7 +364,6 @@ GrOpsTask::GrOpsTask(GrRecordingContext::Arenas arenas,
         : GrRenderTask(std::move(view))
         , fArenas(arenas)
         , fAuditTrail(auditTrail)
-        , fLastClipStackGenID(SK_InvalidUniqueID)
         SkDEBUGCODE(, fNumClips(0)) {
     fTargetView.proxy()->setLastRenderTask(this);
 }
@@ -874,6 +878,12 @@ void GrOpsTask::forwardCombine(const GrCaps& caps) {
 GrRenderTask::ExpectedOutcome GrOpsTask::onMakeClosed(
         const GrCaps& caps, SkIRect* targetUpdateBounds) {
     this->forwardCombine(caps);
+    SkScopeExit triggerObserver([&] {
+        if (fClosedObserver) {
+            fClosedObserver->wasClosed(*this);
+            fClosedObserver = nullptr;
+        }
+    });
     if (!this->isNoOp()) {
         GrSurfaceProxy* proxy = fTargetView.proxy();
         SkRect clippedContentBounds = proxy->getBoundsRect();

src/gpu/GrOpsTask.h

@@ -33,6 +33,13 @@ class GrClearOp;
 class GrGpuBuffer;
 class GrRenderTargetProxy;
 
+/** Observer is notified when a GrOpsTask is closed. */
+class GrOpsTaskClosedObserver {
+public:
+    virtual ~GrOpsTaskClosedObserver() = 0;
+    virtual void wasClosed(const GrOpsTask& task) = 0;
+};
+
 class GrOpsTask : public GrRenderTask {
 private:
     using DstProxyView = GrXferProcessor::DstProxyView;
@@ -45,6 +52,9 @@ public:
 
     GrOpsTask* asOpsTask() override { return this; }
 
+    /** Each OpsTask supports a single observer at a time. */
+    void setClosedObserver(GrOpsTaskClosedObserver* observer) { fClosedObserver = observer; }
+
     bool isEmpty() const { return fOpChains.empty(); }
 
     /**
@@ -285,18 +295,19 @@ private:
     // into the owning DDL.
     GrRecordingContext::Arenas fArenas;
     GrAuditTrail*              fAuditTrail;
+    GrOpsTaskClosedObserver*   fClosedObserver = nullptr;
 
     GrLoadOp fColorLoadOp = GrLoadOp::kLoad;
     SkPMColor4f fLoadClearColor = SK_PMColor4fTRANSPARENT;
     StencilContent fInitialStencilContent = StencilContent::kDontCare;
     bool fMustPreserveStencil = false;
 
-    uint32_t fLastClipStackGenID;
+    uint32_t fLastClipStackGenID = SK_InvalidUniqueID;
     SkIRect fLastDevClipBounds;
     int fLastClipNumAnalyticFPs;
 
     // We must track if we have a wait op so that we don't delete the op when we have a full clear.
-    bool fHasWaitOp = false;;
+    bool fHasWaitOp = false;
 
     // For ops/opsTask we have mean: 5 stdDev: 28
     SkSTArray<25, OpChain, true> fOpChains;

src/gpu/GrRenderTargetContext.cpp

@@ -381,6 +381,9 @@ GrRenderTargetContext::GrRenderTargetContext(GrRecordingContext* context,
         , fOpsTask(sk_ref_sp(this->asSurfaceProxy()->getLastOpsTask()))
         , fSurfaceProps(SkSurfacePropsCopyOrDefault(surfaceProps))
         , fManagedOpsTask(managedOpsTask) {
+    if (fOpsTask) {
+        fOpsTask->setClosedObserver(this);
+    }
     SkASSERT(this->asSurfaceProxy() == fWriteView.proxy());
     SkASSERT(this->origin() == fWriteView.origin());
 
@@ -398,6 +401,9 @@ void GrRenderTargetContext::onValidate() const {
 
 GrRenderTargetContext::~GrRenderTargetContext() {
     ASSERT_SINGLE_OWNER
+    if (fOpsTask) {
+        fOpsTask->setClosedObserver(nullptr);
+    }
 }
 
 inline GrAAType GrRenderTargetContext::chooseAAType(GrAA aa) {
@@ -423,7 +429,7 @@ GrOpsTask* GrRenderTargetContext::getOpsTask() {
     ASSERT_SINGLE_OWNER
     SkDEBUGCODE(this->validate();)
 
-    if (!fOpsTask || fOpsTask->isClosed()) {
+    if (!fOpsTask) {
         sk_sp<GrOpsTask> newOpsTask =
                 this->drawingManager()->newOpsTask(this->writeSurfaceView(), fManagedOpsTask);
         if (fOpsTask && fNumStencilSamples > 0) {
@@ -434,9 +440,10 @@ GrOpsTask* GrRenderTargetContext::getOpsTask() {
             // values?
             newOpsTask->setInitialStencilContent(GrOpsTask::StencilContent::kPreserved);
         }
+        newOpsTask->setClosedObserver(this);
         fOpsTask = std::move(newOpsTask);
     }
-
+    SkASSERT(!fOpsTask->isClosed());
     return fOpsTask.get();
 }
 
@@ -2654,3 +2661,8 @@ bool GrRenderTargetContext::blitTexture(GrSurfaceProxyView view, const SkIRect&
             SkRect::Make(clippedSrcRect));
     return true;
 }
+
+void GrRenderTargetContext::wasClosed(const GrOpsTask& task) {
+    SkASSERT(&task == fOpsTask.get());
+    fOpsTask.reset();
+}

src/gpu/GrRenderTargetContext.h

@@ -57,7 +57,7 @@ class SkVertices;
 /**
  * A helper object to orchestrate commands (draws, etc...) for GrSurfaces that are GrRenderTargets.
  */
-class GrRenderTargetContext : public GrSurfaceContext {
+class GrRenderTargetContext : public GrSurfaceContext, public GrOpsTaskClosedObserver {
 public:
     static std::unique_ptr<GrRenderTargetContext> Make(
             GrRecordingContext*, GrColorType, sk_sp<SkColorSpace>, sk_sp<GrSurfaceProxy>,
@@ -580,6 +580,8 @@ public:
 
     GrTextTarget* textTarget() { return fTextTarget.get(); }
 
+    void wasClosed(const GrOpsTask& task) override;
+
 #if GR_TEST_UTILS
     bool testingOnly_IsInstantiated() const { return this->asSurfaceProxy()->isInstantiated(); }
     void testingOnly_SetPreserveOpsOnFullClear() { fPreserveOpsOnFullClear_TestingOnly = true; }

tests/GrMipMappedTest.cpp

@@ -420,6 +420,7 @@ DEF_GPUTEST(GrManyDependentsMipMappedTest, reporter, /* options */) {
         // Draw the dirty mipmap texture into a render target.
         auto rtc1 = draw_mipmap_into_new_render_target(context.get(), proxyProvider, colorType,
                                                        alphaType, mipmapView, Filter::kMipMap);
+        auto rtc1Task = sk_ref_sp(rtc1->testingOnly_PeekLastOpsTask());
 
         // Mipmaps should have gotten marked dirty during makeClosed, then marked clean again as
         // soon as a GrTextureResolveRenderTask was inserted. The way we know they were resolved is
@@ -434,6 +435,7 @@ DEF_GPUTEST(GrManyDependentsMipMappedTest, reporter, /* options */) {
         // Draw the now-clean mipmap texture into a second target.
         auto rtc2 = draw_mipmap_into_new_render_target(context.get(), proxyProvider, colorType,
                                                        alphaType, mipmapView, Filter::kMipMap);
+        auto rtc2Task = sk_ref_sp(rtc2->testingOnly_PeekLastOpsTask());
 
         // Make sure the mipmap texture still has the same regen task.
         REPORTER_ASSERT(reporter, mipmapProxy->getLastRenderTask() == initialMipmapRegenTask);
@@ -443,17 +445,16 @@ DEF_GPUTEST(GrManyDependentsMipMappedTest, reporter, /* options */) {
         context->flush();
 
         // Mip regen tasks don't get added as dependencies until makeClosed().
-        REPORTER_ASSERT(reporter,
-                rtc1->testingOnly_PeekLastOpsTask()->dependsOn(initialMipmapRegenTask));
-        REPORTER_ASSERT(reporter,
-                rtc2->testingOnly_PeekLastOpsTask()->dependsOn(initialMipmapRegenTask));
+        REPORTER_ASSERT(reporter, rtc1Task->dependsOn(initialMipmapRegenTask));
+        REPORTER_ASSERT(reporter, rtc2Task->dependsOn(initialMipmapRegenTask));
 
         // Render something to dirty the mips.
         mipmapRTC->clear(nullptr, {.1f,.2f,.3f,.4f}, CanClearFullscreen::kYes);
-        REPORTER_ASSERT(reporter, mipmapProxy->getLastRenderTask());
+        auto mipmapRTCTask = sk_ref_sp(mipmapRTC->testingOnly_PeekLastOpsTask());
+        REPORTER_ASSERT(reporter, mipmapRTCTask);
+
         // mipmapProxy's last render task should now just be the opsTask containing the clear.
-        REPORTER_ASSERT(reporter,
-                mipmapRTC->testingOnly_PeekLastOpsTask() == mipmapProxy->getLastRenderTask());
+        REPORTER_ASSERT(reporter, mipmapRTCTask.get() == mipmapProxy->getLastRenderTask());
 
         // Mipmaps don't get marked dirty until makeClosed().
         REPORTER_ASSERT(reporter, !mipmapProxy->mipMapsAreDirty());
@@ -468,24 +469,23 @@ DEF_GPUTEST(GrManyDependentsMipMappedTest, reporter, /* options */) {
         REPORTER_ASSERT(reporter, mipmapProxy->mipMapsAreDirty());
 
         // Since mips weren't regenerated, the last render task shouldn't have changed.
-        REPORTER_ASSERT(reporter,
-                mipmapRTC->testingOnly_PeekLastOpsTask() == mipmapProxy->getLastRenderTask());
+        REPORTER_ASSERT(reporter, mipmapRTCTask.get() == mipmapProxy->getLastRenderTask());
 
         // Draw the stil-dirty mipmap texture into a second target with mipmap filtering.
         rtc2 = draw_mipmap_into_new_render_target(context.get(), proxyProvider, colorType,
                                                   alphaType, std::move(mipmapView),
                                                   Filter::kMipMap);
+        rtc2Task = sk_ref_sp(rtc2->testingOnly_PeekLastOpsTask());
 
         // Make sure the mipmap texture now has a new last render task that regenerates the mips,
         // and that the mipmaps are now clean.
         auto mipRegenTask2 = mipmapProxy->getLastRenderTask();
         REPORTER_ASSERT(reporter, mipRegenTask2);
-        REPORTER_ASSERT(reporter,
-                mipmapRTC->testingOnly_PeekLastOpsTask() != mipRegenTask2);
+        REPORTER_ASSERT(reporter, mipmapRTCTask.get() != mipRegenTask2);
         SkASSERT(!mipmapProxy->mipMapsAreDirty());
 
         // Mip regen tasks don't get added as dependencies until makeClosed().
         context->flush();
-        REPORTER_ASSERT(reporter, rtc2->testingOnly_PeekLastOpsTask()->dependsOn(mipRegenTask2));
+        REPORTER_ASSERT(reporter, rtc2Task->dependsOn(mipRegenTask2));
     }
 }