path ops : fix empty-diff bug, op-in-place

add some debugging around reverse diff, inverse
Review URL: https://codereview.chromium.org/13851015

git-svn-id: http://skia.googlecode.com/svn/trunk@8852 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/src/pathops/SkOpEdgeBuilder.cpp b/src/pathops/SkOpEdgeBuilder.cpp
index 56e7c20..d1d7af8 100644 (file)
--- a/src/pathops/SkOpEdgeBuilder.cpp
+++ b/src/pathops/SkOpEdgeBuilder.cpp
@@ -53,9 +53,13 @@ void SkOpEdgeBuilder::finish() {
     fExtra.reset();  // we're done with this
 }
 
-// FIXME:remove once we can access path pts directly
+// Note that copying the points here avoids copying the resulting path later.
+// To allow Op() to take one of the input paths as an output parameter, either the source data
+// must be copied (as implemented below) or the result must be copied.
+// OPTIMIZATION: This copies both sets of input points every time. If the input data was read
+// directly, the output path would only need to be copied if it was also one of the input paths.
 int SkOpEdgeBuilder::preFetch() {
-    SkPath::RawIter iter(*fPath);  // FIXME: access path directly when allowed
+    SkPath::RawIter iter(*fPath);
     SkPoint pts[4];
     SkPath::Verb verb;
     do {
@@ -78,7 +82,11 @@ void SkOpEdgeBuilder::walk() {
     const SkPoint* finalCurveStart = NULL;
     const SkPoint* finalCurveEnd = NULL;
     SkPath::Verb verb;
-    while ((verb = (SkPath::Verb) *verbPtr++) != SkPath::kDone_Verb) {
+    while ((verb = (SkPath::Verb) *verbPtr) != SkPath::kDone_Verb) {
+        if (verbPtr == endOfFirstHalf) {
+            fOperand = true;
+        }
+        verbPtr++;
         switch (verb) {
             case SkPath::kMove_Verb:
                 complete();
@@ -89,7 +97,7 @@ void SkOpEdgeBuilder::walk() {
                     *fExtra.append() = -1;  // start new contour
                 }
                 finalCurveEnd = pointsPtr++;
-                goto nextVerb;
+                continue;
             case SkPath::kLine_Verb:
                 // skip degenerate points
                 if (pointsPtr[-1].fX != pointsPtr[0].fX || pointsPtr[-1].fY != pointsPtr[0].fY) {
@@ -132,7 +140,7 @@ void SkOpEdgeBuilder::walk() {
                     *fExtra.append() = fCurrentContour->addLine(fReducePts.end() - 2);
                 }
                 complete();
-                goto nextVerb;
+                continue;
             default:
                 SkDEBUGFAIL("bad verb");
                 return;
@@ -140,9 +148,5 @@ void SkOpEdgeBuilder::walk() {
         finalCurveStart = &pointsPtr[verb - 1];
         pointsPtr += verb;
         SkASSERT(fCurrentContour);
-    nextVerb:
-        if (verbPtr == endOfFirstHalf) {
-            fOperand = true;
-        }
     }
 }

diff --git a/src/pathops/SkPathOpsOp.cpp b/src/pathops/SkPathOpsOp.cpp
index 07a8bab..a0071a0 100644 (file)
--- a/src/pathops/SkPathOpsOp.cpp
+++ b/src/pathops/SkPathOpsOp.cpp
@@ -228,10 +228,8 @@ static const bool gOutInverse[kReverseDifference_PathOp + 1][2][2] = {
 
 void Op(const SkPath& one, const SkPath& two, SkPathOp op, SkPath* result) {
     op = gOpInverse[op][one.isInverseFillType()][two.isInverseFillType()];
-    result->reset();
     SkPath::FillType fillType = gOutInverse[op][one.isInverseFillType()][two.isInverseFillType()]
             ? SkPath::kInverseEvenOdd_FillType : SkPath::kEvenOdd_FillType;
-    result->setFillType(fillType);
     const SkPath* minuend = &one;
     const SkPath* subtrahend = &two;
     if (op == kReverseDifference_PathOp) {
@@ -249,6 +247,8 @@ void Op(const SkPath& one, const SkPath& two, SkPathOp op, SkPath* result) {
     const int xorMask = builder.xorMask();
     builder.addOperand(*subtrahend);
     builder.finish();
+    result->reset();
+    result->setFillType(fillType);
     const int xorOpMask = builder.xorMask();
     SkTDArray<SkOpContour*> contourList;
     MakeContourList(contours, contourList, xorMask == kEvenOdd_PathOpsMask,

diff --git a/tests/PathOpsExtendedTest.cpp b/tests/PathOpsExtendedTest.cpp
index c5dbceb..4626454 100644 (file)
--- a/tests/PathOpsExtendedTest.cpp
+++ b/tests/PathOpsExtendedTest.cpp
@@ -29,6 +29,7 @@ static const char* opStrs[] = {
     "kIntersect_PathOp",
     "kUnion_PathOp",
     "kXor_PathOp",
+    "kReverseDifference_PathOp",
 };
 
 static const char* opSuffixes[] = {
@@ -43,7 +44,7 @@ static bool gComparePaths = true;
 static bool gComparePathsAssert = true;
 static bool gPathStrAssert = true;
 
-static void showPathContour(SkPath::Iter& iter) {
+static void showPathContours(SkPath::Iter& iter) {
     uint8_t verb;
     SkPoint pts[4];
     while ((verb = iter.next(pts)) != SkPath::kDone_Verb) {
@@ -77,6 +78,13 @@ void showPath(const SkPath& path, const char* str) {
     showPath(path);
 }
 
+const char* fillTypeStr[] = {
+    "kWinding_FillType",
+    "kEvenOdd_FillType",
+    "kInverseWinding_FillType",
+    "kInverseEvenOdd_FillType"
+};
+
 void showPath(const SkPath& path) {
     SkPath::Iter iter(path, true);
 #define SUPPORT_RECT_CONTOUR_DETECTION 0
@@ -97,8 +105,11 @@ void showPath(const SkPath& path) {
         return;
     }
 #endif
+    SkPath::FillType fillType = path.getFillType();
+    SkASSERT(fillType >= SkPath::kWinding_FillType && fillType <= SkPath::kInverseEvenOdd_FillType);
+    SkDebugf("path.setFillType(%s);\n", fillTypeStr[fillType]);
     iter.setPath(path, true);
-    showPathContour(iter);
+    showPathContours(iter);
 }
 
 void showPathData(const SkPath& path) {
@@ -145,6 +156,9 @@ void showOp(const SkPathOp op) {
         case kXOR_PathOp:
             SkDebugf("op xor\n");
             break;
+        case kReverseDifference_PathOp:
+            SkDebugf("op reverse difference\n");
+            break;
         default:
             SkASSERT(0);
     }
@@ -356,10 +370,13 @@ static int comparePaths(skiatest::Reporter* reporter, const SkPath& one, const S
     int errors2x2;
     int errors = pathsDrawTheSame(bitmap, scaledOne, scaledTwo, errors2x2);
     if (errors2x2 == 0) {
+        if (gShowPath) {
+            showPathOpPath(one, two, a, b, scaledOne, scaledTwo, shapeOp, scale);
+        }
         return 0;
     }
     const int MAX_ERRORS = 8;
-    if (errors2x2 == MAX_ERRORS || errors2x2 == MAX_ERRORS - 1) {
+    if (gShowPath || errors2x2 == MAX_ERRORS || errors2x2 == MAX_ERRORS - 1) {
         showPathOpPath(one, two, a, b, scaledOne, scaledTwo, shapeOp, scale);
     }
     if (errors2x2 > MAX_ERRORS && gComparePathsAssert) {

diff --git a/tests/PathOpsOpTest.cpp b/tests/PathOpsOpTest.cpp
index 3c8765f..995bf16 100644 (file)
--- a/tests/PathOpsOpTest.cpp
+++ b/tests/PathOpsOpTest.cpp
@@ -1144,9 +1144,52 @@ static void cubicOp69d(skiatest::Reporter* reporter) {
     testPathOp(reporter, path, pathB, kDifference_PathOp);
 }
 
+SkPathOp ops[] = {
+    kUnion_PathOp,
+    kXOR_PathOp,
+    kReverseDifference_PathOp,
+    kXOR_PathOp,
+    kReverseDifference_PathOp,
+};
+
+static void rRect1(skiatest::Reporter* reporter) {
+        SkScalar xA = SkFloatToScalar(0.65f);
+        SkScalar xB = SkFloatToScalar(10.65f);
+        SkScalar xC = SkFloatToScalar(20.65f);
+        SkScalar xD = SkFloatToScalar(30.65f);
+        SkScalar xE = SkFloatToScalar(40.65f);
+        SkScalar xF = SkFloatToScalar(50.65f);
+
+        SkScalar yA = SkFloatToScalar(0.65f);
+        SkScalar yB = SkFloatToScalar(10.65f);
+        SkScalar yC = SkFloatToScalar(20.65f);
+        SkScalar yD = SkFloatToScalar(30.65f);
+        SkScalar yE = SkFloatToScalar(40.65f);
+        SkScalar yF = SkFloatToScalar(50.65f);
+        SkPath paths[5];
+        SkRect rects[5];
+        rects[0].set(xB, yB, xE, yE);
+        paths[0].addRoundRect(rects[0], SkIntToScalar(5), SkIntToScalar(5));  // red
+        rects[1].set(xA, yA, xD, yD);
+        paths[1].addRoundRect(rects[1], SkIntToScalar(5), SkIntToScalar(5));  // green
+        rects[2].set(xC, yA, xF, yD);
+        paths[2].addRoundRect(rects[2], SkIntToScalar(5), SkIntToScalar(5));  // blue
+        rects[3].set(xA, yC, xD, yF);
+        paths[3].addRoundRect(rects[3], SkIntToScalar(5), SkIntToScalar(5));  // yellow
+        rects[4].set(xC, yC, xF, yF);
+        paths[4].addRoundRect(rects[4], SkIntToScalar(5), SkIntToScalar(5));  // cyan
+        SkPath path;
+        path.setFillType(SkPath::kInverseEvenOdd_FillType);
+        for (int index = 0; index < 5; ++index) {
+            testPathOp(reporter, path, paths[index], ops[index]);
+            Op(path, paths[index], ops[index], &path);
+        }
+}
+
 static void (*firstTest)(skiatest::Reporter* ) = 0;
 
 static struct TestDesc tests[] = {
+    TEST(rRect1),
     TEST(cubicOp69d),
     TEST(cubicOp68u),
     TEST(cubicOp67u),