Make GrShape track the winding direction and starting point for rrect types.

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"
 #if SK_SUPPORT_GPU
 #include "GrShape.h"
 #include "SkCanvas.h"
 #include "SkDashPathEffect.h"
 #include "SkPath.h"
+#include "SkPathOps.h"
 #include "SkSurface.h"
 
 using Key = SkTArray<uint32_t>;
 
 static bool make_key(Key* key, const GrShape& shape) {
     int size = shape.unstyledKeySize();
     if (size <= 0) {
         key->reset(0);
         return false;
     }
     SkASSERT(size);
     key->reset(size);
     shape.writeUnstyledKey(key->begin());
     return true;
 }
 
+static bool paths_fill_same(const SkPath& a, const SkPath& b) {
+    SkPath pathXor;
+    Op(a, b, SkPathOp::kXOR_SkPathOp, &pathXor);
+    return pathXor.isEmpty();
+}
+
 static bool test_bounds_by_rasterizing(const SkPath& path, const SkRect& bounds) {
     static constexpr int kRes = 2000;
     // This tolerance is in units of 1/kRes fractions of the bounds width/height.
     static constexpr int kTol = 0;
     GR_STATIC_ASSERT(kRes % 4 == 0);
     SkImageInfo info = SkImageInfo::MakeA8(kRes, kRes);
     sk_sp<SkSurface> surface = SkSurface::MakeRaster(info);
     surface->getCanvas()->clear(0x0);
     SkRect clip = SkRect::MakeXYWH(kRes/4, kRes/4, kRes/2, kRes/2);
     SkMatrix matrix;
@@ skipping 85 matching lines @@
         make_key(&fAppliedPEKey, fAppliedPE);
         make_key(&fAppliedPEThenStrokeKey, fAppliedPEThenStroke);
         make_key(&fAppliedFullKey, fAppliedFull);
 
         // Applying the path effect and then the stroke should always be the same as applying
         // both in one go.
         REPORTER_ASSERT(r, fAppliedPEThenStrokeKey == fAppliedFullKey);
         SkPath a, b;
         fAppliedPEThenStroke.asPath(&a);
         fAppliedFull.asPath(&b);
-        REPORTER_ASSERT(r, a == b);
+        // If the output of the path effect is a rrect then it is possible for a and b to be
+        // different paths that fill identically. The reason is that fAppliedFull will do this:
+        // base -> apply path effect -> rrect_as_path -> stroke -> stroked_rrect_as_path
+        // fAppliedPEThenStroke will have converted the rrect_as_path back to a rrect. However,
+        // now that there is no longer a path effect, the direction and starting index get
+        // canonicalized before the stroke.
+        if (fAppliedPE.asRRect(nullptr, nullptr, nullptr)) {
+            REPORTER_ASSERT(r, paths_fill_same(a, b));
+        } else {
+            REPORTER_ASSERT(r, a == b);
+        }
         REPORTER_ASSERT(r, fAppliedFull.isEmpty() == fAppliedPEThenStroke.isEmpty());
 
         SkPath path;
         fBase.asPath(&path);
         REPORTER_ASSERT(r, path.isEmpty() == fBase.isEmpty());
         fAppliedPE.asPath(&path);
         REPORTER_ASSERT(r, path.isEmpty() == fAppliedPE.isEmpty());
         fAppliedFull.asPath(&path);
         REPORTER_ASSERT(r, path.isEmpty() == fAppliedFull.isEmpty());
 
@@ skipping 72 matching lines @@
         fAppliedPE.asPath(&b);
         REPORTER_ASSERT(reporter, a == b);
         if (expectations.fStrokeApplies) {
             REPORTER_ASSERT(reporter, fBaseKey != fAppliedFullKey);
         } else {
             REPORTER_ASSERT(reporter, fBaseKey == fAppliedFullKey);
         }
     }
 }
 
-void TestCase::compare(skiatest::Reporter* reporter, const TestCase& that,
+void check_equivalence(skiatest::Reporter* r, const GrShape& a, const GrShape& b,
+                       const Key& keyA, const Key& keyB) {
+    // GrShape only respects that input winding direction and start point for rrect shapes
+    // when there is a path effect. Thus, if there are two GrShapes representing the same rrect
+    // but one has a path effect in its style and the other doesn't then asPath() and the unstyled
+    // key will differ. GrShape will have canonicalized the direction and start point for the shape
+    // without the path effect. If *both* have path effects then they should have both preserve
+    // the direction and starting point.
+    SkRRect rrectA = SkRRect::MakeEmpty(), rrectB = SkRRect::MakeEmpty();
+    SkPath::Direction dirA = SkPath::kCW_Direction, dirB = SkPath::kCW_Direction;
+    unsigned startA = ~0U, startB = ~0U;

[egdaniel, 2016/06/06 18:34:36]

comment that this initialization is for compiler

[bsalomon, 2016/06/06 19:41:29]

Done.

+    bool aIsRRect = a.asRRect(&rrectA, &dirA, &startA);
+    bool bIsRRect = b.asRRect(&rrectB, &dirB, &startB);
+    bool aHasPE = a.style().pathEffect();
+    bool bHasPE = b.style().pathEffect();
+    bool allowSameRRectButDiffStartAndDir = (aIsRRect && bIsRRect) && (aHasPE != bHasPE);
+    SkPath pathA, pathB;
+    a.asPath(&pathA);
+    b.asPath(&pathB);
+    if (allowSameRRectButDiffStartAndDir) {
+        REPORTER_ASSERT(r, aIsRRect && bIsRRect);

[egdaniel, 2016/06/06 18:34:36]

is this check redundent with allowSameRRectBDSAD?

[bsalomon, 2016/06/06 19:41:29]

Done.

+        REPORTER_ASSERT(r, rrectA == rrectB);
+        REPORTER_ASSERT(r, paths_fill_same(pathA, pathB));
+    } else {
+        REPORTER_ASSERT(r, pathA == pathB);
+        REPORTER_ASSERT(r, keyA == keyB);
+        REPORTER_ASSERT(r, aIsRRect == bIsRRect);
+        if (aIsRRect) {
+            REPORTER_ASSERT(r, rrectA == rrectB);
+            REPORTER_ASSERT(r, dirA == dirB);
+            REPORTER_ASSERT(r, startA == startB);
+        }
+    }
+    REPORTER_ASSERT(r, a.isEmpty() == b.isEmpty());
+    REPORTER_ASSERT(r, a.knownToBeClosed() == b.knownToBeClosed());
+    REPORTER_ASSERT(r, a.bounds() == b.bounds());
+}
+
+void TestCase::compare(skiatest::Reporter* r, const TestCase& that,
                        ComparisonExpecation expectation) const {
     SkPath a, b;
     switch (expectation) {
         case kAllDifferent_ComparisonExpecation:
-            REPORTER_ASSERT(reporter, fBaseKey != that.fBaseKey);
-            REPORTER_ASSERT(reporter, fAppliedPEKey != that.fAppliedPEKey);
-            REPORTER_ASSERT(reporter, fAppliedFullKey != that.fAppliedFullKey);
+            REPORTER_ASSERT(r, fBaseKey != that.fBaseKey);
+            REPORTER_ASSERT(r, fAppliedPEKey != that.fAppliedPEKey);
+            REPORTER_ASSERT(r, fAppliedFullKey != that.fAppliedFullKey);
             break;
         case kSameUpToPE_ComparisonExpecation:
-            REPORTER_ASSERT(reporter, fBaseKey == that.fBaseKey);
-            fBase.asPath(&a);
-            that.fBase.asPath(&b);
-            REPORTER_ASSERT(reporter, a == b);
-            REPORTER_ASSERT(reporter, fBase.isEmpty() == that.fBase.isEmpty());
-            REPORTER_ASSERT(reporter, fAppliedPEKey != that.fAppliedPEKey);
-            REPORTER_ASSERT(reporter, fAppliedFullKey != that.fAppliedFullKey);
+            check_equivalence(r, fBase, that.fBase, fBaseKey, that.fBaseKey);
+            REPORTER_ASSERT(r, fAppliedPEKey != that.fAppliedPEKey);
+            REPORTER_ASSERT(r, fAppliedFullKey != that.fAppliedFullKey);
             break;
         case kSameUpToStroke_ComparisonExpecation:
-            REPORTER_ASSERT(reporter, fBaseKey == that.fBaseKey);
-            fBase.asPath(&a);
-            that.fBase.asPath(&b);
-            REPORTER_ASSERT(reporter, a == b);
-            REPORTER_ASSERT(reporter, fBase.isEmpty() == that.fBase.isEmpty());
-            REPORTER_ASSERT(reporter, fAppliedPEKey == that.fAppliedPEKey);
-            fAppliedPE.asPath(&a);
-            that.fAppliedPE.asPath(&b);
-            REPORTER_ASSERT(reporter, a == b);
-            REPORTER_ASSERT(reporter, fAppliedPE.isEmpty() == that.fAppliedPE.isEmpty());
-            REPORTER_ASSERT(reporter, fAppliedFullKey != that.fAppliedFullKey);
+            check_equivalence(r, fBase, that.fBase, fBaseKey, that.fBaseKey);
+            check_equivalence(r, fAppliedPE, that.fAppliedPE, fAppliedPEKey, that.fAppliedPEKey);
+            REPORTER_ASSERT(r, fAppliedFullKey != that.fAppliedFullKey);
             break;
         case kAllSame_ComparisonExpecation:
-            REPORTER_ASSERT(reporter, fBaseKey == that.fBaseKey);
-            fBase.asPath(&a);
-            that.fBase.asPath(&b);
-            REPORTER_ASSERT(reporter, a == b);
-            REPORTER_ASSERT(reporter, fBase.isEmpty() == that.fBase.isEmpty());
-            REPORTER_ASSERT(reporter, fAppliedPEKey == that.fAppliedPEKey);
-            fAppliedPE.asPath(&a);
-            that.fAppliedPE.asPath(&b);
-            REPORTER_ASSERT(reporter, a == b);
-            REPORTER_ASSERT(reporter, fAppliedPE.isEmpty() == that.fAppliedPE.isEmpty());
-            REPORTER_ASSERT(reporter, fAppliedFullKey == that.fAppliedFullKey);
-            fAppliedFull.asPath(&a);
-            that.fAppliedFull.asPath(&b);
-            REPORTER_ASSERT(reporter, a == b);
-            REPORTER_ASSERT(reporter, fAppliedFull.isEmpty() == that.fAppliedFull.isEmpty());
+            check_equivalence(r, fBase, that.fBase, fBaseKey, that.fBaseKey);
+            check_equivalence(r, fAppliedPE, that.fAppliedPE, fAppliedPEKey, that.fAppliedPEKey);
+            check_equivalence(r, fAppliedFull, that.fAppliedFull, fAppliedFullKey,
+                              that.fAppliedFullKey);
             break;
     }
 }
 }  // namespace
 
 static sk_sp<SkPathEffect> make_dash() {
     static const SkScalar kIntervals[] = { 0.25, 3.f, 0.5, 2.f };
     static const SkScalar kPhase = 0.75;
     return SkDashPathEffect::Make(kIntervals, SK_ARRAY_COUNT(kIntervals), kPhase);
 }
@@ skipping 125 matching lines @@
     strokeDashCase1.compare(reporter, strokeDashCase2,  TestCase::kSameUpToPE_ComparisonExpecation);
 
     // Stroke and fill cases
     SkPaint strokeAndFill = stroke;
     strokeAndFill.setStyle(SkPaint::kStrokeAndFill_Style);
     TestCase strokeAndFillCase1(geo, strokeAndFill, reporter, kS1);
     TestCase strokeAndFillCase2(geo, strokeAndFill, reporter, kS2);
     // Scale affects the stroke. Though, this can wind up creating a rect when the input is a rect.
     // In that case we wind up with a pure geometry key and the geometries are the same.
     SkRRect rrect;
-    if (strokeAndFillCase1.appliedFullStyleShape().asRRect(&rrect)) {
+    if (strokeAndFillCase1.appliedFullStyleShape().asRRect(&rrect, nullptr, nullptr)) {
         // We currently only expect to get here in the rect->rect case.
         REPORTER_ASSERT(reporter, rrect.isRect());
-        REPORTER_ASSERT(reporter, strokeAndFillCase1.baseShape().asRRect(&rrect) && rrect.isRect());
+        REPORTER_ASSERT(reporter,
+                        strokeAndFillCase1.baseShape().asRRect(&rrect, nullptr, nullptr) &&
+                        rrect.isRect());
         strokeAndFillCase1.compare(reporter, strokeAndFillCase2,
                                    TestCase::kAllSame_ComparisonExpecation);
     } else {
         strokeAndFillCase1.compare(reporter, strokeAndFillCase2,
                                    TestCase::kSameUpToStroke_ComparisonExpecation);
     }
 
     SkPaint strokeAndFillDash = strokeDash;
     strokeAndFillDash.setStyle(SkPaint::kStrokeAndFill_Style);
     TestCase strokeAndFillDashCase1(geo, strokeAndFillDash, reporter, kS1);
@@ skipping 218 matching lines @@
                       TestCase::kSameUpToStroke_ComparisonExpecation);
 
     TestCase rrectFillCase(RRectPathEffect::RRect(), fill, reporter);
     SkPaint stroke = peStroke;
     stroke.setPathEffect(nullptr);
     TestCase rrectStrokeCase(RRectPathEffect::RRect(), stroke, reporter);
 
     SkRRect rrect;
     // Applying the path effect should make a SkRRect shape. There is no further stroking in the
     // geoPECase, so the full style should be the same as just the PE.
-    REPORTER_ASSERT(reporter, geoPECase.appliedPathEffectShape().asRRect(&rrect));
+    REPORTER_ASSERT(reporter, geoPECase.appliedPathEffectShape().asRRect(&rrect, nullptr, nullptr));
     REPORTER_ASSERT(reporter, rrect == RRectPathEffect::RRect());
     REPORTER_ASSERT(reporter, geoPECase.appliedPathEffectKey() == rrectFillCase.baseKey());
 
-    REPORTER_ASSERT(reporter, geoPECase.appliedFullStyleShape().asRRect(&rrect));
+    REPORTER_ASSERT(reporter, geoPECase.appliedFullStyleShape().asRRect(&rrect, nullptr, nullptr));
     REPORTER_ASSERT(reporter, rrect == RRectPathEffect::RRect());
     REPORTER_ASSERT(reporter, geoPECase.appliedFullStyleKey() == rrectFillCase.baseKey());
 
     // In the PE+stroke case applying the full style should be the same as just stroking the rrect.
-    REPORTER_ASSERT(reporter, geoPEStrokeCase.appliedPathEffectShape().asRRect(&rrect));
+    REPORTER_ASSERT(reporter,
+                    geoPEStrokeCase.appliedPathEffectShape().asRRect(&rrect, nullptr, nullptr));
     REPORTER_ASSERT(reporter, rrect == RRectPathEffect::RRect());
     REPORTER_ASSERT(reporter, geoPEStrokeCase.appliedPathEffectKey() == rrectFillCase.baseKey());
 
-    REPORTER_ASSERT(reporter, !geoPEStrokeCase.appliedFullStyleShape().asRRect(&rrect));
+    REPORTER_ASSERT(reporter,
+                    !geoPEStrokeCase.appliedFullStyleShape().asRRect(&rrect, nullptr, nullptr));
     REPORTER_ASSERT(reporter, geoPEStrokeCase.appliedFullStyleKey() ==
                               rrectStrokeCase.appliedFullStyleKey());
 }
 
 template <typename GEO>
 void test_unknown_path_effect(skiatest::Reporter* reporter, const GEO& geo) {
     /**
      * This path effect just adds two lineTos to the input path.
      */
     class AddLineTosPathEffect : SkPathEffect {
@@ skipping 54 matching lines @@
     private:
         MakeHairlinePathEffect() {}
     };
 
     SkPaint fill;
     SkPaint pe;
     pe.setPathEffect(MakeHairlinePathEffect::Make());
 
     TestCase peCase(geo, pe, reporter);
 
-    SkPath a, b;
+    SkPath a, b, c;
     peCase.baseShape().asPath(&a);
     peCase.appliedPathEffectShape().asPath(&b);
-    REPORTER_ASSERT(reporter, a == b);
-    peCase.appliedFullStyleShape().asPath(&b);
-    REPORTER_ASSERT(reporter, a == b);
-    REPORTER_ASSERT(reporter, peCase.appliedPathEffectShape().style().isSimpleHairline());
-    REPORTER_ASSERT(reporter, peCase.appliedFullStyleShape().style().isSimpleHairline());
+    peCase.appliedFullStyleShape().asPath(&c);
     if (isNonPath) {
-        REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey() == peCase.baseKey());
-        REPORTER_ASSERT(reporter, peCase.appliedFullStyleKey() == peCase.baseKey());
+        // RRect types can have a change in start index or direction after the PE is applied. This
+        // is because once the PE is applied, GrShape may canonicalize the dir and index since it
+        // is not germane to the styling any longer.
+        // Instead we just check that the paths would fill the same both before and after styling.
+        REPORTER_ASSERT(reporter, paths_fill_same(a, b));
+        REPORTER_ASSERT(reporter, paths_fill_same(a, c));
     } else {
+        REPORTER_ASSERT(reporter, a == b);
+        REPORTER_ASSERT(reporter, a == c);
         REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey().empty());
         REPORTER_ASSERT(reporter, peCase.appliedFullStyleKey().empty());
     }
+    REPORTER_ASSERT(reporter, peCase.appliedPathEffectShape().style().isSimpleHairline());
+    REPORTER_ASSERT(reporter, peCase.appliedFullStyleShape().style().isSimpleHairline());
 }
 
 /**
  * isNonPath indicates whether the initial shape made from the path is expected to be recognized
  * as a simpler shape type (e.g. rrect)
  */
 void test_volatile_path(skiatest::Reporter* reporter, const SkPath& path,
                         bool isNonPath) {
     SkPath vPath(path);
     vPath.setIsVolatile(true);
@@ skipping 110 matching lines @@
                                         TestCase::kAllSame_ComparisonExpecation);
 
     // Same for a rect.
     SkRect emptyRect = SkRect::MakeEmpty();
     TestCase dashAndStrokeEmptyRectCase(emptyRect, dashAndStroke, reporter);
     dashAndStrokeEmptyRectCase.compare(reporter, fillEmptyCase,
                                        TestCase::kAllSame_ComparisonExpecation);
 }
 
 DEF_TEST(GrShape, reporter) {
-    sk_sp<SkPathEffect> dashPE = make_dash();
+    for (auto r : { SkRect::MakeWH(10, 20),
+                    SkRect::MakeWH(-10, -20),
+                    SkRect::MakeWH(-10, 20),
+                    SkRect::MakeWH(10, -20)}) {
+        test_basic(reporter, r);
+        test_scale(reporter, r);
+        test_dash_fill(reporter, r);
+        test_null_dash(reporter, r);
+        // Test modifying various stroke params.
+        test_stroke_param<SkRect, SkScalar>(
+                reporter, r,
+                [](SkPaint* p, SkScalar w) { p->setStrokeWidth(w);},
+                SkIntToScalar(2), SkIntToScalar(4));
+        test_stroke_param<SkRect, SkPaint::Join>(
+                reporter, r,
+                [](SkPaint* p, SkPaint::Join j) { p->setStrokeJoin(j);},
+                SkPaint::kMiter_Join, SkPaint::kRound_Join);
+        test_stroke_cap(reporter, r);
+        test_miter_limit(reporter, r);
+        test_path_effect_makes_rrect(reporter, r);
+        test_unknown_path_effect(reporter, r);
+        test_path_effect_makes_empty_shape(reporter, r);
+        test_make_hairline_path_effect(reporter, r, true);
+    }
 
     for (auto rr : { SkRRect::MakeRect(SkRect::MakeWH(10, 10)),
-                     SkRRect::MakeRectXY(SkRect::MakeWH(10, 10), 3, 4)}) {
+                     SkRRect::MakeRectXY(SkRect::MakeWH(10, 10), 3, 4),
+                     SkRRect::MakeOval(SkRect::MakeWH(20, 20))}) {
         test_basic(reporter, rr);
         test_scale(reporter, rr);
         test_dash_fill(reporter, rr);
         test_null_dash(reporter, rr);
         // Test modifying various stroke params.
         test_stroke_param<SkRRect, SkScalar>(
                           reporter, rr,
                           [](SkPaint* p, SkScalar w) { p->setStrokeWidth(w);},
                           SkIntToScalar(2), SkIntToScalar(4));
         test_stroke_param<SkRRect, SkPaint::Join>(
@@ skipping 66 matching lines @@
         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_make_hairline_path_effect(reporter, path, testPath.fIsRRectForStroke);
 
         SkPaint fillPaint;
         TestCase fillPathCase(path, fillPaint, reporter);
         SkRRect rrect;
         REPORTER_ASSERT(reporter, testPath.fIsRRectForFill ==
-                                  fillPathCase.baseShape().asRRect(&rrect));
+                                  fillPathCase.baseShape().asRRect(&rrect, nullptr, nullptr));
         if (testPath.fIsRRectForFill) {
+            TestCase fillPathCase2(path, fillPaint, reporter);
             REPORTER_ASSERT(reporter, rrect == testPath.fRRect);
             TestCase fillRRectCase(rrect, fillPaint, reporter);
-            fillPathCase.compare(reporter, fillRRectCase, TestCase::kAllSame_ComparisonExpecation);
+            fillPathCase2.compare(reporter, fillRRectCase, TestCase::kAllSame_ComparisonExpecation);
         }
 
         SkPaint strokePaint;
         strokePaint.setStrokeWidth(3.f);
         strokePaint.setStyle(SkPaint::kStroke_Style);
         TestCase strokePathCase(path, strokePaint, reporter);
         REPORTER_ASSERT(reporter, testPath.fIsRRectForStroke ==
-                                  strokePathCase.baseShape().asRRect(&rrect));
+                                  strokePathCase.baseShape().asRRect(&rrect, nullptr, nullptr));
         if (testPath.fIsRRectForStroke) {
             REPORTER_ASSERT(reporter, rrect == testPath.fRRect);
             TestCase strokeRRectCase(rrect, strokePaint, reporter);
             strokePathCase.compare(reporter, strokeRRectCase,
-                                 TestCase::kAllSame_ComparisonExpecation);
+                                   TestCase::kAllSame_ComparisonExpecation);
         }
     }
 
     // Test a volatile empty path.
     test_volatile_path(reporter, SkPath(), true);
 
     test_empty_shape(reporter);
 }
 
 #endif