Revert of Remove the AA requirement for selecting GrEffect-based clipping. (https...

Reason for revert:
We think this is breaking the roll.

Original issue's description:
> Remove the AA requirement for selecting GrEffect-based clipping.
>
> Also, optionally use the scissor for a bw clip rect element instead of an effect.
>
> Committed: https://skia.googlesource.com/skia/+/a73218bbbdcbe458651d10815e8d3b73d71b8e11
>
> Committed: https://skia.googlesource.com/skia/+/e9a729cb4d3f05b9c750dc1f63a9cc65b5659f04

R=robertphillips@google.com, bsalomon@google.com
TBR=bsalomon@google.com, robertphillips@google.com
NOTREECHECKS=true
NOTRY=true

Author: mtklein@google.com

Review URL: https://codereview.chromium.org/361313002

diff --git a/expectations/gm/ignored-tests.txt b/expectations/gm/ignored-tests.txt
index fcd53c8e487b9d01ea5a846db974dc5eec576c8c..03afd61d333948a92dfbcfb127ded936cb70e8df 100644 (file)
--- a/expectations/gm/ignored-tests.txt
+++ b/expectations/gm/ignored-tests.txt
@@ -139,10 +139,3 @@ poly2poly
 perlinnoise
 perlinnoise_localmatrix
 imagefiltersscaled
-
-# bsalomon:
-# Slight clip changes.
-complexclip2
-rrect_clip_bw
-circular-clips
-filltypespersp

diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index d3c0cf3fc140ec3cc1c6cd4a6816bb41f4e4202a..a878894c59949410cf2c0522e766657f23c06336 100644 (file)
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -110,10 +110,7 @@ bool GrClipMaskManager::useSWOnlyPath(const ElementList& elements) {
 bool GrClipMaskManager::installClipEffects(const ElementList& elements,
                                            GrDrawState::AutoRestoreEffects* are,
                                            const SkVector& clipToRTOffset,
-                                           const SkRect* drawBounds,
-                                           SkIRect* scissorRect) {
-
-    SkASSERT(NULL != scissorRect);
+                                           const SkRect* drawBounds) {
 
     GrDrawState* drawState = fGpu->drawState();
     SkRect boundsInClipSpace;
@@ -124,11 +121,7 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
 
     are->set(drawState);
     GrRenderTarget* rt = drawState->getRenderTarget();
-    // We iterate from the top of the stack to the bottom. We do this because we select the first
-    // BW rectangle as the scissor. Clients performing hierarchical rendering tend to use smaller
-    // clips towards the top of the clip stack. Smaller scissor rects can help tiled architectures
-    // skip processing tiles for draws.
-    ElementList::Iter iter(elements, ElementList::Iter::kTail_IterStart);
+    ElementList::Iter iter(elements);
 
     bool setARE = false;
     bool failed = false;
@@ -164,7 +157,7 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
             GrEffectEdgeType edgeType;
             if (GR_AA_CLIP && iter.get()->isAA()) {
                 if (rt->isMultisampled()) {
-                    // Coverage based AA clips don't play nicely with MSAA.
+                    // Coverage based AA clips don't place nicely with MSAA.
                     failed = true;
                     break;
                 }
@@ -172,8 +165,6 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
             } else {
                 edgeType = invert ? kInverseFillBW_GrEffectEdgeType : kFillBW_GrEffectEdgeType;
             }
-            // We don't want to exit if we convert a BW rect clip to a scissor.
-            bool failIfNoEffect = true;
             SkAutoTUnref<GrEffectRef> effect;
             switch (iter.get()->getType()) {
                 case SkClipStack::Element::kPath_Type:
@@ -189,14 +180,7 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
                 case SkClipStack::Element::kRect_Type: {
                     SkRect rect = iter.get()->getRect();
                     rect.offset(clipToRTOffset.fX, clipToRTOffset.fY);
-                    if (kFillBW_GrEffectEdgeType == edgeType && scissorRect->isEmpty()) {
-                        // This is OK because we only allow clip operations that shrink the clip
-                        // to be implemented as effects.
-                        rect.roundOut(scissorRect);
-                        failIfNoEffect = false;
-                    } else {
-                        effect.reset(GrConvexPolyEffect::Create(edgeType, rect));
-                    }
+                    effect.reset(GrConvexPolyEffect::Create(edgeType, rect));
                     break;
                 }
                 default:
@@ -208,12 +192,12 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
                     setARE = true;
                 }
                 fGpu->drawState()->addCoverageEffect(effect);
-            } else if (failIfNoEffect) {
+            } else {
                 failed = true;
                 break;
             }
         }
-        iter.prev();
+        iter.next();
     }
 
     if (failed) {
@@ -223,12 +207,6 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
     return !failed;
 }
 
-static inline bool rect_contains_irect(const SkIRect ir, const SkRect& r) {
-    SkASSERT(!ir.isEmpty());
-    return ir.fLeft <= r.fLeft && ir.fTop <= r.fTop &&
-           ir.fRight >= r.fRight && ir.fBottom >= r.fBottom;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // sort out what kind of clip mask needs to be created: alpha, stencil,
 // scissor, or entirely software
@@ -285,32 +263,17 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn,
     // configuration's relative costs of switching RTs to generate a mask vs
     // longer shaders.
     if (elements.count() <= 4) {
-        SkIRect scissorRect;
-        scissorRect.setEmpty();
         SkVector clipToRTOffset = { SkIntToScalar(-clipDataIn->fOrigin.fX),
                                     SkIntToScalar(-clipDataIn->fOrigin.fY) };
         if (elements.isEmpty() ||
-            this->installClipEffects(elements, are, clipToRTOffset, devBounds, &scissorRect)) {
-            if (scissorRect.isEmpty()) {
-                // We may still want to use a scissor, especially on tiled architectures.
-                scissorRect = clipSpaceIBounds;
-                scissorRect.offset(-clipDataIn->fOrigin);
-                if (NULL == devBounds ||
-                    !rect_contains_irect(scissorRect, *devBounds)) {
-                    fGpu->enableScissor(scissorRect);
-                } else {
-                    // When the vertices that will be rendered fit fully inside the clip's bounds
-                    // then providing the scissor rect will not help the driver eliminate tiles
-                    // from consideration for the draw, but changing the scissor will cause
-                    // state changes between draws.
-                    fGpu->disableScissor();
-                }
+            (requiresAA && this->installClipEffects(elements, are, clipToRTOffset, devBounds))) {
+            SkIRect scissorSpaceIBounds(clipSpaceIBounds);
+            scissorSpaceIBounds.offset(-clipDataIn->fOrigin);
+            if (NULL == devBounds ||
+                !SkRect::Make(scissorSpaceIBounds).contains(*devBounds)) {
+                fGpu->enableScissor(scissorSpaceIBounds);
             } else {
-                scissorRect.fLeft = SkTMax(0, scissorRect.fLeft);
-                scissorRect.fTop = SkTMax(0, scissorRect.fTop);
-                scissorRect.fRight = SkTMin(rt->width(), scissorRect.fRight);
-                scissorRect.fBottom = SkTMin(rt->height(), scissorRect.fBottom);
-                fGpu->enableScissor(scissorRect);
+                fGpu->disableScissor();
             }
             this->setGpuStencil();
             return true;

diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h
index 974c217705c28609f121f4c15719937b11dbbada..c3a21fd8a2b1e9d6771ebde48bbdaa2731b28b85 100644 (file)
--- a/src/gpu/GrClipMaskManager.h
+++ b/src/gpu/GrClipMaskManager.h
@@ -106,14 +106,11 @@ private:
     GrClipMaskCache fAACache;       // cache for the AA path
 
     // Attempts to install a series of coverage effects to implement the clip. Return indicates
-    // whether the element list was successfully converted to effects. One of the elements may
-    // be selected to use the scissor. If so scissorRect will be updated to a valid rectangle
-    // that the caller should set as the scissor rect. If not, scissorRect won't be modified.
+    // whether the element list was successfully converted to effects.
     bool installClipEffects(const GrReducedClip::ElementList&,
                             GrDrawState::AutoRestoreEffects*,
                             const SkVector& clipOffset,
-                            const SkRect* devBounds,
-                            SkIRect* scissorRect);
+                            const SkRect* devBounds);
 
     // Draws the clip into the stencil buffer
     bool createStencilClipMask(int32_t elementsGenID,