Improved mergeFirstSegments handling #67
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a set of changes I feel more confident about, in that:
An earlier version of this PR was filed as a draft because it produced extra zero length lines. After looking at that in more depth I realized I was over-complicating the fix. This version is simpler and should be in better shape.
Some notes about what's going on here.
All this code at the start is worried about whether the first and last parts of segmentedFlatPoints has been split at a bad location. If it has been you'll get extra line segments corresponding to the last bit a curve, which ideally you'd like to avoid. (There may be other problems -- I haven't gone too far down that path of exploration.)
If you join the segments under the wrong circumstances, though, you'll wind up with missing points on the contour. When the missing point is part of a curve that's because of this sort of code:
if
flatSegment[-1]
is some point actually on another segment, we both use a weird t value and, more importantly, don't end the segment at its actual end, resulting in a missing point.(You get different problems when merging line segments in certain cases.)
The problem with the earlier code is that it is sometimes merging segments that are "turned around". If
fp == lastInputSegment.scaledPreviousOnCurve
andlp
is inflatInputPoints
andlastInputSegment
,lp
can be atlastInputSegment.flat[0]
orlastInputSegment.flat[-1]
. It's only correct to merge in the former case.