Revert of Pre-crop filled rects to avoid scissor (patchset #6 id:100001 of https...

Reason for revert:
I believe that this is breaking the roll.

https://codereview.chromium.org/2141923002

https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/258434/steps/cc_unittests%20%28with%20patch%29%20on%20Mac-10.9/logs/stdio

Original issue's description:
> 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

TBR=bsalomon@google.com,robertphillips@google.com,csmartdalton@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

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

diff --git a/gm/croppedrects.cpp b/gm/croppedrects.cpp
deleted file mode 100644 (file)
index 5c36c54..0000000
--- a/gm/croppedrects.cpp
+++ /dev/null
@@ -1,108 +0,0 @@
-/*
- * 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 b63f213..e585281 100644 (file)
--- a/include/gpu/GrClip.h
+++ b/include/gpu/GrClip.h
@@ -109,26 +109,6 @@ 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 ab2acb9..b0577c5 100644 (file)
--- a/src/gpu/GrClip.cpp
+++ b/src/gpu/GrClip.cpp
@@ -54,14 +54,12 @@ bool GrFixedClip::apply(GrContext*, const GrPipelineBuilder& pipelineBuilder,
         if (devBounds && !devBounds->intersects(SkRect::Make(tightScissor))) {
             return false;
         }
-        if (!devBounds || !CanIgnoreScissor(fScissorState.rect(), *devBounds)) {
-            if (fHasStencilClip) {
-                out->makeScissoredStencil(tightScissor, &fDeviceBounds);
-            } else {
-                out->makeScissored(tightScissor);
-            }
-            return true;
+        if (fHasStencilClip) {
+            out->makeScissoredStencil(tightScissor, &fDeviceBounds);
+        } else {
+            out->makeScissored(tightScissor);
         }
+        return true;
     }
 
     out->makeStencil(fHasStencilClip, fDeviceBounds);

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

diff --git a/src/gpu/GrDrawContext.cpp b/src/gpu/GrDrawContext.cpp
index 8f012cd..a86e01e 100644 (file)
--- a/src/gpu/GrDrawContext.cpp
+++ b/src/gpu/GrDrawContext.cpp
@@ -270,70 +270,17 @@ 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(croppedRect, viewMatrix, paint.getColor(),
+        batch.reset(ir->recordRect(rect, viewMatrix, paint.getColor(),
                                    paint.isAntiAlias(), fInstancedPipelineInfo,
                                    &useHWAA));
         if (batch) {
@@ -350,10 +297,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, croppedRect);
+            viewMatrix.mapRect(&devBoundRect, rect);
 
             batch.reset(GrRectBatchFactory::CreateAAFill(paint.getColor(), viewMatrix,
-                                                         croppedRect, devBoundRect));
+                                                         rect, devBoundRect));
             if (batch) {
                 GrPipelineBuilder pipelineBuilder(paint, useHWAA);
                 if (ss) {
@@ -364,7 +311,7 @@ bool GrDrawContext::drawFilledRect(const GrClip& clip,
             }
         }
     } else {
-        this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, nullptr, nullptr, ss);
+        this->drawNonAAFilledRect(clip, paint, viewMatrix, rect, nullptr, nullptr, ss);
         return true;
     }
 
@@ -575,18 +522,12 @@ 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(croppedRect, viewMatrix, paint.getColor(), croppedLocalRect,
+        batch.reset(ir->recordRect(rectToDraw, viewMatrix, paint.getColor(), localRect,
                                    paint.isAntiAlias(), fInstancedPipelineInfo, &useHWAA));
         if (batch) {
             GrPipelineBuilder pipelineBuilder(paint, useHWAA);
@@ -597,15 +538,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,
-                                                           croppedRect, croppedLocalRect));
+        batch.reset(GrAAFillRectBatch::CreateWithLocalRect(paint.getColor(), viewMatrix, rectToDraw,
+                                                           localRect));
         if (batch) {
             GrPipelineBuilder pipelineBuilder(paint, useHWAA);
             this->drawBatch(pipelineBuilder, clip, batch);
             return;
         }
     } else {
-        this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, &croppedLocalRect,
+        this->drawNonAAFilledRect(clip, paint, viewMatrix, rectToDraw, &localRect,
                                   nullptr, nullptr);
     }
 
@@ -621,17 +562,12 @@ 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(croppedRect, viewMatrix, paint.getColor(), localMatrix,
+        batch.reset(ir->recordRect(rectToDraw, viewMatrix, paint.getColor(), localMatrix,
                                    paint.isAntiAlias(), fInstancedPipelineInfo, &useHWAA));
         if (batch) {
             GrPipelineBuilder pipelineBuilder(paint, useHWAA);
@@ -643,11 +579,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,
-                                              croppedRect));
+                                              rectToDraw));
         GrPipelineBuilder pipelineBuilder(paint, useHWAA);
         this->getDrawTarget()->drawBatch(pipelineBuilder, this, clip, batch);
     } else {
-        this->drawNonAAFilledRect(clip, paint, viewMatrix, croppedRect, nullptr,
+        this->drawNonAAFilledRect(clip, paint, viewMatrix, rectToDraw, nullptr,
                                   &localMatrix, nullptr);
     }