use reversePathTo in place of addPathReverse

Path ops was using addPathReverse, instead of reversePathTo.
The former adds a moveTo always, and the latter requires
that the caller add the moveTo if needed.

Simplify the reversePathTo implementation.

R=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2481463002

Committed: https://skia.googlesource.com/skia/+/d8db392be9dd1887df04b10b5670991d6b098c17
Review-Url: https://codereview.chromium.org/2481463002

diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index 46cb15c..36dfdfe 100644 (file)
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -1190,6 +1190,8 @@ private:
     friend class SkAutoPathBoundsUpdate;
     friend class SkAutoDisableOvalCheck;
     friend class SkAutoDisableDirectionCheck;
+    friend class SkPathWriter;
+    friend class SkOpBuilder;
     friend class SkBench_AddPathTest; // perf test reversePathTo
     friend class PathTest_Private; // unit test reversePathTo
     friend class ForceIsRRect_Private; // unit test isRRect

diff --git a/include/pathops/SkPathOps.h b/include/pathops/SkPathOps.h
index fa01788..1e8093f 100644 (file)
--- a/include/pathops/SkPathOps.h
+++ b/include/pathops/SkPathOps.h
@@ -91,6 +91,8 @@ private:
     SkTArray<SkPath> fPathRefs;
     SkTDArray<SkPathOp> fOps;
 
+    static bool FixWinding(SkPath* path);
+    static void ReversePath(SkPath* path);
     void reset();
 };
 

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index c1b6327..8241c10 100644 (file)
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -1567,49 +1567,41 @@ static int pts_in_verb(unsigned verb) {
 
 // ignore the last point of the 1st contour
 void SkPath::reversePathTo(const SkPath& path) {
-    int i, vcount = path.fPathRef->countVerbs();
-    // exit early if the path is empty, or just has a moveTo.
-    if (vcount < 2) {
+    const uint8_t* verbs = path.fPathRef->verbsMemBegin(); // points at the last verb
+    if (!verbs) {  // empty path returns nullptr
         return;
     }
+    const uint8_t* verbsEnd = path.fPathRef->verbs() - 1; // points just past the first verb
+    SkASSERT(verbsEnd[0] == kMove_Verb);
+    const SkPoint*  pts = path.fPathRef->pointsEnd() - 1;
+    const SkScalar* conicWeights = path.fPathRef->conicWeightsEnd();
 
-    SkPathRef::Editor(&fPathRef, vcount, path.countPoints());
-
-    const uint8_t*  verbs = path.fPathRef->verbs();
-    const SkPoint*  pts = path.fPathRef->points();
-    const SkScalar* conicWeights = path.fPathRef->conicWeights();
-
-    SkASSERT(verbs[~0] == kMove_Verb);
-    for (i = 1; i < vcount; ++i) {
-        unsigned v = verbs[~i];
-        int n = pts_in_verb(v);
-        if (n == 0) {
-            break;
-        }
-        pts += n;
-        conicWeights += (SkPath::kConic_Verb == v);
-    }
-
-    while (--i > 0) {
-        switch (verbs[~i]) {
+    while (verbs < verbsEnd) {
+        uint8_t v = *verbs++;
+        pts -= pts_in_verb(v);
+        switch (v) {
+            case kMove_Verb:
+                // if the path has multiple contours, stop after reversing the last
+                return;
             case kLine_Verb:
-                this->lineTo(pts[-1].fX, pts[-1].fY);
+                this->lineTo(pts[0]);
                 break;
             case kQuad_Verb:
-                this->quadTo(pts[-1].fX, pts[-1].fY, pts[-2].fX, pts[-2].fY);
+                this->quadTo(pts[1], pts[0]);
                 break;
             case kConic_Verb:
-                this->conicTo(pts[-1], pts[-2], *--conicWeights);
+                this->conicTo(pts[1], pts[0], *--conicWeights);
                 break;
             case kCubic_Verb:
-                this->cubicTo(pts[-1].fX, pts[-1].fY, pts[-2].fX, pts[-2].fY,
-                              pts[-3].fX, pts[-3].fY);
+                this->cubicTo(pts[2], pts[1], pts[0]);
+                break;
+            case kClose_Verb:
+                SkASSERT(verbs - path.fPathRef->verbsMemBegin() == 1);
                 break;
             default:
                 SkDEBUGFAIL("bad verb");
                 break;
         }
-        pts -= pts_in_verb(verbs[~i]);
     }
 }
 

diff --git a/src/pathops/SkOpBuilder.cpp b/src/pathops/SkOpBuilder.cpp
index 011d6a6..075520d 100644 (file)
--- a/src/pathops/SkOpBuilder.cpp
+++ b/src/pathops/SkOpBuilder.cpp
@@ -25,7 +25,17 @@ static bool one_contour(const SkPath& path) {
     return true;
 }
 
-bool FixWinding(SkPath* path) {
+void SkOpBuilder::ReversePath(SkPath* path) {
+    SkPath temp;
+    SkPoint lastPt;
+    SkAssertResult(path->getLastPt(&lastPt));
+    temp.moveTo(lastPt);
+    temp.reversePathTo(*path);
+    temp.close();
+    *path = temp;
+}
+
+bool SkOpBuilder::FixWinding(SkPath* path) {
     SkPath::FillType fillType = path->getFillType();
     if (fillType == SkPath::kInverseEvenOdd_FillType) {
         fillType = SkPath::kInverseWinding_FillType;
@@ -35,9 +45,7 @@ bool FixWinding(SkPath* path) {
     SkPathPriv::FirstDirection dir;
     if (one_contour(*path) && SkPathPriv::CheapComputeFirstDirection(*path, &dir)) {
         if (dir != SkPathPriv::kCCW_FirstDirection) {
-            SkPath temp;
-            temp.reverseAddPath(*path);
-            *path = temp;
+            ReversePath(path);
         }
         path->setFillType(fillType);
         return true;
@@ -133,9 +141,7 @@ bool SkOpBuilder::resolve(SkPath* result) {
             if (firstDir == SkPathPriv::kUnknown_FirstDirection) {
                 firstDir = dir;
             } else if (firstDir != dir) {
-                SkPath temp;
-                temp.reverseAddPath(*test);
-                *test = temp;
+                ReversePath(test);
             }
             continue;
         }

diff --git a/src/pathops/SkPathWriter.cpp b/src/pathops/SkPathWriter.cpp
index c94809e..9893a50 100644 (file)
--- a/src/pathops/SkPathWriter.cpp
+++ b/src/pathops/SkPathWriter.cpp
@@ -306,7 +306,7 @@ void SkPathWriter::assemble() {
                         first ? SkPath::kAppend_AddPathMode : SkPath::kExtend_AddPathMode);
             } else {
                 SkASSERT(!first);
-                fPathPtr->reverseAddPath(contour);
+                fPathPtr->reversePathTo(contour);
             }
             if (first) {
                 first = false;