Use clipped bounds for reordering decisions

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2137543002

NOTREECHECKS=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2137543002

4 files changed, 97 insertions, 59 deletions

diff --git a/src/gpu/GrClip.cpp b/src/gpu/GrClip.cpp
index d74d935585..b0577c5122 100644
--- a/src/gpu/GrClip.cpp
+++ b/src/gpu/GrClip.cpp
@@ -44,6 +44,7 @@ void GrFixedClip::getConservativeBounds(int width, int height, SkIRect* devResul
 bool GrFixedClip::apply(GrContext*, const GrPipelineBuilder& pipelineBuilder,
                         GrDrawContext* drawContext,
                         const SkRect* devBounds, GrAppliedClip* out) const {
+    SkASSERT(!fDeviceBounds.isLargest());
     if (fScissorState.enabled()) {
         SkIRect tightScissor;
         if (!tightScissor.intersect(fScissorState.rect(),
@@ -53,11 +54,15 @@ bool GrFixedClip::apply(GrContext*, const GrPipelineBuilder& pipelineBuilder,
         if (devBounds && !devBounds->intersects(SkRect::Make(tightScissor))) {
             return false;
         }
-        out->makeScissoredStencil(fHasStencilClip, tightScissor);
+        if (fHasStencilClip) {
+            out->makeScissoredStencil(tightScissor, &fDeviceBounds);
+        } else {
+            out->makeScissored(tightScissor);
+        }
         return true;
     }
 
-    out->makeStencil(fHasStencilClip);
+    out->makeStencil(fHasStencilClip, fDeviceBounds);
     return true;
 }
 
diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index cce0b20eee..c591bf1182 100644
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -306,7 +306,7 @@ bool GrClipMaskManager::SetupClipping(GrContext* context,
                 out->makeScissoredFPBased(std::move(clipFP), scissorSpaceIBounds);
                 return true;
             }
-            out->makeFPBased(std::move(clipFP));
+            out->makeFPBased(std::move(clipFP), SkRect::Make(scissorSpaceIBounds));
             return true;
         }
     }
@@ -347,7 +347,8 @@ bool GrClipMaskManager::SetupClipping(GrContext* context,
             // clipSpace bounds. We determine the mask's position WRT to the render target here.
             SkIRect rtSpaceMaskBounds = clipSpaceIBounds;
             rtSpaceMaskBounds.offset(-clip.origin());
-            out->makeFPBased(create_fp_for_mask(result.get(), rtSpaceMaskBounds));
+            out->makeFPBased(create_fp_for_mask(result.get(), rtSpaceMaskBounds),
+                             SkRect::Make(rtSpaceMaskBounds));
             return true;
         }
         // if alpha clip mask creation fails fall through to the non-AA code paths
@@ -368,7 +369,7 @@ bool GrClipMaskManager::SetupClipping(GrContext* context,
     // use both stencil and scissor test to the bounds for the final draw.
     SkIRect scissorSpaceIBounds(clipSpaceIBounds);
     scissorSpaceIBounds.offset(clipSpaceToStencilSpaceOffset);
-    out->makeScissoredStencil(true, scissorSpaceIBounds);
+    out->makeScissoredStencil(scissorSpaceIBounds);
     return true;
 }
 
@@ -594,7 +595,7 @@ bool GrClipMaskManager::CreateStencilClipMask(GrContext* context,
 
             bool fillInverted = false;
             // enabled at bottom of loop
-            clip.enableStencilClip(false);
+            clip.disableStencilClip();
 
             // This will be used to determine whether the clip shape can be rendered into the
             // stencil with arbitrary stencil settings.
@@ -690,16 +691,20 @@ bool GrClipMaskManager::CreateStencilClipMask(GrContext* context,
 
             // now we modify the clip bit by rendering either the clip
             // element directly or a bounding rect of the entire clip.
-            clip.enableStencilClip(true);
             for (GrUserStencilSettings const* const* pass = stencilPasses; *pass; ++pass) {
 
                 if (drawDirectToClip) {
                     if (Element::kRect_Type == element->getType()) {
+                        clip.enableStencilClip(element->getRect().makeOffset(translate.fX,
+                                                                             translate.fY));
                         drawContext->drawContextPriv().stencilRect(clip, *pass, useHWAA, viewMatrix,
                                                                    element->getRect());
                     } else {
                         GrShape shape(clipPath, GrStyle::SimpleFill());
                         GrPaint paint;
+                        SkRect bounds = clipPath.getBounds();
+                        bounds.offset(translate.fX, translate.fY);
+                        clip.enableStencilClip(bounds);
                         paint.setXPFactory(GrDisableColorXPFactory::Make());
                         paint.setAntiAlias(element->isAA());
                         GrPathRenderer::DrawPathArgs args;
@@ -717,6 +722,9 @@ bool GrClipMaskManager::CreateStencilClipMask(GrContext* context,
                 } else {
                     // The view matrix is setup to do clip space -> stencil space translation, so
                     // draw rect in clip space.
+                    SkRect bounds = SkRect::Make(clipSpaceIBounds);
+                    bounds.offset(translate.fX, translate.fY);
+                    clip.enableStencilClip(bounds);
                     drawContext->drawContextPriv().stencilRect(clip, *pass, false, viewMatrix,
                                                                SkRect::Make(clipSpaceIBounds));
                 }
diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp
index 356c480da8..8beaade32c 100644
--- a/src/gpu/GrDrawTarget.cpp
+++ b/src/gpu/GrDrawTarget.cpp
@@ -115,15 +115,19 @@ void GrDrawTarget::dump() const {
         SkDebugf("%d, ", fDependencies[i]->fDebugID);
     }
     SkDebugf("\n");
-    SkDebugf("batches (%d):\n", fBatches.count());
-    for (int i = 0; i < fBatches.count(); ++i) {
+    SkDebugf("batches (%d):\n", fRecordedBatches.count());
+    for (int i = 0; i < fRecordedBatches.count(); ++i) {
         SkDebugf("*******************************\n");
-        if (!fBatches[i]) {
+        if (!fRecordedBatches[i].fBatch) {
             SkDebugf("%d: <combined forward>\n", i);
         } else {
-            SkDebugf("%d: %s\n", i, fBatches[i]->name());
-            SkString str = fBatches[i]->dumpInfo();
+            SkDebugf("%d: %s\n", i, fRecordedBatches[i].fBatch->name());
+            SkString str = fRecordedBatches[i].fBatch->dumpInfo();
             SkDebugf("%s\n", str.c_str());
+            const SkRect& clippedBounds = fRecordedBatches[i].fClippedBounds;
+            SkDebugf("ClippedBounds: [L: %.2f, T: %.2f, R: %.2f, B: %.2f]\n",
+                     clippedBounds.fLeft, clippedBounds.fTop, clippedBounds.fRight,
+                     clippedBounds.fBottom);
         }
     }
 }
@@ -199,9 +203,9 @@ void GrDrawTarget::prepareBatches(GrBatchFlushState* flushState) {
     this->makeClosed();
 
     // Loop over the batches that haven't yet generated their geometry
-    for (int i = 0; i < fBatches.count(); ++i) {
-        if (fBatches[i]) {
-            fBatches[i]->prepare(flushState);
+    for (int i = 0; i < fRecordedBatches.count(); ++i) {
+        if (fRecordedBatches[i].fBatch) {
+            fRecordedBatches[i].fBatch->prepare(flushState);
         }
     }
 
@@ -216,11 +220,11 @@ void GrDrawTarget::drawBatches(GrBatchFlushState* flushState) {
     GrRenderTarget* currentRT = nullptr;
     SkAutoTDelete<GrGpuCommandBuffer> commandBuffer;
     SkRect bounds = SkRect::MakeEmpty();
-    for (int i = 0; i < fBatches.count(); ++i) {
-        if (!fBatches[i]) {
+    for (int i = 0; i < fRecordedBatches.count(); ++i) {
+        if (!fRecordedBatches[i].fBatch) {
             continue;
         }
-        if (fBatches[i]->renderTarget() != currentRT) {
+        if (fRecordedBatches[i].fBatch->renderTarget() != currentRT) {
             if (commandBuffer) {
                 commandBuffer->end();
                 if (bounds.intersect(0, 0,
@@ -233,7 +237,7 @@ void GrDrawTarget::drawBatches(GrBatchFlushState* flushState) {
                 commandBuffer.reset();
             }
             bounds.setEmpty();
-            currentRT = fBatches[i]->renderTarget();
+            currentRT = fRecordedBatches[i].fBatch->renderTarget();
             if (currentRT) {
                 static const GrGpuCommandBuffer::LoadAndStoreInfo kBasicLoadStoreInfo
                     { GrGpuCommandBuffer::LoadOp::kLoad,GrGpuCommandBuffer::StoreOp::kStore,
@@ -245,19 +249,19 @@ void GrDrawTarget::drawBatches(GrBatchFlushState* flushState) {
             flushState->setCommandBuffer(commandBuffer);
         }
         if (commandBuffer) {
-            bounds.join(fBatches[i]->bounds());
+            bounds.join(fRecordedBatches[i].fClippedBounds);
         }
         if (fDrawBatchBounds) {
-            const SkRect& batchBounds = fBatches[i]->bounds();
-            SkIRect iBatchBounds;
-            batchBounds.roundOut(&iBatchBounds);
+            const SkRect& bounds = fRecordedBatches[i].fClippedBounds;
+            SkIRect ibounds;
+            bounds.roundOut(&ibounds);
             // In multi-draw buffer all the batches use the same render target and we won't need to
             // get the batchs bounds.
-            if (GrRenderTarget* rt = fBatches[i]->renderTarget()) {
-                fGpu->drawDebugWireRect(rt, iBatchBounds, 0xFF000000 | random.nextU());
+            if (GrRenderTarget* rt = fRecordedBatches[i].fBatch->renderTarget()) {
+                fGpu->drawDebugWireRect(rt, ibounds, 0xFF000000 | random.nextU());
             }
         }
-        fBatches[i]->draw(flushState);
+        fRecordedBatches[i].fBatch->draw(flushState);
     }
     if (commandBuffer) {
         commandBuffer->end();
@@ -275,7 +279,7 @@ void GrDrawTarget::drawBatches(GrBatchFlushState* flushState) {
 }
 
 void GrDrawTarget::reset() {
-    fBatches.reset();
+    fRecordedBatches.reset();
     if (fInstancedRendering) {
         fInstancedRendering->endFlush();
     }
@@ -307,6 +311,16 @@ static void batch_bounds(SkRect* bounds, const GrBatch* batch) {
     }
 }
 
+static inline bool intersect(SkRect* out, const SkRect& a, const SkRect& b) {
+    SkASSERT(a.fLeft <= a.fRight && a.fTop <= a.fBottom);
+    SkASSERT(b.fLeft <= b.fRight && b.fTop <= b.fBottom);
+    out->fLeft   = SkTMax(a.fLeft,   b.fLeft);
+    out->fTop    = SkTMax(a.fTop,    b.fTop);
+    out->fRight  = SkTMin(a.fRight,  b.fRight);
+    out->fBottom = SkTMin(a.fBottom, b.fBottom);
+    return (out->fLeft <= out->fRight && out->fTop <= out->fBottom);
+}
+
 void GrDrawTarget::drawBatch(const GrPipelineBuilder& pipelineBuilder,
                              GrDrawContext* drawContext,
                              const GrClip& clip,
@@ -384,8 +398,9 @@ void GrDrawTarget::drawBatch(const GrPipelineBuilder& pipelineBuilder,
     SkASSERT(fRenderTarget);
     batch->pipeline()->addDependenciesTo(fRenderTarget);
 #endif
-
-    this->recordBatch(batch);
+    SkRect clippedBounds;
+    SkAssertResult(intersect(&clippedBounds, bounds, appliedClip.deviceBounds()));
+    this->recordBatch(batch, clippedBounds);
 }
 
 void GrDrawTarget::stencilPath(const GrPipelineBuilder& pipelineBuilder,
@@ -424,7 +439,7 @@ void GrDrawTarget::stencilPath(const GrPipelineBuilder& pipelineBuilder,
                                                 appliedClip.scissorState(),
                                                 drawContext->accessRenderTarget(),
                                                 path);
-    this->recordBatch(batch);
+    this->recordBatch(batch, appliedClip.deviceBounds());
     batch->unref();
 }
 
@@ -465,7 +480,7 @@ void GrDrawTarget::clear(const SkIRect* rect,
         this->drawBatch(pipelineBuilder, drawContext, GrNoClip(), batch);
     } else {
         GrBatch* batch = new GrClearBatch(*rect, color, drawContext->accessRenderTarget());
-        this->recordBatch(batch);
+        this->recordBatch(batch, batch->bounds());
         batch->unref();
     }
 }
@@ -473,7 +488,7 @@ void GrDrawTarget::clear(const SkIRect* rect,
 void GrDrawTarget::discard(GrRenderTarget* renderTarget) {
     if (this->caps()->discardRenderTargetSupport()) {
         GrBatch* batch = new GrDiscardBatch(renderTarget);
-        this->recordBatch(batch);
+        this->recordBatch(batch, batch->bounds());
         batch->unref();
     }
 }
@@ -492,25 +507,26 @@ bool GrDrawTarget::copySurface(GrSurface* dst,
     this->addDependency(src);
 #endif
 
-    this->recordBatch(batch);
+    this->recordBatch(batch, batch->bounds());
     batch->unref();
     return true;
 }
 
-static inline bool exclusive_no_intersection(const SkRect& a, const SkRect& b) {
+static inline bool can_reorder(const SkRect& a, const SkRect& b) {
     return a.fRight <= b.fLeft || a.fBottom <= b.fTop ||
            b.fRight <= a.fLeft || b.fBottom <= a.fTop;
 }
 
-static inline bool can_reorder(const GrBatch* a, const GrBatch* b) {
-    SkRect ra;
-    SkRect rb;
-    batch_bounds(&ra, a);
-    batch_bounds(&rb, a);
-    return exclusive_no_intersection(ra, rb);
+static void join(SkRect* out, const SkRect& a, const SkRect& b) {
+    SkASSERT(a.fLeft <= a.fRight && a.fTop <= a.fBottom);
+    SkASSERT(b.fLeft <= b.fRight && b.fTop <= b.fBottom);
+    out->fLeft   = SkTMin(a.fLeft,   b.fLeft);
+    out->fTop    = SkTMin(a.fTop,    b.fTop);
+    out->fRight  = SkTMax(a.fRight,  b.fRight);
+    out->fBottom = SkTMax(a.fBottom, b.fBottom);
 }
 
-void GrDrawTarget::recordBatch(GrBatch* batch) {
+void GrDrawTarget::recordBatch(GrBatch* batch, const SkRect& clippedBounds) {
     // A closed drawTarget should never receive new/more batches
     SkASSERT(!this->isClosed());
 
@@ -526,12 +542,15 @@ void GrDrawTarget::recordBatch(GrBatch* batch) {
         batch->bounds().fLeft, batch->bounds().fRight,
         batch->bounds().fTop, batch->bounds().fBottom);
     GrBATCH_INFO(SkTabString(batch->dumpInfo(), 1).c_str());
+    GrBATCH_INFO("\tClipped Bounds: [L: %.2f, T: %.2f, R: %.2f, B: %.2f]\n",
+                 clippedBounds.fLeft, clippedBounds.fTop, clippedBounds.fRight,
+                 clippedBounds.fBottom);
     GrBATCH_INFO("\tOutcome:\n");
-    int maxCandidates = SkTMin(fMaxBatchLookback, fBatches.count());
+    int maxCandidates = SkTMin(fMaxBatchLookback, fRecordedBatches.count());
     if (maxCandidates) {
         int i = 0;
         while (true) {
-            GrBatch* candidate = fBatches.fromBack(i);
+            GrBatch* candidate = fRecordedBatches.fromBack(i).fBatch.get();
             // We cannot continue to search backwards if the render target changes
             if (candidate->renderTargetUniqueID() != batch->renderTargetUniqueID()) {
                 GrBATCH_INFO("\t\tBreaking because of (%s, B%u) Rendertarget\n",
@@ -542,12 +561,13 @@ void GrDrawTarget::recordBatch(GrBatch* batch) {
                 GrBATCH_INFO("\t\tCombining with (%s, B%u)\n", candidate->name(),
                     candidate->uniqueID());
                 GR_AUDIT_TRAIL_BATCHING_RESULT_COMBINED(fAuditTrail, candidate, batch);
+                join(&fRecordedBatches.fromBack(i).fClippedBounds,
+                     fRecordedBatches.fromBack(i).fClippedBounds, clippedBounds);
                 return;
             }
             // Stop going backwards if we would cause a painter's order violation.
-            // TODO: The bounds used here do not fully consider the clip. It may be advantageous
-            // to clip each batch's bounds to the clip.
-            if (!can_reorder(candidate, batch)) {
+            const SkRect& candidateBounds = fRecordedBatches.fromBack(i).fClippedBounds;
+            if (!can_reorder(candidateBounds, clippedBounds)) {
                 GrBATCH_INFO("\t\tIntersects with (%s, B%u)\n", candidate->name(),
                     candidate->uniqueID());
                 break;
@@ -562,16 +582,17 @@ void GrDrawTarget::recordBatch(GrBatch* batch) {
         GrBATCH_INFO("\t\tFirstBatch\n");
     }
     GR_AUDIT_TRAIL_BATCHING_RESULT_NEW(fAuditTrail, batch);
-    fBatches.push_back().reset(SkRef(batch));
+    fRecordedBatches.emplace_back(RecordedBatch{sk_ref_sp(batch), clippedBounds});
 }
 
 void GrDrawTarget::forwardCombine() {
-    for (int i = 0; i < fBatches.count() - 2; ++i) {
-        GrBatch* batch = fBatches[i];
-        int maxCandidateIdx = SkTMin(i + fMaxBatchLookahead, fBatches.count() - 1);
+    for (int i = 0; i < fRecordedBatches.count() - 2; ++i) {
+        GrBatch* batch = fRecordedBatches[i].fBatch.get();
+        const SkRect& batchBounds = fRecordedBatches[i].fClippedBounds;
+        int maxCandidateIdx = SkTMin(i + fMaxBatchLookahead, fRecordedBatches.count() - 1);
         int j = i + 1;
         while (true) {
-            GrBatch* candidate = fBatches[j];
+            GrBatch* candidate = fRecordedBatches[j].fBatch.get();
             // We cannot continue to search if the render target changes
             if (candidate->renderTargetUniqueID() != batch->renderTargetUniqueID()) {
                 GrBATCH_INFO("\t\tBreaking because of (%s, B%u) Rendertarget\n",
@@ -586,14 +607,14 @@ void GrDrawTarget::forwardCombine() {
                 GrBATCH_INFO("\t\tCombining with (%s, B%u)\n", candidate->name(),
                              candidate->uniqueID());
                 GR_AUDIT_TRAIL_BATCHING_RESULT_COMBINED(fAuditTrail, batch, candidate);
-                fBatches[j].reset(SkRef(batch));
-                fBatches[i].reset(nullptr);
+                fRecordedBatches[j].fBatch = std::move(fRecordedBatches[i].fBatch);
+                join(&fRecordedBatches[j].fClippedBounds, fRecordedBatches[j].fClippedBounds,
+                     batchBounds);
                 break;
             }
             // Stop going traversing if we would cause a painter's order violation.
-            // TODO: The bounds used here do not fully consider the clip. It may be advantageous
-            // to clip each batch's bounds to the clip.
-            if (!can_reorder(candidate, batch)) {
+            const SkRect& candidateBounds = fRecordedBatches[j].fClippedBounds;
+            if (!can_reorder(candidateBounds, batchBounds)) {
                 GrBATCH_INFO("\t\tIntersects with (%s, B%u)\n", candidate->name(),
                              candidate->uniqueID());
                 break;
@@ -611,6 +632,6 @@ void GrDrawTarget::forwardCombine() {
 
 void GrDrawTarget::clearStencilClip(const SkIRect& rect, bool insideClip, GrRenderTarget* rt) {
     GrBatch* batch = new GrClearStencilClipBatch(rect, insideClip, rt);
-    this->recordBatch(batch);
+    this->recordBatch(batch, batch->bounds());
     batch->unref();
 }
diff --git a/src/gpu/GrDrawTarget.h b/src/gpu/GrDrawTarget.h
index 2a5bbde1a3..7b268c2827 100644
--- a/src/gpu/GrDrawTarget.h
+++ b/src/gpu/GrDrawTarget.h
@@ -196,7 +196,7 @@ private:
         }
     };
 
-    void recordBatch(GrBatch*);
+    void recordBatch(GrBatch*, const SkRect& clippedBounds);
     void forwardCombine();
 
     // Makes a copy of the dst if it is necessary for the draw. Returns false if a copy is required
@@ -214,7 +214,11 @@ private:
     // Used only by drawContextPriv.
     void clearStencilClip(const SkIRect&, bool insideClip, GrRenderTarget*);
 
-    SkSTArray<256, SkAutoTUnref<GrBatch>, true>     fBatches;
+    struct RecordedBatch {
+        sk_sp<GrBatch> fBatch;
+        SkRect         fClippedBounds;
+    };
+    SkSTArray<256, RecordedBatch, true>             fRecordedBatches;
     // The context is only in service of the clip mask manager, remove once CMM doesn't need this.
     GrContext*                                      fContext;
     GrGpu*                                          fGpu;