Make GrReducedClip's gen ID only apply to the element list

src/gpu/GrReducedClip.cpp

 /*
  * 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 "GrReducedClip.h"
 
 #include "GrClip.h"
@@ skipping 300 matching lines @@
                 }
                 result->popHead();
                 element = result->headIter().get();
             }
         }
     }
     *requiresAA = numAAElements > 0;
 
     if (0 == result->count()) {
         if (initialState == InitialTriState::kAllIn) {
             *resultGenID = SkClipStack::kWideOpenGenID;

[csmartdalton, 2016/08/16 05:18:49]

In fact, this can be wrong if you have a clip with an outer pixel aligned rect:
ibounds() takes care of the outer rect so we skip it. Is it really correct for
fGenID to be kWideOpen? Should we just always let it be the top element on the
stack?

[bsalomon, 2016/08/16 14:14:40]

If I follow, I think it is correct. I think the function is documented to say
that the caller must not draw outside of the ibounds. That may not be the best
interface, though.

         } else {
             *resultGenID = SkClipStack::kEmptyGenID;
         }
     }
 
     SkASSERT(SkClipStack::kInvalidGenID != *resultGenID);
     SkASSERT(InitialTriState::kUnknown != initialState);
     return static_cast<GrReducedClip::InitialState>(initialState);
 }
 
 /*
 There are plenty of optimizations that could be added here. Maybe flips could be folded into
 earlier operations. Or would inserting flips and reversing earlier ops ever be a win? Perhaps
 for the case where the bounds are kInsideOut_BoundsType. We could restrict earlier operations
 based on later intersect operations, and perhaps remove intersect-rects. We could optionally
 take a rect in case the caller knows a bound on what is to be drawn through this clip.
 */
 GrReducedClip::GrReducedClip(const SkClipStack& stack, const SkRect& queryBounds) {
     SkASSERT(!queryBounds.isEmpty());
 
     // The clip established by the element list might be cached based on the last
     // generation id. When we make early returns, we do not know what was the generation
     // id that lead to the state. Make a conservative guess.
     fGenID = stack.getTopmostGenID();
     fHasIBounds = false;
 
     if (stack.isWideOpen()) {
+        fGenID = SkClipStack::kWideOpenGenID;
         fInitialState = InitialState::kAllIn;
         return;
     }
 
     SkClipStack::BoundsType stackBoundsType;
     SkRect stackBounds;
     bool iior;
     stack.getBounds(&stackBounds, &stackBoundsType, &iior);
 
     if (stackBounds.isEmpty() || GrClip::IsOutsideClip(stackBounds, queryBounds)) {
-        bool insideOut = SkClipStack::kInsideOut_BoundsType == stackBoundsType;
-        fInitialState = insideOut ? InitialState::kAllIn : InitialState::kAllOut;
+        if (SkClipStack::kInsideOut_BoundsType == stackBoundsType) {
+            fGenID = SkClipStack::kWideOpenGenID;
+            fInitialState = InitialState::kAllIn;
+        } else {
+            fGenID = SkClipStack::kEmptyGenID;
+            fInitialState = InitialState::kAllOut;
+        }
         return;
     }
 
     if (iior) {
         // "Is intersection of rects" means the clip is a single rect indicated by the stack bounds.
-        // This should only be true if aa/non-aa status matches among all elements.
         SkASSERT(SkClipStack::kNormal_BoundsType == stackBoundsType);
+        if (GrClip::IsInsideClip(stackBounds, queryBounds)) {
+            fGenID = SkClipStack::kWideOpenGenID;
+            fInitialState = InitialState::kAllIn;
+            return;
+        }
+        // iior should only be true if aa/non-aa status matches among all elements.
         SkClipStack::Iter iter(stack, SkClipStack::Iter::kTop_IterStart);
         if (!iter.prev()->isAA() || GrClip::IsPixelAligned(stackBounds)) {
             // The clip is a non-aa rect. This is the one spot where we can actually implement the
             // clip (using fIBounds) rather than just telling the caller what it should be.
             stackBounds.round(&fIBounds);
-            fHasIBounds = true;
-            fInitialState = fIBounds.isEmpty() ? InitialState::kAllOut : InitialState::kAllIn;
-            return;
-        }
-        if (GrClip::IsInsideClip(stackBounds, queryBounds)) {
-            fInitialState = InitialState::kAllIn;
+            if (fIBounds.isEmpty()) {
+                fGenID = SkClipStack::kEmptyGenID;
+                fInitialState = InitialState::kAllOut;
+            } else {
+                fHasIBounds = true;
+                fInitialState = InitialState::kAllIn;
+            }
             return;
         }
 
         SkRect tightBounds;
         SkAssertResult(tightBounds.intersect(stackBounds, queryBounds));
         fIBounds = GrClip::GetPixelIBounds(tightBounds);
         SkASSERT(!fIBounds.isEmpty()); // Empty should have been blocked by IsOutsideClip above.
         fHasIBounds = true;
 
         // Implement the clip with an AA rect element.
@@ skipping 13 matching lines @@
 
     fIBounds = GrClip::GetPixelIBounds(tighterQuery);
     SkASSERT(!fIBounds.isEmpty()); // Empty should have been blocked by IsOutsideClip above.
     fHasIBounds = true;
 
     // Now that we have determined the bounds to use and filtered out the trivial cases, call the
     // helper that actually walks the stack.
     fInitialState = reduced_stack_walker(stack, tighterQuery, fIBounds, &fElements, &fGenID,
                                          &fRequiresAA);
 }