GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — TYP: Make glyph indices distinct from character codes by QuLogic · Pull Request #30143 · matplotlib/matplotlib

Conversation

@QuLogic

@QuLogic QuLogic commented Jun 4, 2025

Copy link
Copy Markdown
Member

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

  • [n/a] "closes #0000" is in the body of the PR description to link the related issue
  • new and changed code is tested
  • [n/a] Plotting related features are demonstrated in an example
  • New Features and API Changes are noted with a directive and release note
  • Documentation complies with general and docstring guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@QuLogic QuLogic added the status: waiting for other PR label Jun 4, 2025
@github-actions github-actions Bot added backend: ps topic: text/fonts topic: text/mathtext labels Jun 4, 2025
@QuLogic

QuLogic commented Jun 4, 2025

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic QuLogic added this to Font and text overhaul Jun 5, 2025
@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

QuLogic commented Jun 6, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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 2 times, most recently from cc4ede1 to 73bd5f5 Compare July 9, 2025 06:13
@QuLogic QuLogic removed the status: waiting for other PR label Jul 9, 2025
@github-actions github-actions Bot removed the backend: ps label Jul 9, 2025
@QuLogic
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

QuLogic commented Jul 9, 2025

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@QuLogic
QuLogic force-pushed the character-glyph-types branch 2 times, most recently from 6a03850 to 1cbf39e Compare July 19, 2025 07:32
@QuLogic QuLogic mentioned this pull request Jul 19, 2025
Use glyph indices for font tracking in vector formats #30335
Merged
1 task done
QuLogic added 3 commits July 24, 2025 15:37
@QuLogic
TYP: Make glyph indices distinct from character codes
3a0a773 
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`.
@QuLogic
Fix kerning of mathtext
c6e6904 
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
Update test images for previous changes
733cd7d
@QuLogic
QuLogic force-pushed the character-glyph-types branch from 1cbf39e to 733cd7d Compare July 24, 2025 19:46
ksunden
ksunden approved these changes Jul 24, 2025
View reviewed changes

@ksunden ksunden left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May have slightly preferred the actual output change to be separated from here, but not too fussed in total.

There are only 5 png changes here, the rest are svg, which I'm not concerned about as that is text changes rather than binary files.

Thus I'm good with the scale of change in a PR without needing to group these changes.

But the typing changes look good, are well reasoned, and look correct.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@QuLogic

QuLogic commented Jul 24, 2025

Copy link
Copy Markdown
Member Author

May have slightly preferred the actual output change to be separated from here, but not too fussed in total.

I can drop that and it'll be rolled into the final PR, so that should be fine. This is just for CI, really.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ksunden

ksunden commented Jul 25, 2025

Copy link
Copy Markdown
Member

Like I said, not too fussed, it was easy to see which lines actually changed, just slight preference. I don't think it is worth rolling back though.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
tacaswell
tacaswell approved these changes Jul 31, 2025
View reviewed changes
@tacaswell
tacaswell merged commit 56308af into matplotlib:text-overhaul Jul 31, 2025
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Font and text overhaul Jul 31, 2025
@QuLogic
QuLogic deleted the character-glyph-types branch July 31, 2025 20:05
wavebyrd pushed a commit to wavebyrd/matplotlib that referenced this pull request Mar 13, 2026
@tacaswell
Merge pull request matplotlib#30143 from QuLogic/character-glyph-types
eefcf29 
TYP: Make glyph indices distinct from character codes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell approved these changes
@ksunden ksunden ksunden approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic: text/fonts topic: text/mathtext

Projects

Font and text overhaul
Status: Done

Milestone

v3.11.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@QuLogic @ksunden @tacaswell
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.