Canonicalize path fill types for stroked paths in GrShape.

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

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

src/gpu/GrShape.cpp

@@ -332,27 +332,25 @@ void GrShape::attemptToSimplifyPath() {
                 fPath.get()->setIsVolatile(true);
             }
         }
-        if (fPath.get()->isConvex()) {
+        if (!this->style().hasNonDashPathEffect()) {
-            // There is no distinction between even/odd and non-zero winding count for convex
+            if (this->style().strokeRec().getStyle() == SkStrokeRec::kStroke_Style ||
-            // paths.
+                this->style().strokeRec().getStyle() == SkStrokeRec::kHairline_Style) {
-            if (fPath.get()->isInverseFillType()) {
+                // Stroke styles don't differentiate between winding and even/odd.
-               fPath.get()->setFillType(SkPath::kInverseEvenOdd_FillType);
+                // Moreover, dashing ignores inverseness (skbug.com/5421)
-            } else {
+                bool inverse = !this->fStyle.isDashed() && fPath.get()->isInverseFillType();
-                fPath.get()->setFillType(SkPath::kEvenOdd_FillType);
+                if (inverse) {
-            }
+                    fPath.get()->setFillType(kDefaultPathInverseFillType);
-        }
+                } else {
-        if (this->style().isDashed()) {
+                    fPath.get()->setFillType(kDefaultPathFillType);
-            // Dashing ignores inverseness (skbug.com/5421)
+                }
-            switch (fPath.get()->getFillType()) {
+            } else if (fPath.get()->isConvex()) {
-                case SkPath::kWinding_FillType:
+                // There is no distinction between even/odd and non-zero winding count for convex
-                case SkPath::kEvenOdd_FillType:
+                // paths.
-                    break;
+                if (fPath.get()->isInverseFillType()) {
-                case SkPath::kInverseWinding_FillType:
+                    fPath.get()->setFillType(kDefaultPathInverseFillType);
-                    fPath.get()->setFillType(SkPath::kWinding_FillType);
+                } else {
-                    break;
+                    fPath.get()->setFillType(kDefaultPathFillType);
-                case SkPath::kInverseEvenOdd_FillType:
+                }
-                    fPath.get()->setFillType(SkPath::kEvenOdd_FillType);
-                    break;
             }
         }
     }

src/gpu/GrShape.h

@@ -190,9 +190,9 @@ public:
                 out->addRRect(fRRect, fRRectDir, fRRectStart);
                 // Below matches the fill type that attemptToSimplifyPath uses.
                 if (fRRectIsInverted) {
-                    out->setFillType(SkPath::kInverseEvenOdd_FillType);
+                    out->setFillType(kDefaultPathInverseFillType);
                 } else {
-                    out->setFillType(SkPath::kEvenOdd_FillType);
+                    out->setFillType(kDefaultPathFillType);
                 }
                 break;
             case Type::kPath:
@@ -284,6 +284,11 @@ private:
     void attemptToSimplifyPath();
     void attemptToSimplifyRRect();
 
+    // Defaults to use when there is no distinction between even/odd and winding fills.
+    static constexpr SkPath::FillType kDefaultPathFillType = SkPath::kEvenOdd_FillType;
+    static constexpr SkPath::FillType kDefaultPathInverseFillType =
+            SkPath::kInverseEvenOdd_FillType;
+
     static constexpr SkPath::Direction kDefaultRRectDir = SkPath::kCW_Direction;
     static constexpr unsigned kDefaultRRectStart = 0;
 

tests/GrShapeTest.cpp

@@ -299,7 +299,23 @@ void check_equivalence(skiatest::Reporter* r, const GrShape& a, const GrShape& b
         bool canDropInverse2 = s2->style().isDashed();
         ignoreInversenessDifference = (canDropInverse1 != canDropInverse2);
     }
-
+    bool ignoreWindingVsEvenOdd = false;
+    if (SkPath::ConvertToNonInverseFillType(pathA.getFillType()) !=
+        SkPath::ConvertToNonInverseFillType(pathB.getFillType())) {
+        const SkStrokeRec::Style  strokeRecStyleA = a.style().strokeRec().getStyle();
+        const SkStrokeRec::Style  strokeRecStyleB = b.style().strokeRec().getStyle();
+        bool aCanChange = !a.style().hasNonDashPathEffect() &&
+                          (strokeRecStyleA == SkStrokeRec::kStroke_Style ||
+                           strokeRecStyleA == SkStrokeRec::kHairline_Style ||
+                           (a.style().isSimpleFill() && pathA.isConvex()));
+        bool bCanChange = !b.style().hasNonDashPathEffect() &&
+                          (strokeRecStyleB == SkStrokeRec::kStroke_Style ||
+                           strokeRecStyleB == SkStrokeRec::kHairline_Style ||
+                           (b.style().isSimpleFill() && pathB.isConvex()));
+        if (aCanChange != bCanChange) {
+            ignoreWindingVsEvenOdd = true;
+        }
+    }
     if (allowSameRRectButDiffStartAndDir) {
         REPORTER_ASSERT(r, rrectA == rrectB);
         REPORTER_ASSERT(r, paths_fill_same(pathA, pathB));
@@ -310,9 +326,17 @@ void check_equivalence(skiatest::Reporter* r, const GrShape& a, const GrShape& b
         if (ignoreInversenessDifference) {
             pA.setFillType(SkPath::ConvertToNonInverseFillType(pathA.getFillType()));
             pB.setFillType(SkPath::ConvertToNonInverseFillType(pathB.getFillType()));
-            REPORTER_ASSERT(r, keyA != keyB);
+        }
-        } else {
+        if (ignoreWindingVsEvenOdd) {
+            pA.setFillType(pA.isInverseFillType() ? SkPath::kInverseEvenOdd_FillType
+                                                  : SkPath::kEvenOdd_FillType);
+            pB.setFillType(pB.isInverseFillType() ? SkPath::kInverseEvenOdd_FillType
+                                                  : SkPath::kEvenOdd_FillType);
+        }
+        if (!ignoreInversenessDifference && !ignoreWindingVsEvenOdd) {
             REPORTER_ASSERT(r, keyA == keyB);
+        } else {
+            REPORTER_ASSERT(r, keyA != keyB);
         }
         if (a.style().isSimpleFill() != b.style().isSimpleFill()) {
             // GrShape will close paths with simple fill style. Make the non-filled path closed
@@ -842,6 +866,11 @@ void test_make_hairline_path_effect(skiatest::Reporter* reporter, const GEO& geo
         REPORTER_ASSERT(reporter, paths_fill_same(a, b));
         REPORTER_ASSERT(reporter, paths_fill_same(a, c));
     } else {
+        // The base shape cannot perform canonicalization on the path's fill type because of an
+        // unknown path effect. However, after the path effect is applied the resulting hairline
+        // shape will canonicalize the path fill type since hairlines (and stroking in general)
+        // don't distinguish between even/odd and non-zero winding.
+        a.setFillType(b.getFillType());
         REPORTER_ASSERT(reporter, a == b);
         REPORTER_ASSERT(reporter, a == c);
         REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey().empty());