commit de00dc900427203d3a433dfd6fe1dca5b5b333d3
author Michael Ludwig <michaelludwig@google.com> Fri Jun 05 10:56:32 2020 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Jun 05 15:34:59 2020 +0000
tree 51974e8a536e49d3d3586e5e5caf243e3c8a396e
parent b96995d05f45da9c4c9d0861a43ff03057ff2ba1

Detect empty clip when difference op clips everything

Change-Id: Ifbe0f3ae6e01d65f18351903da8aef63170ce6c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/294457
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

src/gpu/GrReducedClip.cpp

diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp
index 313f1ff..93c01b6 100644
--- a/src/gpu/GrReducedClip.cpp
+++ b/src/gpu/GrReducedClip.cpp
@@ -115,6 +115,13 @@
         // Now that we have determined the bounds to use and filtered out the trivial cases, call
         // the helper that actually walks the stack.
         this->walkStack(stack, tighterQuery);
+
+        if (fInitialState == InitialState::kAllOut && fMaskElements.isEmpty()) {
+            // The clip starts with no coverage and there are no elements to add coverage with
+            // expanding ops. We ignore the AAClipRectGenID since it is an implied intersection.
+            this->makeEmpty();
+            return;
+        }
     }
 
     if (SK_InvalidGenID != fAAClipRectGenID && // Is there an AA clip rect?

tests/ClipStackTest.cpp

diff --git a/tests/ClipStackTest.cpp b/tests/ClipStackTest.cpp
index 1b4979d..eaaf4cc 100644
--- a/tests/ClipStackTest.cpp
+++ b/tests/ClipStackTest.cpp
@@ -924,6 +924,13 @@
     stack->clipPath(path, SkMatrix::I(), op, doAA);
 };
 
+static void add_noop(const SkRect& rect, bool invert, SkClipOp op, SkClipStack* stack,
+                     bool doAA) {
+    // FIXME (michaelludwig) - this will be replaced with add_shader, but this lets us tickle a
+    // bug in GrReducedClip due to the particular set of random clips the test generated, which
+    // requires an additional clip element type.
+};
+
 static void add_elem_to_stack(const SkClipStack::Element& element, SkClipStack* stack) {
     switch (element.getDeviceSpaceType()) {
         case SkClipStack::Element::DeviceSpaceType::kRect:
@@ -986,6 +993,7 @@
         add_rect,
         add_round_rect,
         add_oval,
+        add_noop,
     };
 
     SkRandom r;
@@ -1192,8 +1200,8 @@
             // Other tests:
             { XYWH(0, 0, 100, 100), 4, genIDD, GrReducedClip::InitialState::kAllOut, stackBounds },
 
-            // Rect in the middle, touches none.
-            { XYWH(26, 26, 24, 24), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllOut, IXYWH(26, 26, 24, 24) },
+            // Rect in the middle, touches none (so should not be drawn)
+            { XYWH(26, 26, 24, 24), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllOut, SkIRect::MakeEmpty() },
 
             // Rect in the middle, touches all the rects. GenID is the last rect.
             { XYWH(24, 24, 27, 27), 4, genIDD, GrReducedClip::InitialState::kAllOut, IXYWH(24, 24, 27, 27) },
@@ -1208,17 +1216,16 @@
             const GrReducedClip reduced(stack, testCases[i].testBounds, caps);
             REPORTER_ASSERT(reporter, reduced.maskElements().count() ==
                             testCases[i].reducedClipCount);
-            SkASSERT(reduced.maskElements().count() == testCases[i].reducedClipCount);
             if (reduced.maskElements().count()) {
                 REPORTER_ASSERT(reporter, reduced.maskGenID() == testCases[i].reducedGenID);
-                SkASSERT(reduced.maskGenID() == testCases[i].reducedGenID);
             }
             REPORTER_ASSERT(reporter, reduced.initialState() == testCases[i].initialState);
-            SkASSERT(reduced.initialState() == testCases[i].initialState);
-            REPORTER_ASSERT(reporter, reduced.hasScissor());
-            SkASSERT(reduced.hasScissor());
-            REPORTER_ASSERT(reporter, reduced.scissor() == testCases[i].clipIRect);
-            SkASSERT(reduced.scissor() == testCases[i].clipIRect);
+
+            bool expectsScissor = !testCases[i].clipIRect.isEmpty();
+            REPORTER_ASSERT(reporter, expectsScissor == reduced.hasScissor());
+            if (expectsScissor) {
+                REPORTER_ASSERT(reporter, reduced.scissor() == testCases[i].clipIRect);
+            }
         }
     }
 }