-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Use glyph indices for font tracking in vector formats #30335
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
Conversation
4ca7af0
to
e684f7b
Compare
3d5e48c
to
a2db55c
Compare
a2db55c
to
2118966
Compare
I've decided to restore the character code in the return values from mathtext, because I've found some use for it in PDF output. |
df7fa98
to
8de7f4e
Compare
With libraqm, string layout produces glyph indices, not character codes, and font features may even produce different glyphs for the same character code (e.g., by picking a different Stylistic Set). Thus we cannot rely on character codes as unique items within a font, and must move toward glyph indices everywhere.
8de7f4e
to
f630b48
Compare
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.
Looks like mypy is unhappy for unrelated reasons (beyond the disjoint bases issues) so that can probably be fixed as a separate PR.
I've rebased #30512 as well now. Of course, I wrote the change much after this one, but now it does seem like they go together quite well, so let me know if you want me to move the "pdf/ps: Track full character map in CharacterTracker" commit here to review together. |
I don't really mind either way, the advantage of splitting things in smaller chunks is that once a PR is done we can just forget about it and move on to the next one (whereas if you add another patch here there's no easy way to state "the first commit is ready to go but the second still needs review"). More specifically, it seems to me that the UI of github is not really optimal in that reviews (and the approval process) are "PR-oriented" whereas for these large, multi-step PRs, it would be useful to have "commit-oriented" reviews. Given the current limitations I would suggest splitting the charmap tracking commit into its own PR (on top of this one) as well as I suspect (no guarantees) that it can also be approved quickly. |
Edit: I reviewed that commit and would approve it except for the two doc-related comments I've left. |
OK, I split that into #30566, and fixed the comments as well. |
PR summary
With libraqm, string layout produces glyph indices, not character codes, and font features may even produce different glyphs for the same character code (e.g., by picking a different Stylistic Set). Thus we cannot rely on character codes as unique items within a font, and must move toward glyph indices everywhere.
The only thing I don't quite like is that PDF uses character codes for its lookup, and I have to map glyph indices back through an inverse charmap. I think I may have to send everything throughBetter stuff for this is done in #30512.CharacterTracker
and produce my own limited charmap, but still need to test out what's required.This is based on #30143.PR checklist