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>
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 9816e3f..aa5be38 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -2088,22 +2088,22 @@
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 @@
}
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 @@
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 {
