From 6dc7df69ae6b24c90d231e0d6a4516bf4f1aee2e Mon Sep 17 00:00:00 2001 From: "caryclark@google.com" Date: Thu, 25 Apr 2013 11:51:54 +0000 Subject: [PATCH] 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 --- src/pathops/SkOpEdgeBuilder.cpp | 22 ++++++++++++--------- src/pathops/SkPathOpsOp.cpp | 4 ++-- tests/PathOpsExtendedTest.cpp | 23 +++++++++++++++++++--- tests/PathOpsOpTest.cpp | 43 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 14 deletions(-) diff --git a/src/pathops/SkOpEdgeBuilder.cpp b/src/pathops/SkOpEdgeBuilder.cpp index 56e7c20..d1d7af8 100644 --- 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 --- 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 contourList; MakeContourList(contours, contourList, xorMask == kEvenOdd_PathOpsMask, diff --git a/tests/PathOpsExtendedTest.cpp b/tests/PathOpsExtendedTest.cpp index c5dbceb..4626454 100644 --- 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 --- 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), -- 2.7.4