fix path op builder

The rewrite of path ops caused the inner contour direction to be reversed.
This exposed an existing bug in path ops builder, namely that the implicit
winding of the internal sum path could hide inner contours if they ended
up in the wrong direction.

Setting the sum path's fill type to even-odd ensures that the inner
contours aren't discarded.

R=fmalita@chromium.org
BUG=skia:3838

Review URL: https://codereview.chromium.org/1126193004

diff --git a/src/pathops/SkOpBuilder.cpp b/src/pathops/SkOpBuilder.cpp
index 2f19b33..1a446f1 100644 (file)
--- a/src/pathops/SkOpBuilder.cpp
+++ b/src/pathops/SkOpBuilder.cpp
@@ -85,6 +85,7 @@ bool SkOpBuilder::resolve(SkPath* result) {
         sum.addPath(fPathRefs[index]);
     }
     reset();
+    sum.setFillType(SkPath::kEvenOdd_FillType);
     bool success = Simplify(sum, result);
     if (!success) {
         *result = original;

diff --git a/tests/PathOpsBuilderTest.cpp b/tests/PathOpsBuilderTest.cpp
index c93931d..8ab92c3 100644 (file)
--- a/tests/PathOpsBuilderTest.cpp
+++ b/tests/PathOpsBuilderTest.cpp
@@ -78,3 +78,29 @@ DEF_TEST(PathOpsBuilder, reporter) {
     int pixelDiff = comparePaths(reporter, __FUNCTION__, opCompare, result, bitmap);
     REPORTER_ASSERT(reporter, pixelDiff == 0);
 }
+
+DEF_TEST(Issue3838, reporter) {
+    SkPath path;
+    path.moveTo(200, 170);
+    path.lineTo(220, 170);
+    path.lineTo(220, 230);
+    path.lineTo(240, 230);
+    path.lineTo(240, 210);
+    path.lineTo(180, 210);
+    path.lineTo(180, 190);
+    path.lineTo(260, 190);
+    path.lineTo(260, 250);
+    path.lineTo(200, 250);
+    path.lineTo(200, 170);
+    path.close();
+    testSimplify(reporter, path, __FUNCTION__);
+    SkPath path3;
+    Simplify(path, &path3);
+    SkPath path2;
+    SkOpBuilder builder;
+    builder.add(path, kUnion_SkPathOp);
+    builder.resolve(&path2);
+    SkBitmap bitmap;
+    int pixelDiff = comparePaths(reporter, __FUNCTION__, path, path2, bitmap);
+    REPORTER_ASSERT(reporter, pixelDiff == 0);
+}

diff --git a/tests/PathOpsSimplifyTest.cpp b/tests/PathOpsSimplifyTest.cpp
index 8da3cab..65a6441 100644 (file)
--- a/tests/PathOpsSimplifyTest.cpp
+++ b/tests/PathOpsSimplifyTest.cpp
@@ -4759,11 +4759,35 @@ static void testArc(skiatest::Reporter* reporter,const char* filename) {
     testSimplify(reporter, path, filename);
 }
 
+static void testIssue3838(skiatest::Reporter* reporter,const char* filename) {
+    SkPath path;
+    path.moveTo(220, 170);
+    path.lineTo(200, 170);
+    path.lineTo(200, 190);
+    path.lineTo(180, 190);
+    path.lineTo(180, 210);
+    path.lineTo(200, 210);
+    path.lineTo(200, 250);
+    path.lineTo(260, 250);
+    path.lineTo(260, 190);
+    path.lineTo(220, 190);
+    path.lineTo(220, 170);
+    path.close();
+    path.moveTo(220, 210);
+    path.lineTo(220, 230);
+    path.lineTo(240, 230);
+    path.lineTo(240, 210);
+    path.lineTo(220, 210);
+    path.close();
+    testSimplify(reporter, path, filename);
+}
+
 static void (*skipTest)(skiatest::Reporter* , const char* filename) = 0;
 static void (*firstTest)(skiatest::Reporter* , const char* filename) = 0;
 static void (*stopTest)(skiatest::Reporter* , const char* filename) = 0;
 
 static TestDesc tests[] = {
+    TEST(testIssue3838),
     TEST(testArc),
     TEST(testTriangle2),
     TEST(testTriangle1),