Remove render target unique ID virtual from GrOp.

GrRenderTargetOpList now stores the IDs along side each op.

This should put us closer to using proxy IDs and not forcing early render target instantiation as many comments point towards.

Change-Id: I1ee82b01a0818a80d2bcac39fdf3a4ee7dccecc9
Reviewed-on: https://skia-review.googlesource.com/6403
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

14 files changed, 100 insertions, 107 deletions

diff --git a/include/private/GrAuditTrail.h b/include/private/GrAuditTrail.h
index 0fbbf6f92d..f1ae494c9b 100644
--- a/include/private/GrAuditTrail.h
+++ b/include/private/GrAuditTrail.h
@@ -80,7 +80,7 @@ public:
         fCurrentStackTrace.push_back(SkString(framename));
     }
 
-    void addOp(const GrOp*);
+    void addOp(const GrOp*, GrGpuResource::UniqueID renderTargetID);
 
     void opsCombined(const GrOp* consumer, const GrOp* consumed);
 
@@ -172,7 +172,8 @@ private:
 #define GR_AUDIT_TRAIL_RESET(audit_trail) \
     //GR_AUDIT_TRAIL_INVOKE_GUARD(audit_trail, fullReset);
 
-#define GR_AUDIT_TRAIL_ADD_OP(audit_trail, op) GR_AUDIT_TRAIL_INVOKE_GUARD(audit_trail, addOp, op);
+#define GR_AUDIT_TRAIL_ADD_OP(audit_trail, op, rt_id) \
+    GR_AUDIT_TRAIL_INVOKE_GUARD(audit_trail, addOp, op, rt_id);
 
 #define GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(audit_trail, combineWith, op) \
     GR_AUDIT_TRAIL_INVOKE_GUARD(audit_trail, opsCombined, combineWith, op);
diff --git a/src/gpu/GrAuditTrail.cpp b/src/gpu/GrAuditTrail.cpp
index 203af35792..35139b7623 100644
--- a/src/gpu/GrAuditTrail.cpp
+++ b/src/gpu/GrAuditTrail.cpp
@@ -10,7 +10,7 @@
 
 const int GrAuditTrail::kGrAuditTrailInvalidID = -1;
 
-void GrAuditTrail::addOp(const GrOp* op) {
+void GrAuditTrail::addOp(const GrOp* op, GrGpuResource::UniqueID renderTargetID) {
     SkASSERT(fEnabled);
     Op* auditOp = new Op;
     fOpPool.emplace_back(auditOp);
@@ -44,7 +44,7 @@ void GrAuditTrail::addOp(const GrOp* op) {
 
     // We use the op pointer as a key to find the OpNode we are 'glomming' ops onto
     fIDLookup.set(op->uniqueID(), auditOp->fOpListID);
-    OpNode* opNode = new OpNode(op->renderTargetUniqueID());
+    OpNode* opNode = new OpNode(renderTargetID);
     opNode->fBounds = op->bounds();
     opNode->fChildren.push_back(auditOp);
     fOpList.emplace_back(opNode);
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index dcb1d0b2f7..93e86078fc 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -148,7 +148,7 @@ bool GrRenderTargetContext::onCopy(GrSurfaceProxy* srcProxy,
         return false;
     }
 
-    // TODO: this needs to be fixed up since it ends the deferrable of the GrRenderTarget
+    // TODO: This needs to be fixed up since it ends the deferral of the GrRenderTarget.
     sk_sp<GrRenderTarget> rt(
                         sk_ref_sp(fRenderTargetProxy->instantiate(fContext->textureProvider())));
     if (!rt) {
@@ -213,14 +213,14 @@ void GrRenderTargetContext::discard() {
 
     AutoCheckFlush acf(fDrawingManager);
 
-    // TODO: this needs to be fixed up since it ends the deferrable of the GrRenderTarget
+    // TODO: This needs to be fixed up since it ends the deferral of the GrRenderTarget.
     sk_sp<GrRenderTarget> rt(
                         sk_ref_sp(fRenderTargetProxy->instantiate(fContext->textureProvider())));
     if (!rt) {
         return;
     }
 
-    this->getOpList()->discard(rt.get());
+    this->getOpList()->discard(this);
 }
 
 void GrRenderTargetContext::clear(const SkIRect* rect,
@@ -289,7 +289,7 @@ void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect, const GrColor
         if (!op) {
             return;
         }
-        fRenderTargetContext->getOpList()->addOp(std::move(op));
+        fRenderTargetContext->getOpList()->addOp(std::move(op), fRenderTargetContext);
     }
 }
 
@@ -334,7 +334,7 @@ void GrRenderTargetContext::internalClear(const GrFixedClip& clip,
         this->drawRect(clip, paint, GrAA::kNo, SkMatrix::I(), SkRect::Make(clearRect));
     } else if (isFull) {
         if (this->accessRenderTarget()) {
-            this->getOpList()->fullClear(this->accessRenderTarget(), color);
+            this->getOpList()->fullClear(this, color);
         }
     } else {
         if (!this->accessRenderTarget()) {
@@ -344,7 +344,7 @@ void GrRenderTargetContext::internalClear(const GrFixedClip& clip,
         if (!op) {
             return;
         }
-        this->getOpList()->addOp(std::move(op));
+        this->getOpList()->addOp(std::move(op), this);
     }
 }
 
@@ -644,11 +644,12 @@ void GrRenderTargetContextPriv::clearStencilClip(const GrFixedClip& clip, bool i
                               "GrRenderTargetContextPriv::clearStencilClip");
 
     AutoCheckFlush acf(fRenderTargetContext->fDrawingManager);
+    // TODO: This needs to be fixed up since it ends the deferral of the GrRenderTarget.
     if (!fRenderTargetContext->accessRenderTarget()) {
         return;
     }
     fRenderTargetContext->getOpList()->clearStencilClip(clip, insideStencilMask,
-                                                        fRenderTargetContext->accessRenderTarget());
+                                                        fRenderTargetContext);
 }
 
 void GrRenderTargetContextPriv::stencilPath(const GrClip& clip,
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index 7ca7250eca..0e74ad1b79 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -50,7 +50,6 @@ GrRenderTargetOpList::GrRenderTargetOpList(GrRenderTargetProxy* rtp, GrGpu* gpu,
                                            GrResourceProvider* resourceProvider,
                                            GrAuditTrail* auditTrail, const Options& options)
     : INHERITED(rtp, auditTrail)
-    , fLastFullClearOp(nullptr)
     , fGpu(SkRef(gpu))
     , fResourceProvider(resourceProvider)
     , fLastClipStackGenID(SK_InvalidUniqueID) {
@@ -185,13 +184,13 @@ bool GrRenderTargetOpList::executeOps(GrOpFlushState* flushState) {
         if (!fRecordedOps[i].fOp) {
             continue;
         }
-        if (fRecordedOps[i].fOp->renderTargetUniqueID() != currentRTID) {
+        if (fRecordedOps[i].fRenderTargetID != currentRTID) {
             if (commandBuffer) {
                 commandBuffer->end();
                 commandBuffer->submit();
                 commandBuffer.reset();
             }
-            currentRTID = fRecordedOps[i].fOp->renderTargetUniqueID();
+            currentRTID = fRecordedOps[i].fRenderTargetID;
             if (!currentRTID.isInvalid()) {
                 static const GrGpuCommandBuffer::LoadAndStoreInfo kBasicLoadStoreInfo
                     { GrGpuCommandBuffer::LoadOp::kLoad,GrGpuCommandBuffer::StoreOp::kStore,
@@ -215,6 +214,7 @@ bool GrRenderTargetOpList::executeOps(GrOpFlushState* flushState) {
 
 void GrRenderTargetOpList::reset() {
     fLastFullClearOp = nullptr;
+    fLastFullClearRenderTargetID.makeInvalid();
     fRecordedOps.reset();
     if (fInstancedRendering) {
         fInstancedRendering->endFlush();
@@ -346,7 +346,7 @@ void GrRenderTargetOpList::addDrawOp(const GrPipelineBuilder& pipelineBuilder,
     SkASSERT(fSurface);
     op->pipeline()->addDependenciesTo(fSurface);
 #endif
-    this->recordOp(std::move(op), appliedClip.clippedDrawBounds());
+    this->recordOp(std::move(op), renderTargetContext, appliedClip.clippedDrawBounds());
 }
 
 void GrRenderTargetOpList::stencilPath(GrRenderTargetContext* renderTargetContext,
@@ -392,16 +392,16 @@ void GrRenderTargetOpList::stencilPath(GrRenderTargetContext* renderTargetContex
                                            appliedClip.scissorState(),
                                            renderTargetContext->accessRenderTarget(),
                                            path);
-    this->recordOp(std::move(op), appliedClip.clippedDrawBounds());
+    this->recordOp(std::move(op), renderTargetContext, appliedClip.clippedDrawBounds());
 }
 
-void GrRenderTargetOpList::fullClear(GrRenderTarget* renderTarget, GrColor color) {
+void GrRenderTargetOpList::fullClear(GrRenderTargetContext* renderTargetContext, GrColor color) {
+    GrRenderTarget* renderTarget = renderTargetContext->accessRenderTarget();
     // 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.
     // TODO: this needs to be updated to use GrSurfaceProxy::UniqueID
-    if (fLastFullClearOp &&
-        fLastFullClearOp->renderTargetUniqueID() == renderTarget->uniqueID()) {
+    if (fLastFullClearRenderTargetID == 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);
@@ -409,17 +409,19 @@ void GrRenderTargetOpList::fullClear(GrRenderTarget* renderTarget, GrColor color
         return;
     }
     sk_sp<GrClearOp> op(GrClearOp::Make(GrFixedClip::Disabled(), color, renderTarget));
-    if (GrOp* clearOp = this->recordOp(std::move(op))) {
+    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);
+        fLastFullClearRenderTargetID = renderTarget->uniqueID();
     }
 }
 
-void GrRenderTargetOpList::discard(GrRenderTarget* renderTarget) {
+void GrRenderTargetOpList::discard(GrRenderTargetContext* renderTargetContext) {
     // Currently this just inserts a discard op. However, once in MDB this can remove all the
     // previously recorded ops and change the load op to discard.
     if (this->caps()->discardRenderTargetSupport()) {
-        this->recordOp(GrDiscardOp::Make(renderTarget));
+        this->recordOp(GrDiscardOp::Make(renderTargetContext->accessRenderTarget()),
+                       renderTargetContext);
     }
 }
 
@@ -437,7 +439,10 @@ bool GrRenderTargetOpList::copySurface(GrSurface* dst,
     this->addDependency(src);
 #endif
 
-    this->recordOp(std::move(op));
+    // Copy surface doesn't work through a GrGpuCommandBuffer. By passing nullptr for the context we
+    // force this to occur between command buffers and execute directly on GrGpu. This workaround
+    // goes away with MDB.
+    this->recordOp(std::move(op), nullptr);
     return true;
 }
 
@@ -455,7 +460,13 @@ static void join(SkRect* out, const SkRect& a, const SkRect& b) {
     out->fBottom = SkTMax(a.fBottom, b.fBottom);
 }
 
-GrOp* GrRenderTargetOpList::recordOp(sk_sp<GrOp> op, const SkRect& clippedBounds) {
+GrOp* GrRenderTargetOpList::recordOp(sk_sp<GrOp> op, GrRenderTargetContext* renderTargetContext,
+                                     const SkRect& clippedBounds) {
+    // TODO: Should be proxy ID.
+    GrGpuResource::UniqueID renderTargetID =
+            renderTargetContext ? renderTargetContext->accessRenderTarget()->uniqueID()
+                                : GrGpuResource::UniqueID::InvalidID();
+
     // A closed GrOpList should never receive new/more ops
     SkASSERT(!this->isClosed());
 
@@ -463,7 +474,7 @@ GrOp* GrRenderTargetOpList::recordOp(sk_sp<GrOp> op, const SkRect& clippedBounds
     // 1) check every op
     // 2) intersect with something
     // 3) find a 'blocker'
-    GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get());
+    GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), renderTargetID);
     GrOP_INFO("Re-Recording (%s, B%u)\n"
               "\tBounds LRTB (%f, %f, %f, %f)\n",
                op->name(),
@@ -476,29 +487,30 @@ GrOp* GrRenderTargetOpList::recordOp(sk_sp<GrOp> op, const SkRect& clippedBounds
               clippedBounds.fBottom);
     GrOP_INFO("\tOutcome:\n");
     int maxCandidates = SkTMin(fMaxOpLookback, fRecordedOps.count());
-    if (maxCandidates) {
+    // If we don't have a valid destination render target ID then we cannot reorder.
+    if (maxCandidates && !renderTargetID.isInvalid()) {
         int i = 0;
         while (true) {
-            GrOp* candidate = fRecordedOps.fromBack(i).fOp.get();
+            const RecordedOp& candidate = fRecordedOps.fromBack(i);
             // We cannot continue to search backwards if the render target changes
-            if (candidate->renderTargetUniqueID() != op->renderTargetUniqueID()) {
-                GrOP_INFO("\t\tBreaking because of (%s, B%u) Rendertarget\n",
-                          candidate->name(), candidate->uniqueID());
+            if (candidate.fRenderTargetID != renderTargetID) {
+                GrOP_INFO("\t\tBreaking because of (%s, B%u) Rendertarget\n", candidate.fOp->name(),
+                          candidate.fOp->uniqueID());
                 break;
             }
-            if (candidate->combineIfPossible(op.get(), *this->caps())) {
-                GrOP_INFO("\t\tCombining with (%s, B%u)\n", candidate->name(),
-                          candidate->uniqueID());
-                GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(fAuditTrail, candidate, op.get());
+            if (candidate.fOp->combineIfPossible(op.get(), *this->caps())) {
+                GrOP_INFO("\t\tCombining with (%s, B%u)\n", candidate.fOp->name(),
+                          candidate.fOp->uniqueID());
+                GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(fAuditTrail, candidate.fOp.get(), op.get());
                 join(&fRecordedOps.fromBack(i).fClippedBounds,
                      fRecordedOps.fromBack(i).fClippedBounds, clippedBounds);
-                return candidate;
+                return candidate.fOp.get();
             }
             // Stop going backwards if we would cause a painter's order violation.
             const SkRect& candidateBounds = fRecordedOps.fromBack(i).fClippedBounds;
             if (!can_reorder(candidateBounds, clippedBounds)) {
-                GrOP_INFO("\t\tIntersects with (%s, B%u)\n", candidate->name(),
-                          candidate->uniqueID());
+                GrOP_INFO("\t\tIntersects with (%s, B%u)\n", candidate.fOp->name(),
+                          candidate.fOp->uniqueID());
                 break;
             }
             ++i;
@@ -511,8 +523,9 @@ GrOp* GrRenderTargetOpList::recordOp(sk_sp<GrOp> op, const SkRect& clippedBounds
         GrOP_INFO("\t\tFirstOp\n");
     }
     GR_AUDIT_TRAIL_OP_RESULT_NEW(fAuditTrail, op);
-    fRecordedOps.emplace_back(RecordedOp{std::move(op), clippedBounds});
+    fRecordedOps.emplace_back(RecordedOp{std::move(op), clippedBounds, renderTargetID});
     fLastFullClearOp = nullptr;
+    fLastFullClearRenderTargetID.makeInvalid();
     return fRecordedOps.back().fOp.get();
 }
 
@@ -522,25 +535,30 @@ void GrRenderTargetOpList::forwardCombine() {
     }
     for (int i = 0; i < fRecordedOps.count() - 2; ++i) {
         GrOp* op = fRecordedOps[i].fOp.get();
+        GrGpuResource::UniqueID renderTargetID = fRecordedOps[i].fRenderTargetID;
+        // If we don't have a valid destination render target ID then we cannot reorder.
+        if (renderTargetID.isInvalid()) {
+            continue;
+        }
         const SkRect& opBounds = fRecordedOps[i].fClippedBounds;
         int maxCandidateIdx = SkTMin(i + fMaxOpLookahead, fRecordedOps.count() - 1);
         int j = i + 1;
         while (true) {
-            GrOp* candidate = fRecordedOps[j].fOp.get();
+            const RecordedOp& candidate = fRecordedOps[j];
             // We cannot continue to search if the render target changes
-            if (candidate->renderTargetUniqueID() != op->renderTargetUniqueID()) {
-                GrOP_INFO("\t\tBreaking because of (%s, B%u) Rendertarget\n",
-                          candidate->name(), candidate->uniqueID());
+            if (candidate.fRenderTargetID != renderTargetID) {
+                GrOP_INFO("\t\tBreaking because of (%s, B%u) Rendertarget\n", candidate.fOp->name(),
+                          candidate.fOp->uniqueID());
                 break;
             }
             if (j == i +1) {
                 // We assume op would have combined with candidate when the candidate was added
                 // via backwards combining in recordOp.
-                SkASSERT(!op->combineIfPossible(candidate, *this->caps()));
-            } else if (op->combineIfPossible(candidate, *this->caps())) {
-                GrOP_INFO("\t\tCombining with (%s, B%u)\n", candidate->name(),
-                          candidate->uniqueID());
-                GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(fAuditTrail, op, candidate);
+                SkASSERT(!op->combineIfPossible(candidate.fOp.get(), *this->caps()));
+            } else if (op->combineIfPossible(candidate.fOp.get(), *this->caps())) {
+                GrOP_INFO("\t\tCombining with (%s, B%u)\n", candidate.fOp->name(),
+                          candidate.fOp->uniqueID());
+                GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(fAuditTrail, op, candidate.fOp.get());
                 fRecordedOps[j].fOp = std::move(fRecordedOps[i].fOp);
                 join(&fRecordedOps[j].fClippedBounds, fRecordedOps[j].fClippedBounds, opBounds);
                 break;
@@ -548,8 +566,8 @@ void GrRenderTargetOpList::forwardCombine() {
             // Stop going traversing if we would cause a painter's order violation.
             const SkRect& candidateBounds = fRecordedOps[j].fClippedBounds;
             if (!can_reorder(candidateBounds, opBounds)) {
-                GrOP_INFO("\t\tIntersects with (%s, B%u)\n", candidate->name(),
-                          candidate->uniqueID());
+                GrOP_INFO("\t\tIntersects with (%s, B%u)\n", candidate.fOp->name(),
+                          candidate.fOp->uniqueID());
                 break;
             }
             ++j;
@@ -565,6 +583,8 @@ void GrRenderTargetOpList::forwardCombine() {
 
 void GrRenderTargetOpList::clearStencilClip(const GrFixedClip& clip,
                                             bool insideStencilMask,
-                                            GrRenderTarget* rt) {
-    this->recordOp(GrClearStencilClipOp::Make(clip, insideStencilMask, rt));
+                                            GrRenderTargetContext* renderTargetContext) {
+    this->recordOp(GrClearStencilClipOp::Make(clip, insideStencilMask,
+                                              renderTargetContext->accessRenderTarget()),
+                   renderTargetContext);
 }
diff --git a/src/gpu/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h
index a2b323f118..a66686040b 100644
--- a/src/gpu/GrRenderTargetOpList.h
+++ b/src/gpu/GrRenderTargetOpList.h
@@ -81,7 +81,9 @@ public:
     void addDrawOp(const GrPipelineBuilder&, GrRenderTargetContext*, const GrClip&,
                    sk_sp<GrDrawOp>);
 
-    void addOp(sk_sp<GrOp> op) { this->recordOp(std::move(op)); }
+    void addOp(sk_sp<GrOp> op, GrRenderTargetContext* renderTargetContext) {
+        this->recordOp(std::move(op), renderTargetContext);
+    }
 
     /**
      * Draws the path into user stencil bits. Upon return, all user stencil values
@@ -97,10 +99,10 @@ public:
                      const GrPath*);
 
     /** Clears the entire render target */
-    void fullClear(GrRenderTarget*, GrColor color);
+    void fullClear(GrRenderTargetContext*, GrColor color);
 
     /** Discards the contents render target. */
-    void discard(GrRenderTarget*);
+    void discard(GrRenderTargetContext*);
 
     /**
      * Copies a pixel rectangle from one surface to another. This call may finalize
@@ -131,13 +133,13 @@ private:
 
     // If the input op is combined with an earlier op, this returns the combined op. Otherwise, it
     // returns the input op.
-    GrOp* recordOp(sk_sp<GrOp> op) {
+    GrOp* recordOp(sk_sp<GrOp> op, GrRenderTargetContext* renderTargetContext) {
         SkRect bounds = op->bounds();
-        return this->recordOp(std::move(op), bounds);
+        return this->recordOp(std::move(op), renderTargetContext, bounds);
     }
 
     // Variant that allows an explicit bounds (computed from the Op's bounds and a clip).
-    GrOp* recordOp(sk_sp<GrOp>, const SkRect& clippedBounds);
+    GrOp* recordOp(sk_sp<GrOp>, GrRenderTargetContext*, const SkRect& clippedBounds);
 
     void forwardCombine();
 
@@ -150,14 +152,19 @@ private:
                          GrXferProcessor::DstTexture*);
 
     // Used only via GrRenderTargetContextPriv.
-    void clearStencilClip(const GrFixedClip&, bool insideStencilMask, GrRenderTarget*);
+    void clearStencilClip(const GrFixedClip&, bool insideStencilMask, GrRenderTargetContext*);
 
     struct RecordedOp {
         sk_sp<GrOp> fOp;
         SkRect fClippedBounds;
+        // TODO: Use proxy ID instead of instantiated render target ID.
+        GrGpuResource::UniqueID fRenderTargetID;
     };
     SkSTArray<256, RecordedOp, true> fRecordedOps;
-    GrClearOp* fLastFullClearOp;
+
+    GrClearOp* fLastFullClearOp = nullptr;
+    GrGpuResource::UniqueID fLastFullClearRenderTargetID = GrGpuResource::UniqueID::InvalidID();
+
     // The context is only in service of the GrClip, remove once it doesn't need this.
     GrContext* fContext;
     GrGpu* fGpu;
diff --git a/src/gpu/GrTextureOpList.cpp b/src/gpu/GrTextureOpList.cpp
index b17de3f07f..2aaa8ca200 100644
--- a/src/gpu/GrTextureOpList.cpp
+++ b/src/gpu/GrTextureOpList.cpp
@@ -10,7 +10,7 @@
 #include "GrAuditTrail.h"
 #include "GrGpu.h"
 #include "GrTextureProxy.h"
-
+#include "SkStringUtils.h"
 #include "ops/GrCopySurfaceOp.h"
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -90,15 +90,16 @@ bool GrTextureOpList::copySurface(GrSurface* dst,
     this->addDependency(src);
 #endif
 
-    this->recordOp(std::move(op));
+    // See the comment in GrRenderTargetOpList about why we pass the invalid ID here.
+    this->recordOp(std::move(op), GrGpuResource::UniqueID::InvalidID());
     return true;
 }
 
-void GrTextureOpList::recordOp(sk_sp<GrOp> op) {
+void GrTextureOpList::recordOp(sk_sp<GrOp> op, GrGpuResource::UniqueID renderTargetID) {
     // A closed GrOpList should never receive new/more ops
     SkASSERT(!this->isClosed());
 
-    GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get());
+    GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), renderTargetID);
     GrOP_INFO("Re-Recording (%s, B%u)\n"
         "\tBounds LRTB (%f, %f, %f, %f)\n",
         op->name(),
diff --git a/src/gpu/GrTextureOpList.h b/src/gpu/GrTextureOpList.h
index 367fa0c336..ccfa1fbb7e 100644
--- a/src/gpu/GrTextureOpList.h
+++ b/src/gpu/GrTextureOpList.h
@@ -8,6 +8,7 @@
 #ifndef GrTexureOpList_DEFINED
 #define GrTexureOpList_DEFINED
 
+#include "GrGpuResource.h"
 #include "GrOpList.h"
 
 #include "SkTArray.h"
@@ -26,7 +27,7 @@ public:
     ~GrTextureOpList() override;
 
     /**
-     * Empties the draw buffer of any queued up draws.
+     * Empties the draw buffer of any queued ops.
      */
     void reset() override;
 
@@ -34,8 +35,8 @@ public:
     void freeGpuResources() override {}
 
     /**
-     * Together these two functions flush all queued up draws to GrCommandBuffer. The return value
-     * of drawOps() indicates whether any commands were actually issued to the GPU.
+     * Together these two functions flush all queued ops to GrGpuCommandBuffer. The return value
+     * of executeOps() indicates whether any commands were actually issued to the GPU.
      */
     void prepareOps(GrOpFlushState* flushState) override;
     bool executeOps(GrOpFlushState* flushState) override;
@@ -60,7 +61,8 @@ public:
     SkDEBUGCODE(void dump() const override;)
 
 private:
-    void recordOp(sk_sp<GrOp>);
+    // The unique ID is only needed for the audit trail. This should be removed with MDB.
+    void recordOp(sk_sp<GrOp>, GrGpuResource::UniqueID renderTargetID);
 
     SkSTArray<2, sk_sp<GrOp>, true> fRecordedOps;
     GrGpu*                          fGpu;
diff --git a/src/gpu/ops/GrClearOp.h b/src/gpu/ops/GrClearOp.h
index 1a4b33a766..81371884d8 100644
--- a/src/gpu/ops/GrClearOp.h
+++ b/src/gpu/ops/GrClearOp.h
@@ -34,11 +34,6 @@ public:
 
     const char* name() const override { return "Clear"; }
 
-    // TODO: this needs to be updated to return GrSurfaceProxy::UniqueID
-    GrGpuResource::UniqueID renderTargetUniqueID() const override {
-        return fRenderTarget.get()->uniqueID();
-    }
-
     SkString dumpInfo() const override {
         SkString string("Scissor [");
         if (fClip.scissorEnabled()) {
diff --git a/src/gpu/ops/GrClearStencilClipOp.h b/src/gpu/ops/GrClearStencilClipOp.h
index a5d6a03bb3..7f3f68a70b 100644
--- a/src/gpu/ops/GrClearStencilClipOp.h
+++ b/src/gpu/ops/GrClearStencilClipOp.h
@@ -25,11 +25,6 @@ public:
 
     const char* name() const override { return "ClearStencilClip"; }
 
-    // TODO: this needs to be updated to return GrSurfaceProxy::UniqueID
-    GrGpuResource::UniqueID renderTargetUniqueID() const override {
-        return fRenderTarget.get()->uniqueID();
-    }
-
     SkString dumpInfo() const override {
         SkString string("Scissor [");
         if (fClip.scissorEnabled()) {
diff --git a/src/gpu/ops/GrCopySurfaceOp.h b/src/gpu/ops/GrCopySurfaceOp.h
index 789f7a25a1..1f4fbfa8a9 100644
--- a/src/gpu/ops/GrCopySurfaceOp.h
+++ b/src/gpu/ops/GrCopySurfaceOp.h
@@ -32,14 +32,6 @@ public:
 
     const char* name() const override { return "CopySurface"; }
 
-    // TODO: this needs to be updated to return GrSurfaceProxy::UniqueID
-    GrGpuResource::UniqueID renderTargetUniqueID() const override {
-        // Copy surface doesn't work through a GrGpuCommandBuffer. By returning an invalid RT ID we
-        // force the caller to end the previous command buffer and execute this copy before
-        // beginning a new one.
-        return GrGpuResource::UniqueID::InvalidID();
-    }
-
     SkString dumpInfo() const override {
         SkString string;
         string.printf(
diff --git a/src/gpu/ops/GrDiscardOp.h b/src/gpu/ops/GrDiscardOp.h
index e7225af0d2..6923be2a49 100644
--- a/src/gpu/ops/GrDiscardOp.h
+++ b/src/gpu/ops/GrDiscardOp.h
@@ -20,11 +20,6 @@ public:
 
     const char* name() const override { return "Discard"; }
 
-    // TODO: this needs to be updated to return GrSurfaceProxy::UniqueID
-    GrGpuResource::UniqueID renderTargetUniqueID() const override {
-        return fRenderTarget.get()->uniqueID();
-    }
-
     SkString dumpInfo() const override {
         SkString string;
         string.printf("RT: %d", fRenderTarget.get()->uniqueID().asUInt());
@@ -39,7 +34,7 @@ private:
     }
 
     bool onCombineIfPossible(GrOp* that, const GrCaps& caps) override {
-        return this->renderTargetUniqueID() == that->renderTargetUniqueID();
+        return fRenderTarget.get() == that->cast<GrDiscardOp>()->fRenderTarget.get();
     }
 
     void onPrepare(GrOpFlushState*) override {}
diff --git a/src/gpu/ops/GrDrawOp.h b/src/gpu/ops/GrDrawOp.h
index ad8a545b78..9e949b13cd 100644
--- a/src/gpu/ops/GrDrawOp.h
+++ b/src/gpu/ops/GrDrawOp.h
@@ -67,12 +67,6 @@ public:
     // TODO no GrPrimitiveProcessors yet read fragment position
     bool willReadFragmentPosition() const { return false; }
 
-    // TODO: this needs to be updated to return GrSurfaceProxy::UniqueID
-    // This is a bit more exciting than the other call sites since it uses the pipeline
-    GrGpuResource::UniqueID renderTargetUniqueID() const final {
-        return this->pipeline()->getRenderTarget()->uniqueID();
-    }
-
 protected:
     static SkString DumpPipelineInfo(const GrPipeline& pipeline) {
         SkString string;
diff --git a/src/gpu/ops/GrOp.h b/src/gpu/ops/GrOp.h
index 47a8097fa0..87748f52f8 100644
--- a/src/gpu/ops/GrOp.h
+++ b/src/gpu/ops/GrOp.h
@@ -129,11 +129,6 @@ public:
     /** Issues the op's commands to GrGpu. */
     void execute(GrOpFlushState* state, const SkRect& bounds) { this->onExecute(state, bounds); }
 
-    /** Used to block combining across render target changes. Remove this once we store
-        GrOps for different RTs in different targets. */
-    // TODO: this needs to be updated to return GrSurfaceProxy::UniqueID
-    virtual GrGpuResource::UniqueID renderTargetUniqueID() const = 0;
-
     /** Used for spewing information about ops when debugging. */
     virtual SkString dumpInfo() const {
         SkString string;
diff --git a/src/gpu/ops/GrStencilPathOp.h b/src/gpu/ops/GrStencilPathOp.h
index fc29402258..03dc7ee067 100644
--- a/src/gpu/ops/GrStencilPathOp.h
+++ b/src/gpu/ops/GrStencilPathOp.h
@@ -33,11 +33,6 @@ public:
 
     const char* name() const override { return "StencilPathOp"; }
 
-    // TODO: this needs to be updated to return GrSurfaceProxy::UniqueID
-    GrGpuResource::UniqueID renderTargetUniqueID() const override {
-        return fRenderTarget.get()->uniqueID();
-    }
-
     SkString dumpInfo() const override {
         SkString string;
         string.printf("PATH: 0x%p, AA:%d", fPath.get(), fUseHWAA);