less clever skipMoveTo logic

Replace the old, possibly buggy logic with something that I think is too
simple to be wrong: we need to moveTo(pts[0]) if pts[0] is not where we
currently are.

Either the old comments were wrong, or the old code was wrong, or I'm
missing how the two connect.  In any case, this new version of the logic
for skipping moveTo() makes the GM we added to track this bug draw the same
in debug and release builds, where it was quite wrong in release before.

Bug: skia:9331
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Change-Id: If7c164f0fc62a371e7009bf12d320cc62cb88ef2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/234001
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
Mike Klein 2019-08-12 15:38:58 -04:00 committed by Skia Commit-Bot
parent fbe9e0ee5f
commit fb83927366

View File

@ -195,19 +195,14 @@ static bool cull_path(const SkPath& srcPath, const SkStrokeRec& rec,
// Notice this vector v and accum work with the original unclipped length.
SkVector v = pts[1] - pts[0];
// TODO: this whole bit of skip-moveTo() logic seems wrong to me.
// If the line is entirely outside clip rect, skip it.
bool inY = bounds.fTop <= pts[0].fY && pts[0].fY <= bounds.fBottom,
inX = bounds.fLeft <= pts[0].fX && pts[0].fX <= bounds.fRight ;
if (v.fX ? inY : inX) {
bool skipMoveTo = inX && inY;
if (clip_line(pts, bounds, intervalLength, SkScalarMod(accum, intervalLength))) {
if (accum == 0 || !skipMoveTo) {
dstPath->moveTo(pts[0]);
}
dstPath->lineTo(pts[1]);
if (clip_line(pts, bounds, intervalLength, SkScalarMod(accum, intervalLength))) {
// pts[0] may have just been changed by clip_line().
// If that's not where we ended the previous lineTo(), we need to moveTo() there.
SkPoint last;
if (!dstPath->getLastPt(&last) || last != pts[0]) {
dstPath->moveTo(pts[0]);
}
dstPath->lineTo(pts[1]);
}
// We either just traveled v.fX horizontally or v.fY vertically.