Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct for (rare) bad tracking of previousIntersectionPoint #65

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

skef
Copy link

@skef skef commented Oct 24, 2022

The single E glyph in prevint.zip exhibits a problem with previousIntersectionPoint tracking in flatten.py:reCurveSubSegments(). This is a real example from a production font but unfortunately I haven't been able to generate other similar cases synthetically. (I tried less hard with this one because the fix seemed straightforward.)

Anyway, it appears that very rarely the previousIntersectionPoint is not on the segment handled next. This is a particular problem for the elif flatSegment[-1] == inputSegment.flat[-1] and flatSegment[0] != inputSegment.flat[0]: code, which will use that point as the start of the spline if it isn't None.

My added code just checks to see if the previousIntersectionPoint is actually on the current inputSegment and if not sets it back to None so that it won't cause problems.

@frankrolf
Copy link

I talked about this with @typemytype, and he was concerned that this commit might create unexpected scenarios for other high-profile users of this repository. He suggested checking back with @anthrotype – do you think you’d have some bandwidth for checking this update in your tools? Thank you :-)

@anthrotype
Copy link
Collaborator

anthrotype commented Oct 2, 2024

Hey Frank, thanks for checking with me. To be honest, it's too hard for me to track all the potential unexpected changes that this patch may produce for fonts built with fontmake (where booleanOperations is still the default remove-overlap backend).
I can see that for the test font attached above the patch does fix a real problem. If @typemytype says this is safe to do in general, feel free to merge and cut a release, and we can always roll back if problems arise.

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.

3 participants