From 8ced56f05f2cb947551d9c70f6a43c1eff32686f Mon Sep 17 00:00:00 2001
From: Michael Ludwig <michaelludwig@google.com>
Date: Mon, 19 Apr 2021 15:17:21 -0400
Subject: [PATCH] Allow Convexicator to implicitly close correctly

In the old code, fFirstPt actually pointed to the first added point that
came *after* a call to setMovePt. The moved-to point that started the
contour was never remembered in the Convexicator, so it required the
SkPath::Iter to force close. When the iterator force-closed, it made
sure to add a line-to back to the start of the contour, which it could
do because the Iter was also tracking the contours.
 - It's important that the old code's close()'s addPt(fFirstPt) actually
   re-added the first point after the move-to because it catches
   concavities that occur at vertex that started the contour. The first
   time that point is added, we have no initial vector to check for
   direction changes against.

In the new code, I've updated the state tracking to be more straight
forward. fCurrPt and fLastPt were redundant (they always equaled each
other once addPt returned). fPriorPt was also redundant when combined
with the expected direction enum, since it's only use was to determine
when to cache the initial vector. I also found it confusing to know
the difference between fLastPt and fPriorPt based just on variable
names.

Now fFirstPt refers to the location of the moveTo/"start of contour",
and fFirstVec is the direction vector leaving that point to the next
point in the contour. fLastPt is the point that was last passed to
addPt and fLastVec is the direction that comes into 'fLastPt'.  The
close() function is updated to add a point back to fFirstPt (which does
nothing if the close is explicit, or handled by SkPath::Iter) AND it
checks for a dir change on the first vector.

This shouldn't change any behavior, so I wanted it as a standalone
change, but it will allow
https://skia-review.googlesource.com/c/skia/+/396056/ to
update computeConvexity() to move away from
SkPath::Iter(forceClose=true).

Bug: skia:1187385
Change-Id: Iff383693956b28e3c21b23e5a5c6ce814227bb27
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398018
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
---
 src/core/SkPath.cpp | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 9816e3f286..aa5be38fe2 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -2088,22 +2088,22 @@ struct Convexicator {
     SkPathFirstDirection getFirstDirection() const { return fFirstDirection; }
 
     void setMovePt(const SkPoint& pt) {
-        fPriorPt = fLastPt = fCurrPt = pt;
+        fFirstPt = fLastPt = pt;
+        fExpectedDir = kInvalid_DirChange;
     }
 
     bool addPt(const SkPoint& pt) {
-        if (fCurrPt == pt) {
+        if (fLastPt == pt) {
             return true;
         }
-        fCurrPt = pt;
-        if (fPriorPt == fLastPt) {  // should only be true for first non-zero vector
-            fLastVec = fCurrPt - fLastPt;
-            fFirstPt = pt;
-        } else if (!this->addVec(fCurrPt - fLastPt)) {
+        // should only be true for first non-zero vector after setMovePt was called.
+        if (fFirstPt == fLastPt && fExpectedDir == kInvalid_DirChange) {
+            fLastVec = pt - fLastPt;
+            fFirstVec = fLastVec;
+        } else if (!this->addVec(pt - fLastPt)) {
             return false;
         }
-        fPriorPt = fLastPt;
-        fLastPt = fCurrPt;
+        fLastPt = pt;
         return true;
     }
 
@@ -2144,7 +2144,10 @@ struct Convexicator {
     }
 
     bool close() {
-        return this->addPt(fFirstPt);
+        // If this was an explicit close, there was already a lineTo to fFirstPoint, so this
+        // addPt() is a no-op. Otherwise, the addPt implicitly closes the contour. In either case,
+        // we have to check the direction change along the first vector in case it is concave.
+        return this->addPt(fFirstPt) && this->addVec(fFirstVec);
     }
 
     bool isFinite() const {
@@ -2200,15 +2203,16 @@ private:
         return true;
     }
 
-    SkPoint             fFirstPt {0, 0};
-    SkPoint             fPriorPt {0, 0};
-    SkPoint             fLastPt {0, 0};
-    SkPoint             fCurrPt {0, 0};
-    SkVector            fLastVec {0, 0};
-    DirChange           fExpectedDir { kInvalid_DirChange };
-    SkPathFirstDirection   fFirstDirection { SkPathFirstDirection::kUnknown };
-    int                 fReversals { 0 };
-    bool                fIsFinite { true };
+    SkPoint              fFirstPt {0, 0};  // The first point of the contour, e.g. moveTo(x,y)
+    SkVector             fFirstVec {0, 0}; // The direction leaving fFirstPt to the next vertex
+
+    SkPoint              fLastPt {0, 0};   // The last point passed to addPt()
+    SkVector             fLastVec {0, 0};  // The direction that brought the path to fLastPt
+
+    DirChange            fExpectedDir { kInvalid_DirChange };
+    SkPathFirstDirection fFirstDirection { SkPathFirstDirection::kUnknown };
+    int                  fReversals { 0 };
+    bool                 fIsFinite { true };
 };
 
 SkPathConvexity SkPath::computeConvexity() const {