Set SkPath.fLastMoveToIndex field from SkPathBuilder

This is a "legacy" field in SkPath, and only needed for editing the
path (in funny cases, such as a relative verb or missing moveto).
When we finally make SkPath immutable, we won't need this field at all.

Note: this CL "fixes" the last 2 columns in path_append_extend gm.
They should appear the same as the previous 2 columns.

Change-Id: Ia5f2e8ec586b5f5189fc3ac2cd513fe89d31cd22
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436958
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>

include/core/SkPathBuilder.h

@@ -228,6 +228,7 @@ private:
 
     unsigned    fSegmentMask;
     SkPoint     fLastMovePoint;
+    int         fLastMoveIndex; // only needed until SkPath is immutable
     bool        fNeedsMoveVerb;
 
     enum IsA {

src/core/SkPathBuilder.cpp

@@ -41,6 +41,7 @@ SkPathBuilder& SkPathBuilder::reset() {
 
     fSegmentMask = 0;
     fLastMovePoint = {0, 0};
+    fLastMoveIndex = -1;        // illegal
     fNeedsMoveVerb = true;
 
     // testing
@@ -85,6 +86,9 @@ SkRect SkPathBuilder::computeBounds() const {
  */
 
 SkPathBuilder& SkPathBuilder::moveTo(SkPoint pt) {
+    // only needed while SkPath is mutable
+    fLastMoveIndex = SkToInt(fPts.size());
+
     fPts.push_back(pt);
     fVerbs.push_back((uint8_t)SkPathVerb::kMove);
 
@@ -206,7 +210,20 @@ SkPath SkPathBuilder::make(sk_sp<SkPathRef> pr) const {
     //  unknown, convex_cw, convex_ccw, concave
     // Do we ever have direction w/o convexity, or viceversa (inside path)?
     //
-    return SkPath(std::move(pr), fFillType, fIsVolatile, convexity, dir);
+    auto path = SkPath(std::move(pr), fFillType, fIsVolatile, convexity, dir);
+
+    // This hopefully can go away in the future when Paths are immutable,
+    // but if while they are still editable, we need to correctly set this.
+    const uint8_t* start = path.fPathRef->verbsBegin();
+    const uint8_t* stop  = path.fPathRef->verbsEnd();
+    if (start < stop) {
+        SkASSERT(fLastMoveIndex >= 0);
+        // peek at the last verb, to know if our last contour is closed
+        const bool isClosed = (stop[-1] == (uint8_t)SkPathVerb::kClose);
+        path.fLastMoveToIndex = isClosed ? ~fLastMoveIndex : fLastMoveIndex;
+    }
+
+    return path;
 }
 
 SkPath SkPathBuilder::snapshot() const {

src/core/SkPathPriv.h

@@ -333,6 +333,8 @@ public:
         return true;
     }
 
+    static int LastMoveToIndex(const SkPath& path) { return path.fLastMoveToIndex; }
+
     static bool IsRectContour(const SkPath&, bool allowPartial, int* currVerb,
                               const SkPoint** ptsPtr, bool* isClosed, SkPathDirection* direction,
                               SkRect* rect);

tests/PathBuilderTest.cpp

@@ -320,3 +320,38 @@ DEF_TEST(pathbuilder_addPath, reporter) {
 
     REPORTER_ASSERT(reporter, p == SkPathBuilder().addPath(p).detach());
 }
+
+/*
+ *  If paths were immutable, we would not have to track this, but until that day, we need
+ *  to ensure that paths are built correctly/consistently with this field, regardless of
+ *  either the classic mutable apis, or via SkPathBuilder (SkPath::Polygon uses builder).
+ */
+DEF_TEST(pathbuilder_lastmoveindex, reporter) {
+    const SkPoint pts[] = {
+        {0, 1}, {2, 3}, {4, 5},
+    };
+    constexpr int N = (int)SK_ARRAY_COUNT(pts);
+
+    for (int ctrCount = 1; ctrCount < 4; ++ctrCount) {
+        const int lastMoveToIndex = (ctrCount - 1) * N;
+
+        for (bool isClosed : {false, true}) {
+            SkPath a, b;
+
+            SkPathBuilder builder;
+            for (int i = 0; i < ctrCount; ++i) {
+                builder.addPolygon(pts, N, isClosed);  // new-school way
+                b.addPoly(pts, N, isClosed);        // old-school way
+            }
+            a = builder.detach();
+
+            // We track the last moveTo verb index, and we invert it if the last verb was a close
+            const int expected = isClosed ? ~lastMoveToIndex : lastMoveToIndex;
+            const int a_last = SkPathPriv::LastMoveToIndex(a);
+            const int b_last = SkPathPriv::LastMoveToIndex(b);
+
+            REPORTER_ASSERT(reporter, a_last == expected);
+            REPORTER_ASSERT(reporter, b_last == expected);
+        }
+    }
+}