Make all parameters reqiured to GrReducedClip::ReduceClipStack

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

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

diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp
index cbfd896..26b8936 100644 (file)
--- a/src/gpu/GrReducedClip.cpp
+++ b/src/gpu/GrReducedClip.cpp
@@ -302,9 +302,7 @@ static void reduced_stack_walker(const SkClipStack& stack,
             }
         }
     }
-    if (requiresAA) {
-        *requiresAA = numAAElements > 0;
-    }
+    *requiresAA = numAAElements > 0;
 
     if (0 == result->count()) {
         if (*initialState == GrReducedClip::kAllIn_InitialState) {
@@ -329,6 +327,8 @@ void GrReducedClip::ReduceClipStack(const SkClipStack& stack,
                                     InitialState* initialState,
                                     SkIRect* tighterBounds,
                                     bool* requiresAA) {
+    SkASSERT(tighterBounds);
+    SkASSERT(requiresAA);
     result->reset();
 
     // The clip established by the element list might be cached based on the last
@@ -359,60 +359,44 @@ void GrReducedClip::ReduceClipStack(const SkClipStack& stack,
         SkRect isectRect;
         if (stackBounds.contains(scalarQueryBounds)) {
             *initialState = GrReducedClip::kAllIn_InitialState;
-            if (tighterBounds) {
-                *tighterBounds = queryBounds;
-            }
-            if (requiresAA) {
-               *requiresAA = false;
-            }
+            *tighterBounds = queryBounds;
+            *requiresAA = false;
         } else if (isectRect.intersect(stackBounds, scalarQueryBounds)) {
             // If the caller asked for tighter integer bounds we may be able to
             // return kAllIn and give the bounds with no elements
-            if (tighterBounds) {
-                isectRect.roundOut(tighterBounds);
-                SkRect scalarTighterBounds = SkRect::Make(*tighterBounds);
-                if (scalarTighterBounds == isectRect) {
-                    // the round-out didn't add any area outside the clip rect.
-                    if (requiresAA) {
-                        *requiresAA = false;
-                    }
-                    *initialState = GrReducedClip::kAllIn_InitialState;
-                    return;
-                }
+            isectRect.roundOut(tighterBounds);
+            SkRect scalarTighterBounds = SkRect::Make(*tighterBounds);
+            if (scalarTighterBounds == isectRect) {
+                // the round-out didn't add any area outside the clip rect.
+                *requiresAA = false;
+                *initialState = GrReducedClip::kAllIn_InitialState;
+                return;
             }
             *initialState = kAllOut_InitialState;
             // iior should only be true if aa/non-aa status matches among all elements.
             SkClipStack::Iter iter(stack, SkClipStack::Iter::kTop_IterStart);
             bool doAA = iter.prev()->isAA();
             result->addToHead(isectRect, SkRegion::kReplace_Op, doAA);
-            if (requiresAA) {
-                *requiresAA = doAA;
-            }
+            *requiresAA = doAA;
         } else {
             *initialState = kAllOut_InitialState;
-             if (requiresAA) {
-                *requiresAA = false;
-             }
+            *requiresAA = false;
         }
         return;
     } else {
         if (SkClipStack::kNormal_BoundsType == stackBoundsType) {
             if (!SkRect::Intersects(stackBounds, scalarQueryBounds)) {
                 *initialState = kAllOut_InitialState;
-                if (requiresAA) {
-                   *requiresAA = false;
-                }
+               *requiresAA = false;
                 return;
             }
-            if (tighterBounds) {
-                SkIRect stackIBounds;
-                stackBounds.roundOut(&stackIBounds);
-                if (!tighterBounds->intersect(queryBounds, stackIBounds)) {
-                    SkASSERT(0);
-                    tighterBounds->setEmpty();
-                }
-                bounds = tighterBounds;
+            SkIRect stackIBounds;
+            stackBounds.roundOut(&stackIBounds);
+            if (!tighterBounds->intersect(queryBounds, stackIBounds)) {
+                SkASSERT(0);
+                tighterBounds->setEmpty();
             }
+            bounds = tighterBounds;
         } else {
             if (stackBounds.contains(scalarQueryBounds)) {
                 *initialState = kAllOut_InitialState;
@@ -420,9 +404,7 @@ void GrReducedClip::ReduceClipStack(const SkClipStack& stack,
                 // but we don't know that *all* the pixels in the box are outside the clip. So
                 // proceed to walking the stack.
             }
-            if (tighterBounds) {
-                *tighterBounds = queryBounds;
-            }
+            *tighterBounds = queryBounds;
         }
     }
 

diff --git a/src/gpu/GrReducedClip.h b/src/gpu/GrReducedClip.h
index 3a7c173..da0bae6 100644 (file)
--- a/src/gpu/GrReducedClip.h
+++ b/src/gpu/GrReducedClip.h
@@ -26,24 +26,19 @@ public:
      * full stack to the rectangle. The clip stack generation id that represents
      * the list of elements is returned in resultGenID. The initial state of the
      * query rectangle before the first clip element is applied is returned via
-     * initialState. Optionally, the caller can request a tighter bounds on the
-     * clip be returned via tighterBounds. If not nullptr, tighterBounds will
-     * always be contained by queryBounds after return. If tighterBounds is
-     * specified then it is assumed that the caller will implicitly clip against
-     * it. If the caller specifies non-nullptr for requiresAA then it will indicate
-     * whether anti-aliasing is required to process any of the elements in the
-     * result.
-     *
-     * This may become a member function of SkClipStack when its interface is
-     * determined to be stable.
+     * initialState. The reducer output tighterBounds is a tighter bounds on the
+     * clip. tighterBounds will always be contained by queryBounds after return.
+     * It is assumed that the caller will not draw outside of tighterBounds.
+     * The requiresAA output will indicate whether anti-aliasing is required to
+     * process any of the elements in the element list result.
      */
     static void ReduceClipStack(const SkClipStack& stack,
                                 const SkIRect& queryBounds,
                                 ElementList* result,
                                 int32_t* resultGenID,
                                 InitialState* initialState,
-                                SkIRect* tighterBounds = nullptr,
-                                bool* requiresAA = nullptr);
+                                SkIRect* tighterBounds,
+                                bool* requiresAA);
 };
 
 #endif

diff --git a/tests/ClipStackTest.cpp b/tests/ClipStackTest.cpp
index c2ad21b..27753c4 100644 (file)
--- a/tests/ClipStackTest.cpp
+++ b/tests/ClipStackTest.cpp
@@ -963,14 +963,15 @@ static void test_reduced_clip_stack(skiatest::Reporter* reporter) {
         ElementList reducedClips;
         int32_t reducedGenID;
         GrReducedClip::InitialState initial;
-        SkIRect tBounds(inflatedIBounds);
-        SkIRect* tightBounds = r.nextBool() ? &tBounds : nullptr;
+        SkIRect tighterBounds;
+        bool requiresAA;
         GrReducedClip::ReduceClipStack(stack,
                                        inflatedIBounds,
                                        &reducedClips,
                                        &reducedGenID,
                                        &initial,
-                                       tightBounds);
+                                       &tighterBounds,
+                                       &requiresAA);
 
         REPORTER_ASSERT(reporter, SkClipStack::kInvalidGenID != reducedGenID);
 
@@ -985,9 +986,8 @@ static void test_reduced_clip_stack(skiatest::Reporter* reporter) {
         }
 
         // GrReducedClipStack assumes that the final result is clipped to the returned bounds
-        if (tightBounds) {
-            reducedStack.clipDevRect(*tightBounds, SkRegion::kIntersect_Op);
-        }
+        reducedStack.clipDevRect(tighterBounds, SkRegion::kIntersect_Op);
+        stack.clipDevRect(tighterBounds, SkRegion::kIntersect_Op);
 
         // convert both the original stack and reduced stack to SkRegions and see if they're equal
         SkRegion region;
@@ -1028,13 +1028,15 @@ static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
         int32_t reducedGenID;
         GrReducedClip::InitialState initial;
         SkIRect tightBounds;
+        bool requiresAA;
 
         GrReducedClip::ReduceClipStack(stack,
                                        inflatedIBounds,
                                        &reducedClips,
                                        &reducedGenID,
                                        &initial,
-                                       &tightBounds);
+                                       &tightBounds,
+                                       &requiresAA);
 
         REPORTER_ASSERT(reporter, reducedClips.count() == 1);
         // Clips will be cached based on the generation id. Make sure the gen id is valid.
@@ -1059,8 +1061,6 @@ static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
 
 #define XYWH SkIRect::MakeXYWH
 
-        SkIRect unused;
-        unused.setEmpty();
         SkIRect stackBounds = XYWH(0, 0, 76, 76);
 
         // The base test is to test each rect in two ways:
@@ -1081,38 +1081,27 @@ static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
         } testCases[] = {
             // Rect A.
             { XYWH(0, 0, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::kAllIn_InitialState, XYWH(0, 0, 25, 25) },
-            { XYWH(0, 0, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::kAllIn_InitialState, unused },
             { XYWH(0, 0, 27, 27), 1, genIDA, GrReducedClip::kAllOut_InitialState, XYWH(0, 0, 27, 27)},
-            { XYWH(0, 0, 27, 27), 1, genIDA, GrReducedClip::kAllOut_InitialState, unused },
 
             // Rect B.
             { XYWH(50, 0, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::kAllIn_InitialState, XYWH(50, 0, 25, 25) },
-            { XYWH(50, 0, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::kAllIn_InitialState, unused },
             { XYWH(50, 0, 27, 27), 1, genIDB, GrReducedClip::kAllOut_InitialState, XYWH(50, 0, 26, 27) },
-            { XYWH(50, 0, 27, 27), 1, genIDB, GrReducedClip::kAllOut_InitialState, unused },
 
             // Rect C.
             { XYWH(0, 50, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::kAllIn_InitialState, XYWH(0, 50, 25, 25) },
-            { XYWH(0, 50, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::kAllIn_InitialState, unused },
             { XYWH(0, 50, 27, 27), 1, genIDC, GrReducedClip::kAllOut_InitialState, XYWH(0, 50, 27, 26) },
-            { XYWH(0, 50, 27, 27), 1, genIDC, GrReducedClip::kAllOut_InitialState, unused },
 
             // Rect D.
-            { XYWH(50, 50, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::kAllIn_InitialState, unused },
             { XYWH(50, 50, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::kAllIn_InitialState, XYWH(50, 50, 25, 25)},
-            { XYWH(50, 50, 27, 27), 1, genIDD, GrReducedClip::kAllOut_InitialState, unused },
             { XYWH(50, 50, 27, 27), 1, genIDD, GrReducedClip::kAllOut_InitialState,  XYWH(50, 50, 26, 26)},
 
             // Other tests:
-            { XYWH(0, 0, 100, 100), 4, genIDD, GrReducedClip::kAllOut_InitialState, unused },
             { XYWH(0, 0, 100, 100), 4, genIDD, GrReducedClip::kAllOut_InitialState, stackBounds },
 
             // Rect in the middle, touches none.
-            { XYWH(26, 26, 24, 24), 0, SkClipStack::kEmptyGenID, GrReducedClip::kAllOut_InitialState, unused },
             { XYWH(26, 26, 24, 24), 0, SkClipStack::kEmptyGenID, GrReducedClip::kAllOut_InitialState, XYWH(26, 26, 24, 24) },
 
             // Rect in the middle, touches all the rects. GenID is the last rect.
-            { XYWH(24, 24, 27, 27), 4, genIDD, GrReducedClip::kAllOut_InitialState, unused },
             { XYWH(24, 24, 27, 27), 4, genIDD, GrReducedClip::kAllOut_InitialState, XYWH(24, 24, 27, 27) },
         };
 
@@ -1123,13 +1112,15 @@ static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
             int32_t reducedGenID;
             GrReducedClip::InitialState initial;
             SkIRect tightBounds;
+            bool requiresAA;
 
             GrReducedClip::ReduceClipStack(stack,
                                            testCases[i].testBounds,
                                            &reducedClips,
                                            &reducedGenID,
                                            &initial,
-                                           testCases[i].tighterBounds.isEmpty() ? nullptr : &tightBounds);
+                                           &tightBounds,
+                                           &requiresAA);
 
             REPORTER_ASSERT(reporter, reducedClips.count() == testCases[i].reducedClipCount);
             SkASSERT(reducedClips.count() == testCases[i].reducedClipCount);
@@ -1137,10 +1128,8 @@ static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
             SkASSERT(reducedGenID == testCases[i].reducedGenID);
             REPORTER_ASSERT(reporter, initial == testCases[i].initialState);
             SkASSERT(initial == testCases[i].initialState);
-            if (!testCases[i].tighterBounds.isEmpty()) {
-                REPORTER_ASSERT(reporter, tightBounds == testCases[i].tighterBounds);
-                SkASSERT(tightBounds == testCases[i].tighterBounds);
-            }
+            REPORTER_ASSERT(reporter, tightBounds == testCases[i].tighterBounds);
+            SkASSERT(tightBounds == testCases[i].tighterBounds);
         }
     }
 }
@@ -1155,6 +1144,7 @@ static void test_reduced_clip_stack_no_aa_crash(skiatest::Reporter* reporter) {
     int32_t reducedGenID;
     GrReducedClip::InitialState initial;
     SkIRect tightBounds;
+    bool requiresAA;
 
     // At the time, this would crash.
     GrReducedClip::ReduceClipStack(stack,
@@ -1162,7 +1152,8 @@ static void test_reduced_clip_stack_no_aa_crash(skiatest::Reporter* reporter) {
                                    &reducedClips,
                                    &reducedGenID,
                                    &initial,
-                                   &tightBounds);
+                                   &tightBounds,
+                                   &requiresAA);
 
     REPORTER_ASSERT(reporter, 0 == reducedClips.count());
 }