8000 TYP: Make glyph indices distinct from character codes by QuLogic · Pull Request #30143 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

TYP: Make glyph indices distinct from character codes #30143

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

Open
wants to merge 2 commits into
base: text-overhaul
Choose a base branch
from

Conversation

QuLogic
Copy link
Member
@QuLogic QuLogic commented Jun 4, 2025

PR summary

Previously, these were both typed as int, which means you could mix and match them erroneously. While the character code can't be made a distinct type (because it's used for chr/ord), typing glyph indices as a distinct type means these can't be fully swapped.

Unfortunately, you can still go back to the base type, so glyph indices still work as character codes. But this is still sufficient to catch errors such as the wrong call to FT2Font.get_kerning in _mathtext.py.

This depends on #30134.

PR checklist

@QuLogic
Copy link
Member Author
QuLogic commented Jun 4, 2025

But this is still sufficient to catch errors such as the wrong call to FT2Font.get_kerning in _mathtext.py.

Note, fixing this would require updating 20 images (for very minimal changes), which I originally rolled into #29816, but it could be moved here.

@QuLogic QuLogic moved this to Waiting for other PR in Font and text overhaul Jun 5, 2025
@QuLogic QuLogic added this to the v3.11.0 milestone Jun 5, 2025
@QuLogic
Copy link
Member Author
QuLogic commented Jun 6, 2025

Since #29816 is now just the FreeType change, I've moved the kerning correction here.

@QuLogic QuLogic changed the base branch from main to text-overhaul June 11, 2025 02:57
@QuLogic QuLogic force-pushed the character-glyph-types branch from 387a3c1 to 755eab1 Compare June 19, 2025 22:17
@QuLogic QuLogic force-pushed the character-glyph-types branch from 755eab1 to cc4ede1 Compare July 8, 2025 05:38
QuLogic added 2 commits July 9, 2025 02:13
Previously, these were both typed as `int`, which means you could mix
and match them erroneously. While the character code can't be made a
distinct type (because it's used for `chr`/`ord`), typing glyph indices
as a distinct type means these can't be fully swapped.

Unfortunately, you can still go back to the base type, so glyph indices
still work as character codes. But this is still sufficient to catch
errors such as the wrong call to `FT2Font.get_kerning` in
`_mathtext.py`.
The `FontInfo.num` value returned by `TruetypeFonts._get_info` is a
character code, but `FT2Font.get_kerning` takes *glyph indices*, meaning
that kerning was likely off in most cases.
@QuLogic QuLogic force-pushed the character-glyph-types branch from cc4ede1 to 73bd5f5 Compare July 9, 2025 06:13
QuLogic marked this pull request as ready for review July 9, 2025 06:14
@QuLogic QuLogic moved this from Waiting for other PR to Ready for Review in Font and text overhaul Jul 9, 2025
@QuLogic
Copy link
Member Author
QuLogic commented Jul 9, 2025

There aren't too many test image changes here, so I'm debating whether to merge them in, but we've generally agreed not to until the whole feature branch is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Ready for Review
Development

Successfully merging this pull request may close these issues.

1 participant
0