Skip to content

Fix extraneous on-curve points after remove overlap - #73

Open
skef wants to merge 1 commit into
typemytype:masterfrom
skef:chordcheck
Open

skef wants to merge 1 commit into
typemytype:masterfrom
skef:chordcheck

Conversation

@skef

@skef skef commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #72.

The check I added in #65 drops previousIntersectionPoint when it isn't on
the segment being processed. It was also dropping points that were genuinely
on it. The start of the segment then got re-derived from the next point in
the flattened outline, so an inside corner came out as two on-curve points a
few units apart.

When checking this case we want to know whether a point is on a segment. My
previous fix answered that by recovering a t corresponding to the point,
calculating that point on the curve, and measuring the distance. However, in
the present (pre-existing) function t is recovered by matching a single
axis, which can be quite inaccurate for "steep" curves.

We could, and perhaps should, improve that function, but it's preferable to
change the code to compare against the flattened outline instead, which is
what this updated code does. Clipper computes its intersection points from
the flattened chords, so each point will lie exactly on a chord before
rounding and within less than an integer distance after. Measuring against
the original curve involves an extra degree of approximation: the rounding,
and then the distance of the chords from the curve. Checking distances from
chords also enables a quick rejection based on bounding-box.

Also adds a regression test covering the two contours from #72, and removes
tValueToPoint() and _getLinePoint(), which #65 added only to support the
old implementation of hasPoint() and are now unused.

Testing

  • The E glyph from Correct for (rare) bad tracking of previousIntersectionPoint #65 is unchanged — that guard still fires there, and the
    output is byte-identical to master.
  • Both contours from Extraneous points after remove overlap #72 now produce a single on-curve point at the corner.
  • No output change across 1231 glyphs of Libertinus Serif.
  • Over 3693 forced-overlap cases (each glyph unioned with shifted copies of
    itself), the old check discarded a point 8700 times and the new one 0 —
    the misfire was not rare, it just needed overlapping curves to show up.
  • Slightly faster than before: 4.98 → 3.14 µs per call, and about 2.5% off
    the full-corpus union.

The check added in typemytype#65, which drops previousIntersectionPoint when it is
not on the segment being processed, was also dropping points that were
genuinely on it. The start of the segment was then re-derived from the
next point in the flattened outline, so an inside corner came out as two
on-curve points a few units apart (typemytype#72).

When checking this case we want to know whether a point is on a segment.
My previous fix answered that by recovering a t corresponding to the
point, calculating that point on the curve, and measuring the distance.
However, in the present (pre-existing) function t is recovered by
matching a single axis, which can be quite inaccurate for "steep" curves.

We could, and perhaps should, improve that function, but it's preferable
to change the code to compare against the flattened outline instead,
which is what this updated code does. Clipper computes its intersection
points from the flattened chords, so each point will lie exactly on a
chord before rounding and within less than an integer distance after.
Measuring against the original curve involves an extra degree of
approximation: the rounding, and then the distance of the chords from
the curve. Checking distances from chords also enables a quick rejection
based on bounding-box.

Also adds a regression test covering the two contours from typemytype#72, and
removes tValueToPoint() and _getLinePoint(), which typemytype#65 added only to
support the old implementation of hasPoint() and which are now unused.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@skef

skef commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

On the "could, and perhaps should" above — I tried it. Solving both axes and
keeping whichever root lands nearest does reduce the recovery error
substantially (p99 1.31 → 0.16 units). But it doesn't improve the output:
across 200 glyphs the mean deviation of the result from the input outline
moves by about 1–3%, and the sign depends on which statistic you use. The
gains are real but confined to round shapes with steep sections (circle
0.105 → 0.026), and they're offset by small scattered regressions elsewhere.

The reason is that t only positions the two control points — the endpoints
are overwritten with the exact intersection points regardless — so the error
floor is set by the flattening, which no choice of t changes. Against that,
it alters output for 88% of glyphs and needs 8 golden tests regenerated, so
I've left it alone.

@ryanbugden

Copy link
Copy Markdown

Thank you for looking at this so quickly @skef!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extraneous points after remove overlap

2 participants