Revert "Update clearOp for split-OpList world (take 2)"

This reverts commit 3fdd0bf2d90b1b82c1ac3aa982bdca600de7f4a8.

Reason for revert: Chrome roll dryrun?

Original change's description:
> Update clearOp for split-OpList world (take 2)
> 
> It would reduce a lot of noise if the GrRenderTargetOpList kept a pointer to the GrCaps but, for now, I'm trying to shrink the GrRTOpList, not expand it.
> 
> Reland of: https://skia-review.googlesource.com/c/17323/ (Update clearOp for split-OpList world)
> 
> Change-Id: I97f3fb81e6258d430e7f7cf1ea8bd51a392f9f47
> Reviewed-on: https://skia-review.googlesource.com/17830
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>
> 

TBR=bsalomon@google.com,robertphillips@google.com
No-Presubmit: true
No-Tree-Checks: true
No-Try: true

Change-Id: Ib70285bb484a1ed8babaa519c74bfdf619313806
Reviewed-on: https://skia-review.googlesource.com/18024
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>

4 files changed, 107 insertions, 63 deletions

diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index ccecd66a55..fa1e0b3d00 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -200,7 +200,7 @@ void GrRenderTargetContext::discard() {
         if (!op) {
             return;
         }
-        this->getOpList()->addOp(std::move(op), *this->caps());
+        this->getOpList()->addOp(std::move(op), this);
     }
 }
 
@@ -259,11 +259,11 @@ void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect, const GrColor
         // This path doesn't handle coalescing of full screen clears b.c. it
         // has to clear the entire render target - not just the content area.
         // It could be done but will take more finagling.
-        std::unique_ptr<GrOp> op(GrClearOp::Make(rtRect, color, !clearRect));
+        std::unique_ptr<GrOp> op(GrClearOp::Make(rtRect, color, fRenderTargetContext, !clearRect));
         if (!op) {
             return;
         }
-        fRenderTargetContext->getOpList()->addOp(std::move(op), *fRenderTargetContext->caps());
+        fRenderTargetContext->getOpList()->addOp(std::move(op), fRenderTargetContext);
     }
 }
 
@@ -307,13 +307,13 @@ void GrRenderTargetContext::internalClear(const GrFixedClip& clip,
 
         this->drawRect(clip, std::move(paint), GrAA::kNo, SkMatrix::I(), SkRect::Make(clearRect));
     } else if (isFull) {
-        this->getOpList()->fullClear(*this->caps(), color);
+        this->getOpList()->fullClear(this, color);
     } else {
-        std::unique_ptr<GrOp> op(GrClearOp::Make(clip, color, this->asSurfaceProxy()));
+        std::unique_ptr<GrOp> op(GrClearOp::Make(clip, color, this));
         if (!op) {
             return;
         }
-        this->getOpList()->addOp(std::move(op), *this->caps());
+        this->getOpList()->addOp(std::move(op), this);
     }
 }
 
@@ -610,7 +610,7 @@ void GrRenderTargetContextPriv::clearStencilClip(const GrFixedClip& clip, bool i
     if (!op) {
         return;
     }
-    fRenderTargetContext->getOpList()->addOp(std::move(op), *fRenderTargetContext->caps());
+    fRenderTargetContext->getOpList()->addOp(std::move(op), fRenderTargetContext);
 }
 
 void GrRenderTargetContextPriv::stencilPath(const GrClip& clip,
@@ -668,7 +668,7 @@ void GrRenderTargetContextPriv::stencilPath(const GrClip& clip,
         return;
     }
     op->setClippedBounds(bounds);
-    fRenderTargetContext->getOpList()->addOp(std::move(op), *fRenderTargetContext->caps());
+    fRenderTargetContext->getOpList()->addOp(std::move(op), fRenderTargetContext);
 }
 
 void GrRenderTargetContextPriv::stencilRect(const GrClip& clip,
@@ -1759,14 +1759,14 @@ uint32_t GrRenderTargetContext::addDrawOp(const GrClip& clip, std::unique_ptr<Gr
         return SK_InvalidUniqueID;
     }
 
+    // This forces instantiation of the render target.
+    GrRenderTarget* rt = this->accessRenderTarget();
+    if (!rt) {
+        return SK_InvalidUniqueID;
+    }
+
     if (fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesStencil ||
         appliedClip.hasStencilClip()) {
-        // This forces instantiation of the render target.
-        GrRenderTarget* rt = this->accessRenderTarget();
-        if (!rt) {
-            return SK_InvalidUniqueID;
-        }
-
         if (!fContext->resourceProvider()->attachStencilAttachment(rt)) {
             SkDebugf("ERROR creating stencil attachment. Draw skipped.\n");
             return SK_InvalidUniqueID;
@@ -1781,8 +1781,7 @@ uint32_t GrRenderTargetContext::addDrawOp(const GrClip& clip, std::unique_ptr<Gr
     }
 
     op->setClippedBounds(bounds);
-    return this->getOpList()->addOp(std::move(op), *this->caps(),
-                                    std::move(appliedClip), dstTexture);
+    return this->getOpList()->addOp(std::move(op), this, std::move(appliedClip), dstTexture);
 }
 
 uint32_t GrRenderTargetContext::addLegacyMeshDrawOp(GrPipelineBuilder&& pipelineBuilder,
@@ -1844,7 +1843,7 @@ uint32_t GrRenderTargetContext::addLegacyMeshDrawOp(GrPipelineBuilder&& pipeline
     op->addDependenciesTo(fRenderTargetProxy.get());
 
     op->setClippedBounds(bounds);
-    return this->getOpList()->addOp(std::move(op), *this->caps());
+    return this->getOpList()->addOp(std::move(op), this);
 }
 
 bool GrRenderTargetContext::setupDstTexture(GrRenderTargetProxy* rtProxy, const GrClip& clip,
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index 7cdcdd9061..537f9e67fc 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -34,14 +34,6 @@ GrRenderTargetOpList::GrRenderTargetOpList(GrRenderTargetProxy* proxy, GrGpu* gp
     if (GrCaps::InstancedSupport::kNone != gpu->caps()->instancedSupport()) {
         fInstancedRendering.reset(gpu->createInstancedRendering());
     }
-
-    // MDB TODO: remove this! We are currently moving to having all the ops that target
-    // the RT as a dest (e.g., clear, etc.) rely on the opList's 'fTarget' pointer 
-    // for the IO Ref. This works well but until they are all swapped over (and none
-    // are pre-emptively instantiating proxies themselves) we need to instantiate
-    // here so that the GrSurfaces are created in an order that preserves the GrSurface
-    // re-use assumptions.
-    fTarget.get()->instantiate(gpu->getContext()->resourceProvider());
 }
 
 GrRenderTargetOpList::~GrRenderTargetOpList() {
@@ -166,6 +158,8 @@ bool GrRenderTargetOpList::executeOps(GrOpFlushState* flushState) {
 
 void GrRenderTargetOpList::reset() {
     fLastFullClearOp = nullptr;
+    fLastFullClearResourceID.makeInvalid();
+    fLastFullClearProxyID.makeInvalid();
     fLastClipStackGenID = SK_InvalidUniqueID;
     fRecordedOps.reset();
     if (fInstancedRendering) {
@@ -188,11 +182,24 @@ void GrRenderTargetOpList::freeGpuResources() {
     }
 }
 
-void GrRenderTargetOpList::fullClear(const GrCaps& caps, GrColor color) {
+void GrRenderTargetOpList::fullClear(GrRenderTargetContext* renderTargetContext, GrColor color) {
+    // MDB TODO: remove this. Right now we need the renderTargetContext for the
+    // accessRenderTarget call. This method should just take the renderTargetProxy.
+    GrRenderTarget* renderTarget = renderTargetContext->accessRenderTarget();
+    if (!renderTarget) {
+        return;
+    }
+
     // Currently this just inserts or updates the last clear op. However, once in MDB this can
     // remove all the previously recorded ops and change the load op to clear with supplied
     // color.
-    if (fLastFullClearOp) {
+    // TODO: this needs to be updated to use GrSurfaceProxy::UniqueID
+    // MDB TODO: re-enable once opLists are divided. This assertion fails when a rendering is
+    // aborted but the same RT is reused for the next draw. The clears really shouldn't be
+    // fused in that case.
+    //SkASSERT((fLastFullClearResourceID == renderTarget->uniqueID()) ==
+    //         (fLastFullClearProxyID == renderTargetContext->asRenderTargetProxy()->uniqueID()));
+    if (fLastFullClearResourceID == renderTarget->uniqueID()) {
         // As currently implemented, fLastFullClearOp should be the last op because we would
         // have cleared it when another op was recorded.
         SkASSERT(fRecordedOps.back().fOp.get() == fLastFullClearOp);
@@ -203,14 +210,16 @@ void GrRenderTargetOpList::fullClear(const GrCaps& caps, GrColor color) {
         fLastFullClearOp->setColor(color);
         return;
     }
-    std::unique_ptr<GrClearOp> op(GrClearOp::Make(GrFixedClip::Disabled(), color, fTarget.get()));
+    std::unique_ptr<GrClearOp> op(GrClearOp::Make(GrFixedClip::Disabled(), color,
+                                                  renderTargetContext));
     if (!op) {
         return;
     }
-
-    if (GrOp* clearOp = this->recordOp(std::move(op), caps)) {
+    if (GrOp* clearOp = this->recordOp(std::move(op), renderTargetContext)) {
         // This is either the clear op we just created or another one that it combined with.
         fLastFullClearOp = static_cast<GrClearOp*>(clearOp);
+        fLastFullClearResourceID = renderTarget->uniqueID();
+        fLastFullClearProxyID = renderTargetContext->asRenderTargetProxy()->uniqueID();
     }
 }
 
@@ -222,8 +231,6 @@ bool GrRenderTargetOpList::copySurface(GrResourceProvider* resourceProvider,
                                        GrSurfaceProxy* src,
                                        const SkIRect& srcRect,
                                        const SkIPoint& dstPoint) {
-    SkASSERT(dst->asRenderTargetProxy() == fTarget.get());
-
     std::unique_ptr<GrOp> op = GrCopySurfaceOp::Make(resourceProvider, dst->asSurfaceProxy(),
                                                      src, srcRect, dstPoint);
     if (!op) {
@@ -233,7 +240,7 @@ bool GrRenderTargetOpList::copySurface(GrResourceProvider* resourceProvider,
     this->addDependency(src);
 #endif
 
-    this->recordOp(std::move(op), *resourceProvider->caps());
+    this->recordOp(std::move(op), dst);
     return true;
 }
 
@@ -264,10 +271,11 @@ bool GrRenderTargetOpList::combineIfPossible(const RecordedOp& a, GrOp* b,
 }
 
 GrOp* GrRenderTargetOpList::recordOp(std::unique_ptr<GrOp> op,
-                                     const GrCaps& caps,
+                                     GrRenderTargetContext* renderTargetContext,
                                      GrAppliedClip* clip,
                                      const DstTexture* dstTexture) {
     SkASSERT(fTarget.get());
+    const GrCaps* caps = renderTargetContext->caps();
 
     // A closed GrOpList should never receive new/more ops
     SkASSERT(!this->isClosed());
@@ -276,7 +284,8 @@ GrOp* GrRenderTargetOpList::recordOp(std::unique_ptr<GrOp> op,
     // 1) check every op
     // 2) intersect with something
     // 3) find a 'blocker'
-    GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), fTarget.get()->uniqueID());
+    GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(),
+                          renderTargetContext->asRenderTargetProxy()->uniqueID());
     GrOP_INFO("opList: %d Recording (%s, opID: %u)\n"
               "\tBounds [L: %.2f, T: %.2f R: %.2f B: %.2f]\n",
                this->uniqueID(),
@@ -293,7 +302,7 @@ GrOp* GrRenderTargetOpList::recordOp(std::unique_ptr<GrOp> op,
         while (true) {
             const RecordedOp& candidate = fRecordedOps.fromBack(i);
 
-            if (this->combineIfPossible(candidate, op.get(), clip, dstTexture, caps)) {
+            if (this->combineIfPossible(candidate, op.get(), clip, dstTexture, *caps)) {
                 GrOP_INFO("\t\tBackward: Combining with (%s, opID: %u)\n", candidate.fOp->name(),
                           candidate.fOp->uniqueID());
                 GrOP_INFO("\t\t\tBackward: Combined op info:\n");
@@ -324,6 +333,8 @@ GrOp* GrRenderTargetOpList::recordOp(std::unique_ptr<GrOp> op,
     fRecordedOps.emplace_back(std::move(op), clip, dstTexture);
     fRecordedOps.back().fOp->wasRecorded(this);
     fLastFullClearOp = nullptr;
+    fLastFullClearResourceID.makeInvalid();
+    fLastFullClearProxyID.makeInvalid();
     return fRecordedOps.back().fOp.get();
 }
 
diff --git a/src/gpu/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h
index e9850698f3..7f407824db 100644
--- a/src/gpu/GrRenderTargetOpList.h
+++ b/src/gpu/GrRenderTargetOpList.h
@@ -65,18 +65,19 @@ public:
     void prepareOps(GrOpFlushState* flushState) override;
     bool executeOps(GrOpFlushState* flushState) override;
 
-    uint32_t addOp(std::unique_ptr<GrOp> op, const GrCaps& caps) {
-        this->recordOp(std::move(op), caps, nullptr, nullptr);
+    uint32_t addOp(std::unique_ptr<GrOp> op, GrRenderTargetContext* renderTargetContext) {
+        this->recordOp(std::move(op), renderTargetContext, nullptr, nullptr);
         return this->uniqueID();
     }
-    uint32_t addOp(std::unique_ptr<GrOp> op, const GrCaps& caps,
+    uint32_t addOp(std::unique_ptr<GrOp> op, GrRenderTargetContext* renderTargetContext,
                    GrAppliedClip&& clip, const DstTexture& dstTexture) {
-        this->recordOp(std::move(op), caps, clip.doesClip() ? &clip : nullptr, &dstTexture);
+        this->recordOp(std::move(op), renderTargetContext, clip.doesClip() ? &clip : nullptr,
+                       &dstTexture);
         return this->uniqueID();
     }
 
     /** Clears the entire render target */
-    void fullClear(const GrCaps& caps, GrColor color);
+    void fullClear(GrRenderTargetContext*, GrColor color);
 
     /**
      * Copies a pixel rectangle from one surface to another. This call may finalize
@@ -126,8 +127,8 @@ private:
 
     // If the input op is combined with an earlier op, this returns the combined op. Otherwise, it
     // returns the input op.
-    GrOp* recordOp(std::unique_ptr<GrOp>, const GrCaps& caps,
-                   GrAppliedClip* = nullptr, const DstTexture* = nullptr);
+    GrOp* recordOp(std::unique_ptr<GrOp>, GrRenderTargetContext*, GrAppliedClip* = nullptr,
+                   const DstTexture* = nullptr);
 
     void forwardCombine(const GrCaps&);
 
@@ -135,20 +136,22 @@ private:
     bool combineIfPossible(const RecordedOp& a, GrOp* b, const GrAppliedClip* bClip,
                            const DstTexture* bDstTexture, const GrCaps&);
 
-    GrClearOp*                     fLastFullClearOp = nullptr;
+    GrClearOp* fLastFullClearOp = nullptr;
+    GrGpuResource::UniqueID fLastFullClearResourceID = GrGpuResource::UniqueID::InvalidID();
+    GrSurfaceProxy::UniqueID fLastFullClearProxyID = GrSurfaceProxy::UniqueID::InvalidID();
 
     std::unique_ptr<gr_instanced::InstancedRendering> fInstancedRendering;
 
-    int32_t                        fLastClipStackGenID;
-    SkIRect                        fLastDevClipBounds;
+    int32_t fLastClipStackGenID;
+    SkIRect fLastDevClipBounds;
 
     // For ops/opList we have mean: 5 stdDev: 28
     SkSTArray<5, RecordedOp, true> fRecordedOps;
 
     // MDB TODO: 4096 for the first allocation of the clip space will be huge overkill.
     // Gather statistics to determine the correct size.
-    SkArenaAlloc                   fClipAllocator{4096};
-    SkDEBUGCODE(int                fNumClips;)
+    SkArenaAlloc fClipAllocator{4096};
+    SkDEBUGCODE(int          fNumClips;)
 
     typedef GrOpList INHERITED;
 };
diff --git a/src/gpu/ops/GrClearOp.h b/src/gpu/ops/GrClearOp.h
index 6d0cf28775..b18332fa21 100644
--- a/src/gpu/ops/GrClearOp.h
+++ b/src/gpu/ops/GrClearOp.h
@@ -9,30 +9,49 @@
 #define GrClearOp_DEFINED
 
 #include "GrFixedClip.h"
+#include "GrGpu.h"
 #include "GrGpuCommandBuffer.h"
 #include "GrOp.h"
 #include "GrOpFlushState.h"
+#include "GrRenderTarget.h"
+#include "GrRenderTargetContext.h"
 #include "GrResourceProvider.h"
 
 class GrClearOp final : public GrOp {
 public:
     DEFINE_OP_CLASS_ID
 
+    // MDB TODO: replace the renderTargetContext with just the renderTargetProxy.
+    // For now, we need the renderTargetContext for its accessRenderTarget powers.
     static std::unique_ptr<GrClearOp> Make(const GrFixedClip& clip, GrColor color,
-                                           GrSurfaceProxy* dstProxy) {
-        const SkIRect rect = SkIRect::MakeWH(dstProxy->width(), dstProxy->height());
-        if (clip.scissorEnabled() && !SkIRect::Intersects(clip.scissorRect(), rect)) {
+                                           GrRenderTargetContext* rtc) {
+        const SkIRect rtRect = SkIRect::MakeWH(rtc->width(), rtc->height());
+        if (clip.scissorEnabled() && !SkIRect::Intersects(clip.scissorRect(), rtRect)) {
             return nullptr;
         }
 
-        return std::unique_ptr<GrClearOp>(new GrClearOp(clip, color, dstProxy));
+        // MDB TODO: remove this. In this hybrid state we need to be sure the RT is instantiable
+        // so it can carry the IO refs. In the future we will just get the proxy and
+        // it carry the IO refs.
+        if (!rtc->accessRenderTarget()) {
+            return nullptr;
+        }
+
+        return std::unique_ptr<GrClearOp>(new GrClearOp(clip, color, rtc));
     }
 
+    // MDB TODO: replace the renderTargetContext with just the renderTargetProxy.
     static std::unique_ptr<GrClearOp> Make(const SkIRect& rect, GrColor color,
+                                           GrRenderTargetContext* rtc,
                                            bool fullScreen) {
         SkASSERT(fullScreen || !rect.isEmpty());
 
-        return std::unique_ptr<GrClearOp>(new GrClearOp(rect, color, fullScreen));
+        // MDB TODO: remove this. See above comment.
+        if (!rtc->accessRenderTarget()) {
+            return nullptr;
+        }
+
+        return std::unique_ptr<GrClearOp>(new GrClearOp(rect, color, rtc, fullScreen));
     }
 
     const char* name() const override { return "Clear"; }
@@ -40,7 +59,9 @@ public:
     SkString dumpInfo() const override {
         SkString string;
         string.append(INHERITED::dumpInfo());
-        string.appendf("Scissor [ ");
+        string.appendf("rtID: %d proxyID: %d Scissor [",
+                       fRenderTarget.get()->uniqueID().asUInt(),
+                       fProxyUniqueID.asUInt());
         if (fClip.scissorEnabled()) {
             const SkIRect& r = fClip.scissorRect();
             string.appendf("L: %d, T: %d, R: %d, B: %d", r.fLeft, r.fTop, r.fRight, r.fBottom);
@@ -55,11 +76,13 @@ public:
     void setColor(GrColor color) { fColor = color; }
 
 private:
-    GrClearOp(const GrFixedClip& clip, GrColor color, GrSurfaceProxy* proxy)
+    GrClearOp(const GrFixedClip& clip, GrColor color, GrRenderTargetContext* rtc)
         : INHERITED(ClassID())
         , fClip(clip)
-        , fColor(color) {
+        , fColor(color)
+        , fProxyUniqueID(rtc->asSurfaceProxy()->uniqueID()) {
 
+        GrSurfaceProxy* proxy = rtc->asSurfaceProxy();
         const SkIRect rtRect = SkIRect::MakeWH(proxy->width(), proxy->height());
         if (fClip.scissorEnabled()) {
             // Don't let scissors extend outside the RT. This may improve op combining.
@@ -74,17 +97,20 @@ private:
         }
         this->setBounds(SkRect::Make(fClip.scissorEnabled() ? fClip.scissorRect() : rtRect),
                         HasAABloat::kNo, IsZeroArea::kNo);
+        fRenderTarget.reset(rtc->accessRenderTarget());
     }
 
-    GrClearOp(const SkIRect& rect, GrColor color, bool fullScreen)
+    GrClearOp(const SkIRect& rect, GrColor color, GrRenderTargetContext* rtc, bool fullScreen)
         : INHERITED(ClassID())
         , fClip(GrFixedClip(rect))
-        , fColor(color) {
+        , fColor(color)
+        , fProxyUniqueID(rtc->asSurfaceProxy()->uniqueID()) {
 
         if (fullScreen) {
             fClip.disableScissor();
         }
         this->setBounds(SkRect::Make(rect), HasAABloat::kNo, IsZeroArea::kNo);
+        fRenderTarget.reset(rtc->accessRenderTarget());
     }
 
     bool onCombineIfPossible(GrOp* t, const GrCaps& caps) override {
@@ -92,6 +118,8 @@ private:
         // contains the old clear, or when the new clear is a subset of the old clear and is the
         // same color.
         GrClearOp* cb = t->cast<GrClearOp>();
+        SkASSERT(cb->fRenderTarget == fRenderTarget);
+        SkASSERT(cb->fProxyUniqueID == fProxyUniqueID);
         if (fClip.windowRectsState() != cb->fClip.windowRectsState()) {
             return false;
         }
@@ -116,13 +144,16 @@ private:
     void onPrepare(GrOpFlushState*) override {}
 
     void onExecute(GrOpFlushState* state) override {
-        SkASSERT(state->drawOpArgs().fRenderTarget);
-
-        state->commandBuffer()->clear(state->drawOpArgs().fRenderTarget, fClip, fColor);
+        // MDB TODO: instantiate the renderTarget from the proxy in here
+        state->commandBuffer()->clear(fRenderTarget.get(), fClip, fColor);
     }
 
-    GrFixedClip fClip;
-    GrColor     fColor;
+    GrFixedClip                                             fClip;
+    GrColor                                                 fColor;
+
+    // MDB TODO: remove this. When the renderTargetProxy carries the refs this will be redundant.
+    GrSurfaceProxy::UniqueID                                fProxyUniqueID;
+    GrPendingIOResource<GrRenderTarget, kWrite_GrIOType>    fRenderTarget;
 
     typedef GrOp INHERITED;
 };