Pre-crop filled rects to avoid scissor

Updates GrDrawContext to crop filled rects to the clip bounds before
creating batches for them. Also adds clipping logic to ignore scissor
when the draw falls completely inside. These two changes combined
reduce API traffic and improve batching.

In the future this can and should be improved by switching to floating
point clip boundaries, thus allowing us to throw out non pixel aligned
rectangle clips as well.

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

Committed: https://skia.googlesource.com/skia/+/7969838702135b9f127bd738728da61bc49b050a
Review-Url: https://codereview.chromium.org/2132073002

diff --git a/gm/croppedrects.cpp b/gm/croppedrects.cpp
new file mode 100644 (file)
index 0000000..5c36c54
--- /dev/null
+++ b/gm/croppedrects.cpp
@@ -0,0 +1,108 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "gm.h"
+#include "SkPath.h"
+#include "SkRandom.h"
+#include "SkRRect.h"
+#include "SkSurface.h"
+
+namespace skiagm {
+
+constexpr SkRect kSrcImageClip{75, 75, 275, 275};
+
+/*
+ * The purpose of this test is to exercise all three codepaths in GrDrawContext (drawFilledRect,
+ * fillRectToRect, fillRectWithLocalMatrix) that pre-crop filled rects based on the clip.
+ *
+ * The test creates an image of a green square surrounded by red background, then draws this image
+ * in various ways with the red clipped out. The test is successful if there is no visible red
+ * background, scissor is never used, and ideally, all the rectangles draw in one batch.
+ */
+class CroppedRectsGM : public GM {
+private:
+    SkString onShortName() override final { return SkString("croppedrects"); }
+    SkISize onISize() override { return SkISize::Make(500, 500); }
+
+    void onOnceBeforeDraw() override {
+        sk_sp<SkSurface> srcSurface = SkSurface::MakeRasterN32Premul(500, 500);
+        SkCanvas* srcCanvas = srcSurface->getCanvas();
+
+        srcCanvas->clear(SK_ColorRED);
+
+        SkPaint paint;
+        paint.setColor(0xff00ff00);
+        srcCanvas->drawRect(kSrcImageClip, paint);
+
+        constexpr SkScalar kStrokeWidth = 10;
+        SkPaint stroke;
+        stroke.setStyle(SkPaint::kStroke_Style);
+        stroke.setStrokeWidth(kStrokeWidth);
+        stroke.setColor(0xff008800);
+        srcCanvas->drawRect(kSrcImageClip.makeInset(kStrokeWidth / 2, kStrokeWidth / 2), stroke);
+
+        fSrcImage = srcSurface->makeImageSnapshot(SkBudgeted::kYes, SkSurface::kNo_ForceUnique);
+        fSrcImageShader = fSrcImage->makeShader(SkShader::kClamp_TileMode,
+                                                SkShader::kClamp_TileMode);
+    }
+
+    void onDraw(SkCanvas* canvas) override {
+        canvas->clear(SK_ColorWHITE);
+
+        {
+            // GrDrawContext::drawFilledRect.
+            SkAutoCanvasRestore acr(canvas, true);
+            SkPaint paint;
+            paint.setShader(fSrcImageShader);
+            paint.setFilterQuality(kNone_SkFilterQuality);
+            canvas->clipRect(kSrcImageClip);
+            canvas->drawPaint(paint);
+        }
+
+        {
+            // GrDrawContext::fillRectToRect.
+            SkAutoCanvasRestore acr(canvas, true);
+            SkPaint paint;
+            paint.setFilterQuality(kNone_SkFilterQuality);
+            SkRect drawRect = SkRect::MakeXYWH(350, 100, 100, 300);
+            canvas->clipRect(drawRect);
+            canvas->drawImageRect(fSrcImage.get(),
+                                  kSrcImageClip.makeOutset(0.5f * kSrcImageClip.width(),
+                                                           kSrcImageClip.height()),
+                                  drawRect.makeOutset(0.5f * drawRect.width(), drawRect.height()),
+                                  &paint);
+        }
+
+        {
+            // GrDrawContext::fillRectWithLocalMatrix.
+            SkAutoCanvasRestore acr(canvas, true);
+            SkPath path;
+            path.moveTo(kSrcImageClip.fLeft - kSrcImageClip.width(), kSrcImageClip.centerY());
+            path.lineTo(kSrcImageClip.fRight + 3 * kSrcImageClip.width(), kSrcImageClip.centerY());
+            SkPaint paint;
+            paint.setStyle(SkPaint::kStroke_Style);
+            paint.setStrokeWidth(2 * kSrcImageClip.height());
+            paint.setShader(fSrcImageShader);
+            paint.setFilterQuality(kNone_SkFilterQuality);
+            canvas->translate(-90, 263);
+            canvas->scale(300 / kSrcImageClip.width(), 100 / kSrcImageClip.height());
+            canvas->clipRect(kSrcImageClip);
+            canvas->drawPath(path, paint);
+        }
+
+        // TODO: assert the draw target only has one batch in the post-MDB world.
+    }
+
+    sk_sp<SkImage> fSrcImage;
+    sk_sp<SkShader> fSrcImageShader;
+
+    typedef GM INHERITED;
+};
+
+DEF_GM( return new CroppedRectsGM(); )
+
+}

diff --git a/include/gpu/GrClip.h b/include/gpu/GrClip.h
index e585281..b63f213 100644 (file)
--- a/include/gpu/GrClip.h
+++ b/include/gpu/GrClip.h
@@ -109,6 +109,26 @@ public:
                        const SkRect* devBounds, GrAppliedClip*) const = 0;
 
     virtual ~GrClip() {}
+
+protected:
+    /**
+     * Returns true if a clip can safely disable its scissor test for a particular draw.
+     */
+    static bool CanIgnoreScissor(const SkIRect& scissorRect, const SkRect& drawBounds) {
+        // This is the maximum distance that a draw may extend beyond a clip's scissor and still
+        // count as inside. We use a sloppy compare because the draw may have chosen its bounds in a
+        // different coord system. The rationale for 1e-3 is that in the coverage case (and barring
+        // unexpected rounding), as long as coverage stays below 0.5 * 1/256 we ought to be OK.
+        constexpr SkScalar fuzz = 1e-3f;
+        SkASSERT(!scissorRect.isEmpty());
+        SkASSERT(!drawBounds.isEmpty());
+        return scissorRect.fLeft <= drawBounds.fLeft + fuzz &&
+               scissorRect.fTop <= drawBounds.fTop + fuzz &&
+               scissorRect.fRight >= drawBounds.fRight - fuzz &&
+               scissorRect.fBottom >= drawBounds.fBottom - fuzz;
+    }
+
+    friend class GrClipMaskManager;
 };
 
 /**

diff --git a/src/gpu/GrClip.cpp b/src/gpu/GrClip.cpp
index b0577c5..ab2acb9 100644 (file)
--- a/src/gpu/GrClip.cpp
+++ b/src/gpu/GrClip.cpp
@@ -54,12 +54,14 @@ bool GrFixedClip::apply(GrContext*, const GrPipelineBuilder& pipelineBuilder,
         if (devBounds && !devBounds->intersects(SkRect::Make(tightScissor))) {
             return false;
         }
-        if (fHasStencilClip) {
-            out->makeScissoredStencil(tightScissor, &fDeviceBounds);
-        } else {
-            out->makeScissored(tightScissor);
+        if (!devBounds || !CanIgnoreScissor(fScissorState.rect(), *devBounds)) {
+            if (fHasStencilClip) {
+                out->makeScissoredStencil(tightScissor, &fDeviceBounds);
+            } else {
+                out->makeScissored(tightScissor);
+            }
+            return true;
         }
-        return true;
     }
 
     out->makeStencil(fHasStencilClip, fDeviceBounds);

diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index c591bf1..d710fef 100644 (file)
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -268,7 +268,7 @@ bool GrClipMaskManager::SetupClipping(GrContext* context,
         } else {
             SkIRect scissorSpaceIBounds(clipSpaceIBounds);
             scissorSpaceIBounds.offset(-clip.origin());
-            if (!SkRect::Make(scissorSpaceIBounds).contains(devBounds)) {
+            if (!GrClip::CanIgnoreScissor(scissorSpaceIBounds, devBounds)) {
                 out->makeScissored(scissorSpaceIBounds);
             }
             return true;
@@ -302,11 +302,11 @@ bool GrClipMaskManager::SetupClipping(GrContext* context,
                                         &clipFP)) {
             SkIRect scissorSpaceIBounds(clipSpaceIBounds);
             scissorSpaceIBounds.offset(-clip.origin());
-            if (!SkRect::Make(scissorSpaceIBounds).contains(devBounds)) {
+            if (GrClip::CanIgnoreScissor(scissorSpaceIBounds, devBounds)) {
+                out->makeFPBased(std::move(clipFP), SkRect::Make(scissorSpaceIBounds));
+            } else {
                 out->makeScissoredFPBased(std::move(clipFP), scissorSpaceIBounds);
-                return true;
             }
-            out->makeFPBased(std::move(clipFP), SkRect::Make(scissorSpaceIBounds));
             return true;
         }
     }
@@ -369,7 +369,11 @@ 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(scissorSpaceIBounds);
+    if (GrClip::CanIgnoreScissor(scissorSpaceIBounds, devBounds)) {
+        out->makeStencil(true, devBounds);
+    } else {
+        out->makeScissoredStencil(scissorSpaceIBounds);
+    }
     return true;
 }
 

diff --git a/src/gpu/GrDrawContext.cpp b/src/gpu/GrDrawContext.cpp
index a86e01e..8f012cd 100644 (file)
--- a/src/gpu/GrDrawContext.cpp
+++ b/src/gpu/GrDrawContext.cpp
@@ -270,17 +270,70 @@ static bool should_apply_coverage_aa(const GrPaint& paint, GrRenderTarget* rt,
     }
 }
 
+// Attempts to crop a rect and optional local rect to the clip boundaries.
+// Returns false if the draw can be skipped entirely.
+static bool crop_filled_rect(const GrRenderTarget* rt, const GrClip& clip,
+                             const SkMatrix& viewMatrix, SkRect* rect,
+                             SkRect* localRect = nullptr) {
+    if (!viewMatrix.rectStaysRect()) {
+        return true;
+    }
+
+    SkMatrix inverseViewMatrix;
+    if (!viewMatrix.invert(&inverseViewMatrix)) {
+        return false;
+    }
+
+    SkIRect clipDevBounds;
+    SkRect clipBounds;
+    SkASSERT(inverseViewMatrix.rectStaysRect());
+
+    clip.getConservativeBounds(rt->width(), rt->height(), &clipDevBounds);
+    inverseViewMatrix.mapRect(&clipBounds, SkRect::Make(clipDevBounds));
+
+    if (localRect) {
+        if (!rect->intersects(clipBounds)) {
+            return false;
+        }
+        const SkScalar dx = localRect->width() / rect->width();
+        const SkScalar dy = localRect->height() / rect->height();
+        if (clipBounds.fLeft > rect->fLeft) {
+            localRect->fLeft += (clipBounds.fLeft - rect->fLeft) * dx;
+            rect->fLeft = clipBounds.fLeft;
+        }
+        if (clipBounds.fTop > rect->fTop) {
+            localRect->fTop += (clipBounds.fTop - rect->fTop) * dy;
+            rect->fTop = clipBounds.fTop;
+        }
+        if (clipBounds.fRight < rect->fRight) {
+            localRect->fRight -= (rect->fRight - clipBounds.fRight) * dx;
+            rect->fRight = clipBounds.fRight;
+        }
+        if (clipBounds.fBottom < rect->fBottom) {
+            localRect->fBottom -= (rect->fBottom - clipBounds.fBottom) * dy;
+            rect->fBottom = clipBounds.fBottom;
+        }
+        return true;
+    }
+
+    return rect->intersect(clipBounds);
+}
+
 bool GrDrawContext::drawFilledRect(const GrClip& clip,
                                    const GrPaint& paint,
                                    const SkMatrix& viewMatrix,
                                    const SkRect& rect,
                                    const GrUserStencilSettings* ss) {
+    SkRect croppedRect = rect;
+    if (!crop_filled_rect(fRenderTarget.get(), clip, viewMatrix, &croppedRect)) {
+        return true;
+    }
 
     SkAutoTUnref<GrDrawBatch> batch;
     bool useHWAA;
 
     if (InstancedRendering* ir = this->getDrawTarget()->instancedRendering()) {
-        batch.reset(ir->recordRect(rect, viewMatrix, paint.getColor(),
+        batch.reset(ir->recordRect(croppedRect, viewMatrix, paint.getColor(),
                                    paint.isAntiAlias(), fInstancedPipelineInfo,
                                    &useHWAA));
         if (batch) {
@@ -297,10 +350,10 @@ bool GrDrawContext::drawFilledRect(const GrClip& clip,
         // The fill path can handle rotation but not skew.
         if (view_matrix_ok_for_aa_fill_rect(viewMatrix)) {
             SkRect devBoundRect;
-            viewMatrix.mapRect(&devBoundRect, rect);
+            viewMatrix.mapRect(&devBoundRect, croppedRect);
 
             batch.reset(GrRectBatchFactory::CreateAAFill(paint.getColor(), viewMatrix,
-                                                         rect, devBoundRect));
+                                                         croppedRect, devBoundRect));
             if (batch) {
                 GrPipelineBuilder pipelineBuilder(paint, useHWAA);
                 if (ss) {
@@ -311,7 +364,7 @@ bool GrDrawContext::drawFilledRect(const GrClip& clip,
             }
         }
     } else {
-        this->drawNonAAFilledRect(clip, paint, viewMatrix, rect, nullptr, nullptr, ss);
+        this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, nullptr, nullptr, ss);
         return true;
     }
 
@@ -522,12 +575,18 @@ void GrDrawContext::fillRectToRect(const GrClip& clip,
     SkDEBUGCODE(this->validate();)
     GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrDrawContext::fillRectToRect");
 
+    SkRect croppedRect = rectToDraw;
+    SkRect croppedLocalRect = localRect;
+    if (!crop_filled_rect(fRenderTarget.get(), clip, viewMatrix, &croppedRect, &croppedLocalRect)) {
+        return;
+    }
+
     AutoCheckFlush acf(fDrawingManager);
     SkAutoTUnref<GrDrawBatch> batch;
     bool useHWAA;
 
     if (InstancedRendering* ir = this->getDrawTarget()->instancedRendering()) {
-        batch.reset(ir->recordRect(rectToDraw, viewMatrix, paint.getColor(), localRect,
+        batch.reset(ir->recordRect(croppedRect, viewMatrix, paint.getColor(), croppedLocalRect,
                                    paint.isAntiAlias(), fInstancedPipelineInfo, &useHWAA));
         if (batch) {
             GrPipelineBuilder pipelineBuilder(paint, useHWAA);
@@ -538,15 +597,15 @@ void GrDrawContext::fillRectToRect(const GrClip& clip,
 
     if (should_apply_coverage_aa(paint, fRenderTarget.get(), &useHWAA) &&
         view_matrix_ok_for_aa_fill_rect(viewMatrix)) {
-        batch.reset(GrAAFillRectBatch::CreateWithLocalRect(paint.getColor(), viewMatrix, rectToDraw,
-                                                           localRect));
+        batch.reset(GrAAFillRectBatch::CreateWithLocalRect(paint.getColor(), viewMatrix,
+                                                           croppedRect, croppedLocalRect));
         if (batch) {
             GrPipelineBuilder pipelineBuilder(paint, useHWAA);
             this->drawBatch(pipelineBuilder, clip, batch);
             return;
         }
     } else {
-        this->drawNonAAFilledRect(clip, paint, viewMatrix, rectToDraw, &localRect,
+        this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, &croppedLocalRect,
                                   nullptr, nullptr);
     }
 
@@ -562,12 +621,17 @@ void GrDrawContext::fillRectWithLocalMatrix(const GrClip& clip,
     SkDEBUGCODE(this->validate();)
     GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrDrawContext::fillRectWithLocalMatrix");
 
+    SkRect croppedRect = rectToDraw;
+    if (!crop_filled_rect(fRenderTarget.get(), clip, viewMatrix, &croppedRect)) {
+        return;
+    }
+
     AutoCheckFlush acf(fDrawingManager);
     SkAutoTUnref<GrDrawBatch> batch;
     bool useHWAA;
 
     if (InstancedRendering* ir = this->getDrawTarget()->instancedRendering()) {
-        batch.reset(ir->recordRect(rectToDraw, viewMatrix, paint.getColor(), localMatrix,
+        batch.reset(ir->recordRect(croppedRect, viewMatrix, paint.getColor(), localMatrix,
                                    paint.isAntiAlias(), fInstancedPipelineInfo, &useHWAA));
         if (batch) {
             GrPipelineBuilder pipelineBuilder(paint, useHWAA);
@@ -579,11 +643,11 @@ void GrDrawContext::fillRectWithLocalMatrix(const GrClip& clip,
     if (should_apply_coverage_aa(paint, fRenderTarget.get(), &useHWAA) &&
         view_matrix_ok_for_aa_fill_rect(viewMatrix)) {
         batch.reset(GrAAFillRectBatch::Create(paint.getColor(), viewMatrix, localMatrix,
-                                              rectToDraw));
+                                              croppedRect));
         GrPipelineBuilder pipelineBuilder(paint, useHWAA);
         this->getDrawTarget()->drawBatch(pipelineBuilder, this, clip, batch);
     } else {
-        this->drawNonAAFilledRect(clip, paint, viewMatrix, rectToDraw, nullptr,
+        this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, nullptr,
                                   &localMatrix, nullptr);
     }