Fix GrShape to preserve inverseness of rrects for strokes but not dashes.

tests/GrShapeTest.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 <initializer_list>
 #include <functional>
 #include "Test.h"
@@ skipping 263 matching lines @@
     bool aIsRRect = a.asRRect(&rrectA, &dirA, &startA, &invertedA);
     bool bIsRRect = b.asRRect(&rrectB, &dirB, &startB, &invertedB);
     bool aHasPE = a.style().hasPathEffect();
     bool bHasPE = b.style().hasPathEffect();
     bool allowSameRRectButDiffStartAndDir = (aIsRRect && bIsRRect) && (aHasPE != bHasPE);
 
     SkPath pathA, pathB;
     a.asPath(&pathA);
     b.asPath(&pathB);
 
-    // Having a fill style or non-dash path effect can prevent 'a' but not 'b' from turning an
-    // inverse fill type into a non-inverse fill type.
+    // Having a dash path effect can allow 'a' but not 'b' to turn a inverse fill type into a
+    // non-inverse fill type  (or vice versa).
     bool ignoreInversenessDifference = false;
     if (pathA.isInverseFillType() != pathB.isInverseFillType()) {
         const GrShape* s1 = pathA.isInverseFillType() ? &a : &b;
         const GrShape* s2 = pathA.isInverseFillType() ? &b : &a;
-        SkStrokeRec::Style style1 = s1->style().strokeRec().getStyle();
-        SkStrokeRec::Style style2 = s2->style().strokeRec().getStyle();
-        bool canDropInverse1 = !s1->style().hasNonDashPathEffect() &&
-                                (SkStrokeRec::kStroke_Style == style1 ||
-                                 SkStrokeRec::kHairline_Style == style1);
-        bool canDropInverse2 = !s2->style().hasNonDashPathEffect() &&
-                               (SkStrokeRec::kStroke_Style == style2 ||
-                                SkStrokeRec::kHairline_Style == style2);
-        ignoreInversenessDifference = !canDropInverse1 && canDropInverse2;
+        bool canDropInverse1 = s1->style().isDashed();
+        bool canDropInverse2 = s2->style().isDashed();
+        ignoreInversenessDifference = (canDropInverse1 != canDropInverse2);
     }
 
     if (allowSameRRectButDiffStartAndDir) {
         REPORTER_ASSERT(r, rrectA == rrectB);
         REPORTER_ASSERT(r, paths_fill_same(pathA, pathB));
         REPORTER_ASSERT(r, ignoreInversenessDifference || invertedA == invertedB);
     } else {
         SkPath pA = pathA;
         SkPath pB = pathB;
         if (ignoreInversenessDifference) {
@@ skipping 778 matching lines @@
                         SkPathEffect* pe = dash ? dashEffect.get() : nullptr;
                         shapes[index(inverted, dir, start, style, dash)] =
                                 GrShape(rrect, dir, start, SkToBool(inverted),
                                         GrStyle(strokeRecs[style], pe));
                     }
                 }
             }
         }
     }
 
-    static const SkPath::Direction kDir = SkPath::kCW_Direction; // arbitrary
-    const GrShape& exampleFillCase = shapes[index(false, kDir, 0, kFill, false)];
+    // Get the keys for some example shape instances that we'll use for comparision against the
+    // rest.
+    static const SkPath::Direction kExamplesDir = SkPath::kCW_Direction;
+    static const SkPath::Direction kExamplesStart = SkPath::kCW_Direction;

[egdaniel, 2016/06/14 19:44:43]

Should the start be a index here and not a direction?

[bsalomon, 2016/06/14 19:48:27]

derrrr. Fixed

+    const GrShape& exampleFillCase = shapes[index(false, kExamplesDir, 0, kFill, false)];
     Key exampleFillCaseKey;
     make_key(&exampleFillCaseKey, exampleFillCase);
 
-    const GrShape& exampleStrokeAndFillCase = shapes[index(false, kDir, 0, kStrokeAndFill, false)];
+    const GrShape& exampleStrokeAndFillCase = shapes[index(false, kExamplesDir, kExamplesStart,
+                                                           kStrokeAndFill, false)];
     Key exampleStrokeAndFillCaseKey;
     make_key(&exampleStrokeAndFillCaseKey, exampleStrokeAndFillCase);
 
-    const GrShape& exampleInvFillCase = shapes[index(true, kDir, 0, kFill, false)];
+    const GrShape& exampleInvFillCase = shapes[index(true, kExamplesDir, kExamplesStart, kFill,
+                                                     false)];
     Key exampleInvFillCaseKey;
     make_key(&exampleInvFillCaseKey, exampleInvFillCase);
 
-    const GrShape& exampleInvStrokeAndFillCase =
-            shapes[index(true, kDir, 0, kStrokeAndFill, false)];
+    const GrShape& exampleInvStrokeAndFillCase = shapes[index(true, kExamplesDir, kExamplesStart,
+                                                              kStrokeAndFill, false)];
     Key exampleInvStrokeAndFillCaseKey;
     make_key(&exampleInvStrokeAndFillCaseKey, exampleInvStrokeAndFillCase);
 
-    const GrShape& exampleStrokeCase = shapes[index(false, kDir, 0, kStroke, false)];
+    const GrShape& exampleStrokeCase = shapes[index(false, kExamplesDir, kExamplesStart, kStroke,
+                                                    false)];
     Key exampleStrokeCaseKey;
     make_key(&exampleStrokeCaseKey, exampleStrokeCase);
 
-    const GrShape& exampleHairlineCase = shapes[index(false, kDir, 0, kHairline, false)];
+    const GrShape& exampleInvStrokeCase = shapes[index(true, kExamplesDir, kExamplesStart, kStroke,
+                                                       false)];
+    Key exampleInvStrokeCaseKey;
+    make_key(&exampleInvStrokeCaseKey, exampleInvStrokeCase);
+
+    const GrShape& exampleHairlineCase = shapes[index(false, kExamplesDir, kExamplesStart,
+                                                      kHairline, false)];
     Key exampleHairlineCaseKey;
     make_key(&exampleHairlineCaseKey, exampleHairlineCase);
 
+    const GrShape& exampleInvHairlineCase = shapes[index(true, kExamplesDir, kExamplesStart,
+                                                         kHairline, false)];
+    Key exampleInvHairlineCaseKey;
+    make_key(&exampleInvHairlineCaseKey, exampleInvHairlineCase);
+
     // These are dummy initializations to suppress warnings.
     SkRRect queryRR = SkRRect::MakeEmpty();
     SkPath::Direction queryDir = SkPath::kCW_Direction;
     unsigned queryStart = ~0U;
     bool queryInverted = true;
 
     REPORTER_ASSERT(r, exampleFillCase.asRRect(&queryRR, &queryDir, &queryStart, &queryInverted));
     REPORTER_ASSERT(r, queryRR == rrect);
     REPORTER_ASSERT(r, SkPath::kCW_Direction == queryDir);
     REPORTER_ASSERT(r, 0 == queryStart);
@@ skipping 20 matching lines @@
     REPORTER_ASSERT(r, 0 == queryStart);
     REPORTER_ASSERT(r, queryInverted);
 
     REPORTER_ASSERT(r, exampleHairlineCase.asRRect(&queryRR, &queryDir, &queryStart,
                                                    &queryInverted));
     REPORTER_ASSERT(r, queryRR == rrect);
     REPORTER_ASSERT(r, SkPath::kCW_Direction == queryDir);
     REPORTER_ASSERT(r, 0 == queryStart);
     REPORTER_ASSERT(r, !queryInverted);
 
+    REPORTER_ASSERT(r, exampleInvHairlineCase.asRRect(&queryRR, &queryDir, &queryStart,
+                                                      &queryInverted));
+    REPORTER_ASSERT(r, queryRR == rrect);
+    REPORTER_ASSERT(r, SkPath::kCW_Direction == queryDir);
+    REPORTER_ASSERT(r, 0 == queryStart);
+    REPORTER_ASSERT(r, queryInverted);
+
     REPORTER_ASSERT(r, exampleStrokeCase.asRRect(&queryRR, &queryDir, &queryStart, &queryInverted));
     REPORTER_ASSERT(r, queryRR == rrect);
     REPORTER_ASSERT(r, SkPath::kCW_Direction == queryDir);
     REPORTER_ASSERT(r, 0 == queryStart);
     REPORTER_ASSERT(r, !queryInverted);
 
+    REPORTER_ASSERT(r, exampleInvStrokeCase.asRRect(&queryRR, &queryDir, &queryStart,
+                                                    &queryInverted));
+    REPORTER_ASSERT(r, queryRR == rrect);
+    REPORTER_ASSERT(r, SkPath::kCW_Direction == queryDir);
+    REPORTER_ASSERT(r, 0 == queryStart);
+    REPORTER_ASSERT(r, queryInverted);
+
     // Remember that the key reflects the geometry before styling is applied.
     REPORTER_ASSERT(r, exampleFillCaseKey != exampleInvFillCaseKey);
     REPORTER_ASSERT(r, exampleFillCaseKey == exampleStrokeAndFillCaseKey);
     REPORTER_ASSERT(r, exampleFillCaseKey != exampleInvStrokeAndFillCaseKey);
     REPORTER_ASSERT(r, exampleFillCaseKey == exampleStrokeCaseKey);
+    REPORTER_ASSERT(r, exampleFillCaseKey != exampleInvStrokeCaseKey);
     REPORTER_ASSERT(r, exampleFillCaseKey == exampleHairlineCaseKey);
+    REPORTER_ASSERT(r, exampleFillCaseKey != exampleInvHairlineCaseKey);
     REPORTER_ASSERT(r, exampleInvStrokeAndFillCaseKey == exampleInvFillCaseKey);
+    REPORTER_ASSERT(r, exampleInvStrokeAndFillCaseKey == exampleInvStrokeCaseKey);
+    REPORTER_ASSERT(r, exampleInvStrokeAndFillCaseKey == exampleInvHairlineCaseKey);
 
     for (bool inverted : {false, true}) {
         for (SkPath::Direction dir : {SkPath::kCW_Direction, SkPath::kCCW_Direction}) {
             for (unsigned start = 0; start < 8; ++start) {
                 for (bool dash : {false, true}) {
                     const GrShape& fillCase = shapes[index(inverted, dir, start, kFill, dash)];
                     Key fillCaseKey;
                     make_key(&fillCaseKey, fillCase);
 
                     const GrShape& strokeAndFillCase = shapes[index(inverted, dir, start,
@@ skipping 11 matching lines @@
                     TestCase d(inverted ? exampleInvStrokeAndFillCase
                                         : exampleStrokeAndFillCase, r);
                     a.compare(r, b, TestCase::kAllSame_ComparisonExpecation);
                     c.compare(r, d, TestCase::kAllSame_ComparisonExpecation);
 
                     const GrShape& strokeCase = shapes[index(inverted, dir, start, kStroke, dash)];
                     const GrShape& hairlineCase = shapes[index(inverted, dir, start, kHairline,
                                                                dash)];
 
                     TestCase e(strokeCase, r);
-                    TestCase f(exampleStrokeCase, r);
                     TestCase g(hairlineCase, r);
-                    TestCase h(exampleHairlineCase, r);
 
-                    // Both hairline and stroke shapes must respect the dashing and both
-                    // ignore inverseness.
+                    // Both hairline and stroke shapes must respect the dashing.
                     if (dash) {
+                        // Dashing always ignores the inverseness. skbug.com/5421
+                        TestCase f(exampleStrokeCase, r);
+                        TestCase h(exampleHairlineCase, r);
                         unsigned expectedStart = canonicalize_rrect_start(start, rrect);
                         REPORTER_ASSERT(r, strokeCase.style().pathEffect());
                         REPORTER_ASSERT(r, hairlineCase.style().pathEffect());
 
                         REPORTER_ASSERT(r, strokeCase.asRRect(&queryRR, &queryDir, &queryStart,
                                                               &queryInverted));
                         REPORTER_ASSERT(r, queryRR == rrect);
                         REPORTER_ASSERT(r, queryDir == dir);
                         REPORTER_ASSERT(r, queryStart == expectedStart);
                         REPORTER_ASSERT(r, !queryInverted);
                         REPORTER_ASSERT(r, hairlineCase.asRRect(&queryRR, &queryDir, &queryStart,
                                                                 &queryInverted));
                         REPORTER_ASSERT(r, queryRR == rrect);
                         REPORTER_ASSERT(r, queryDir == dir);
                         REPORTER_ASSERT(r, queryStart == expectedStart);
                         REPORTER_ASSERT(r, !queryInverted);
 
                         // The pre-style case for the dash will match the non-dash example iff the
                         // dir and start match (dir=cw, start=0).
                         if (0 == expectedStart && SkPath::kCW_Direction == dir) {
                             e.compare(r, f, TestCase::kSameUpToPE_ComparisonExpecation);
                             g.compare(r, h, TestCase::kSameUpToPE_ComparisonExpecation);
                         } else {
                             e.compare(r, f, TestCase::kAllDifferent_ComparisonExpecation);
                             g.compare(r, h, TestCase::kAllDifferent_ComparisonExpecation);
                         }
                     } else {
+                        TestCase f(inverted ? exampleInvStrokeCase : exampleStrokeCase, r);
+                        TestCase h(inverted ? exampleInvHairlineCase : exampleHairlineCase, r);
                         REPORTER_ASSERT(r, !strokeCase.style().pathEffect());
                         REPORTER_ASSERT(r, !hairlineCase.style().pathEffect());
                         e.compare(r, f, TestCase::kAllSame_ComparisonExpecation);
                         g.compare(r, h, TestCase::kAllSame_ComparisonExpecation);
                     }
                 }
             }
         }
     }
 }
@@ skipping 127 matching lines @@
                 [](SkPaint* p, SkPaint::Join j) { p->setStrokeJoin(j);},
                 SkPaint::kMiter_Join, SkPaint::kRound_Join);
             test_stroke_cap(reporter, path);
             test_miter_limit(reporter, path);
             test_unknown_path_effect(reporter, path);
             test_path_effect_makes_empty_shape(reporter, path);
             test_path_effect_fails(reporter, path);
             test_make_hairline_path_effect(reporter, path, testPath.fIsRRectForStroke);
         }
     }
+
     for (auto testPath : paths) {
         const SkPath& path = testPath.fPath;
 
         SkPaint fillPaint;
         TestCase fillPathCase(path, fillPaint, reporter);
         SkRRect rrect;
         REPORTER_ASSERT(reporter, testPath.fIsRRectForFill ==
                                   fillPathCase.baseShape().asRRect(&rrect, nullptr, nullptr,
                                                                    nullptr));
         if (testPath.fIsRRectForFill) {
@@ skipping 20 matching lines @@
         REPORTER_ASSERT(reporter, testPath.fIsLine == strokePathCase.baseShape().asLine(nullptr));
     }
 
     // Test a volatile empty path.
     test_volatile_path(reporter, SkPath(), true);
 
     test_empty_shape(reporter);
 }
 
 #endif