Make GrReducedClip's gen ID only apply to the element list
authorcsmartdalton <csmartdalton@google.com>
Wed, 17 Aug 2016 16:39:38 +0000 (09:39 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 17 Aug 2016 16:39:38 +0000 (09:39 -0700)
Renames fGenID to fElementsGenID and designates this value as
undefined when when the element list is empty.

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

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

src/gpu/GrClipStackClip.cpp
src/gpu/GrReducedClip.cpp
src/gpu/GrReducedClip.h
tests/ClipStackTest.cpp

index a3a0de5..0d6270f 100644 (file)
@@ -426,7 +426,7 @@ sk_sp<GrTexture> GrClipStackClip::CreateAlphaClipMask(GrContext* context,
                                                       const SkVector& clipToMaskOffset) {
     GrResourceProvider* resourceProvider = context->resourceProvider();
     GrUniqueKey key;
-    GetClipMaskKey(reducedClip.genID(), reducedClip.ibounds(), &key);
+    GetClipMaskKey(reducedClip.elementsGenID(), reducedClip.ibounds(), &key);
     if (GrTexture* texture = resourceProvider->findAndRefTextureByUniqueKey(key)) {
         return sk_sp<GrTexture>(texture);
     }
@@ -534,9 +534,9 @@ bool GrClipStackClip::CreateStencilClipMask(GrContext* context,
     }
 
     // TODO: these need to be swapped over to using a StencilAttachmentProxy
-    if (stencilAttachment->mustRenderClip(reducedClip.genID(), reducedClip.ibounds(),
+    if (stencilAttachment->mustRenderClip(reducedClip.elementsGenID(), reducedClip.ibounds(),
                                           clipSpaceToStencilOffset)) {
-        stencilAttachment->setLastClip(reducedClip.genID(), reducedClip.ibounds(),
+        stencilAttachment->setLastClip(reducedClip.elementsGenID(), reducedClip.ibounds(),
                                        clipSpaceToStencilOffset);
         // Set the matrix so that rendered clip elements are transformed from clip to stencil space.
         SkVector translate = {
@@ -701,7 +701,7 @@ sk_sp<GrTexture> GrClipStackClip::CreateSoftwareClipMask(GrTextureProvider* texP
                                                          const GrReducedClip& reducedClip,
                                                          const SkVector& clipToMaskOffset) {
     GrUniqueKey key;
-    GetClipMaskKey(reducedClip.genID(), reducedClip.ibounds(), &key);
+    GetClipMaskKey(reducedClip.elementsGenID(), reducedClip.ibounds(), &key);
     if (GrTexture* texture = texProvider->findAndRefTextureByUniqueKey(key)) {
         return sk_sp<GrTexture>(texture);
     }
index 132936c..251155b 100644 (file)
@@ -316,15 +316,6 @@ static GrReducedClip::InitialState reduced_stack_walker(const SkClipStack& stack
     }
     *requiresAA = numAAElements > 0;
 
-    if (0 == result->count()) {
-        if (initialState == InitialTriState::kAllIn) {
-            *resultGenID = SkClipStack::kWideOpenGenID;
-        } else {
-            *resultGenID = SkClipStack::kEmptyGenID;
-        }
-    }
-
-    SkASSERT(SkClipStack::kInvalidGenID != *resultGenID);
     SkASSERT(InitialTriState::kUnknown != initialState);
     return static_cast<GrReducedClip::InitialState>(initialState);
 }
@@ -338,11 +329,6 @@ take a rect in case the caller knows a bound on what is to be drawn through this
 */
 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()) {
@@ -387,6 +373,7 @@ GrReducedClip::GrReducedClip(const SkClipStack& stack, const SkRect& queryBounds
 
         // Implement the clip with an AA rect element.
         fElements.addToHead(stackBounds, SkRegion::kReplace_Op, true/*doAA*/);
+        fElementsGenID = stack.getTopmostGenID();
         fRequiresAA = true;
 
         fInitialState = InitialState::kAllOut;
@@ -406,6 +393,6 @@ GrReducedClip::GrReducedClip(const SkClipStack& stack, const SkRect& queryBounds
 
     // 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,
+    fInitialState = reduced_stack_walker(stack, tighterQuery, fIBounds, &fElements, &fElementsGenID,
                                          &fRequiresAA);
 }
index d7b7ea8..a867483 100644 (file)
@@ -20,11 +20,6 @@ public:
     GrReducedClip(const SkClipStack& stack, const SkRect& queryBounds);
 
     /**
-     * Uniquely identifies this reduced clip.
-     */
-    int32_t genID() const { return fGenID; }
-
-    /**
      * If hasIBounds() is true, this is the bounding box within which the reduced clip is valid, and
      * the caller must not modify any pixels outside this box. Undefined if hasIBounds() is false.
      */
@@ -48,6 +43,12 @@ public:
     const ElementList& elements() const { return fElements; }
 
     /**
+     * If elements() are nonempty, uniquely identifies the list of elements within ibounds().
+     * Otherwise undefined.
+     */
+    int32_t elementsGenID() const { SkASSERT(!fElements.isEmpty()); return fElementsGenID; }
+
+    /**
      * Indicates whether antialiasing is required to process any of the clip elements.
      */
     bool requiresAA() const { return fRequiresAA; }
@@ -60,10 +61,10 @@ public:
     InitialState initialState() const { return fInitialState; }
 
 private:
-    int32_t        fGenID;
     SkIRect        fIBounds;
     bool           fHasIBounds;
     ElementList    fElements;
+    int32_t        fElementsGenID;
     bool           fRequiresAA;
     InitialState   fInitialState;
 };
index 5265ac2..394835b 100644 (file)
@@ -1007,39 +1007,48 @@ static void test_reduced_clip_stack(skiatest::Reporter* reporter) {
             }
         }
 
+        // Zero the memory we will new the GrReducedClip into. This ensures the elements gen ID
+        // will be kInvalidGenID if left uninitialized.
+        SkAlignedSTStorage<1, GrReducedClip> storage;
+        memset(storage.get(), 0, sizeof(GrReducedClip));
+        GR_STATIC_ASSERT(0 == SkClipStack::kInvalidGenID);
+
         // Get the reduced version of the stack.
         SkRect queryBounds = kBounds;
         queryBounds.outset(kBounds.width() / 2, kBounds.height() / 2);
-        const GrReducedClip reduced(stack, queryBounds);
+        const GrReducedClip* reduced = new (storage.get()) GrReducedClip(stack, queryBounds);
 
-        REPORTER_ASSERT_MESSAGE(reporter, SkClipStack::kInvalidGenID != reduced.genID(),
+        REPORTER_ASSERT_MESSAGE(reporter,
+                                reduced->elements().isEmpty() ||
+                                SkClipStack::kInvalidGenID != reduced->elementsGenID(),
                                 testCase.c_str());
 
-        if (!reduced.elements().isEmpty()) {
-            REPORTER_ASSERT_MESSAGE(reporter, reduced.hasIBounds(), testCase.c_str());
+        if (!reduced->elements().isEmpty()) {
+            REPORTER_ASSERT_MESSAGE(reporter, reduced->hasIBounds(), testCase.c_str());
             SkRect stackBounds;
             SkClipStack::BoundsType stackBoundsType;
             stack.getBounds(&stackBounds, &stackBoundsType);
             if (SkClipStack::kNormal_BoundsType == stackBoundsType) {
                 // Unless GrReducedClip starts doing some heroic tightening of the clip bounds, this
                 // will be true since the stack bounds are completely contained inside the query.
-                REPORTER_ASSERT_MESSAGE(reporter, GrClip::IsInsideClip(reduced.ibounds(), stackBounds),
+                REPORTER_ASSERT_MESSAGE(reporter,
+                                        GrClip::IsInsideClip(reduced->ibounds(), stackBounds),
                                         testCase.c_str());
             }
-            REPORTER_ASSERT_MESSAGE(reporter, reduced.requiresAA() == doAA, testCase.c_str());
+            REPORTER_ASSERT_MESSAGE(reporter, reduced->requiresAA() == doAA, testCase.c_str());
         }
 
         // Build a new clip stack based on the reduced clip elements
         SkClipStack reducedStack;
-        if (GrReducedClip::InitialState::kAllOut == reduced.initialState()) {
+        if (GrReducedClip::InitialState::kAllOut == reduced->initialState()) {
             // whether the result is bounded or not, the whole plane should start outside the clip.
             reducedStack.clipEmpty();
         }
-        for (ElementList::Iter iter(reduced.elements()); iter.get(); iter.next()) {
+        for (ElementList::Iter iter(reduced->elements()); iter.get(); iter.next()) {
             add_elem_to_stack(*iter.get(), &reducedStack);
         }
 
-        SkIRect ibounds = reduced.hasIBounds() ? reduced.ibounds() : kIBounds;
+        SkIRect ibounds = reduced->hasIBounds() ? reduced->ibounds() : kIBounds;
 
         // GrReducedClipStack assumes that the final result is clipped to the returned bounds
         reducedStack.clipDevRect(ibounds, SkRegion::kIntersect_Op);
@@ -1053,6 +1062,8 @@ static void test_reduced_clip_stack(skiatest::Reporter* reporter) {
         set_region_to_stack(reducedStack, ibounds, &reducedRegion);
 
         REPORTER_ASSERT_MESSAGE(reporter, region == reducedRegion, testCase.c_str());
+
+        reduced->~GrReducedClip();
     }
 }
 
@@ -1069,11 +1080,16 @@ static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
         stack.clipDevRect(SkRect::MakeXYWH(0, 0, SkScalar(50.3), SkScalar(50.3)), SkRegion::kReplace_Op, true);
         SkRect bounds = SkRect::MakeXYWH(0, 0, 100, 100);
 
-        const GrReducedClip reduced(stack, bounds);
+        SkAlignedSTStorage<1, GrReducedClip> storage;
+        memset(storage.get(), 0, sizeof(GrReducedClip));
+        GR_STATIC_ASSERT(0 == SkClipStack::kInvalidGenID);
+        const GrReducedClip* reduced = new (storage.get()) GrReducedClip(stack, bounds);
 
-        REPORTER_ASSERT(reporter, reduced.elements().count() == 1);
+        REPORTER_ASSERT(reporter, reduced->elements().count() == 1);
         // Clips will be cached based on the generation id. Make sure the gen id is valid.
-        REPORTER_ASSERT(reporter, SkClipStack::kInvalidGenID != reduced.genID());
+        REPORTER_ASSERT(reporter, SkClipStack::kInvalidGenID != reduced->elementsGenID());
+
+        reduced->~GrReducedClip();
     }
     {
         SkClipStack stack;
@@ -1115,30 +1131,30 @@ static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
         } testCases[] = {
 
             // Rect A.
-            { XYWH(0, 0, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::InitialState::kAllIn, IXYWH(0, 0, 25, 25) },
-            { XYWH(0.1f, 0.1f, 25.1f, 25.1f), 0, SkClipStack::kWideOpenGenID, GrReducedClip::InitialState::kAllIn, IXYWH(0, 0, 26, 26) },
+            { XYWH(0, 0, 25, 25), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllIn, IXYWH(0, 0, 25, 25) },
+            { XYWH(0.1f, 0.1f, 25.1f, 25.1f), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllIn, IXYWH(0, 0, 26, 26) },
             { XYWH(0, 0, 27, 27), 1, genIDA, GrReducedClip::InitialState::kAllOut, IXYWH(0, 0, 27, 27)},
 
             // Rect B.
-            { XYWH(50, 0, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::InitialState::kAllIn, IXYWH(50, 0, 25, 25) },
-            { XYWH(50, 0, 25.3f, 25.3f), 0, SkClipStack::kWideOpenGenID, GrReducedClip::InitialState::kAllIn, IXYWH(50, 0, 26, 26) },
+            { XYWH(50, 0, 25, 25), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllIn, IXYWH(50, 0, 25, 25) },
+            { XYWH(50, 0, 25.3f, 25.3f), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllIn, IXYWH(50, 0, 26, 26) },
             { XYWH(50, 0, 27, 27), 1, genIDB, GrReducedClip::InitialState::kAllOut, IXYWH(50, 0, 26, 27) },
 
             // Rect C.
-            { XYWH(0, 50, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::InitialState::kAllIn, IXYWH(0, 50, 25, 25) },
-            { XYWH(0.2f, 50.1f, 25.1f, 25.2f), 0, SkClipStack::kWideOpenGenID, GrReducedClip::InitialState::kAllIn, IXYWH(0, 50, 26, 26) },
+            { XYWH(0, 50, 25, 25), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllIn, IXYWH(0, 50, 25, 25) },
+            { XYWH(0.2f, 50.1f, 25.1f, 25.2f), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllIn, IXYWH(0, 50, 26, 26) },
             { XYWH(0, 50, 27, 27), 1, genIDC, GrReducedClip::InitialState::kAllOut, IXYWH(0, 50, 27, 26) },
 
             // Rect D.
-            { XYWH(50, 50, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::InitialState::kAllIn, IXYWH(50, 50, 25, 25)},
-            { XYWH(50.3f, 50.3f, 25, 25), 0, SkClipStack::kWideOpenGenID, GrReducedClip::InitialState::kAllIn, IXYWH(50, 50, 26, 26)},
+            { XYWH(50, 50, 25, 25), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllIn, IXYWH(50, 50, 25, 25)},
+            { XYWH(50.3f, 50.3f, 25, 25), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllIn, IXYWH(50, 50, 26, 26)},
             { XYWH(50, 50, 27, 27), 1, genIDD, GrReducedClip::InitialState::kAllOut,  IXYWH(50, 50, 26, 26)},
 
             // 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::kEmptyGenID, GrReducedClip::InitialState::kAllOut, IXYWH(26, 26, 24, 24) },
+            { XYWH(26, 26, 24, 24), 0, SkClipStack::kInvalidGenID, GrReducedClip::InitialState::kAllOut, IXYWH(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::InitialState::kAllOut, IXYWH(24, 24, 27, 27) },
@@ -1151,8 +1167,10 @@ static void test_reduced_clip_stack_genid(skiatest::Reporter* reporter) {
             const GrReducedClip reduced(stack, testCases[i].testBounds);
             REPORTER_ASSERT(reporter, reduced.elements().count() == testCases[i].reducedClipCount);
             SkASSERT(reduced.elements().count() == testCases[i].reducedClipCount);
-            REPORTER_ASSERT(reporter, reduced.genID() == testCases[i].reducedGenID);
-            SkASSERT(reduced.genID() == testCases[i].reducedGenID);
+            if (reduced.elements().count()) {
+                REPORTER_ASSERT(reporter, reduced.elementsGenID() == testCases[i].reducedGenID);
+                SkASSERT(reduced.elementsGenID() == testCases[i].reducedGenID);
+            }
             REPORTER_ASSERT(reporter, reduced.initialState() == testCases[i].initialState);
             SkASSERT(reduced.initialState() == testCases[i].initialState);
             REPORTER_ASSERT(reporter, reduced.hasIBounds());