GeezerVison is showing github.com — Rework mapping of dvi glyph indices to freetype indices. by anntzer · Pull Request #29829 · matplotlib/matplotlib
Conversation
anntzer
commented
Mar 29, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
In 89a7e19, an API for converting "dvi glyph indices" (as stored in a dvi file) to FreeType-compatible keys (either "indices into the native charmap" or "glyph names") was introduced. It was intended that end users (i.e., backends) would check the type of text.glyph_name_or_index ((A) int or (B) str) and load the glyph accordingly ((A) FT_Set_Charmap(native_cmap); FT_Load_Char(index); or (B) FT_Load_Glyph(FT_Get_Name_Index(name));); however, with the future introduction of {xe,lua}tex support, this kind of type checking becomes inconvenient, because {xe,lua}tex's "dvi glyph indices", which are directly equal to FreeType glyph indices (i.e. they would be loaded with FT_Load_Glyph(index);), would normally also be converted to ints.
This PR introduces a new API (Text.index) that performs this mapping (via the new DviFont._index_dvi_to_freetype), always mapping to FreeType glyph indices (i.e. one can always just call FT_Load_Glyph on the result). To do so, in case (A) it loads itself the native charmap (something the end user needed to do by themselves previously) and performs the cmap-to-index conversion (FT_Get_Char_Index) previously implicit in FT_Load_Char; in case (B) it performs itself the name-to-index conversion (FT_Get_Name_Index). When {xe,lua}tex support is introduced in the future, _index_dvi_to_freetype will just return the index as is.
The old APIs are not deprecated yet, as other changes will occur for {xe,lua}tex support (e.g. font_effects will go away and be replaced by .font.effects, as {xe,lua}tex don't store that info in the pdftexmap entry), and grouping all API changes together seems nicer (to only require a single adaptation step by the API consumer).
PR summary
PR checklist
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
In 89a7e19, an API for converting "dvi glyph indices" (as stored
in a dvi file) to FreeType-compatible keys (either "indices into
the native charmap" or "glyph names") was introduced. It was
intended that end users (i.e., backends) would check the type of
`text.glyph_name_or_index` ((A) int or (B) str) and load the glyph
accordingly ((A) `FT_Set_Charmap(native_cmap); FT_Load_Char(index);` or
(B) `FT_Load_Glyph(FT_Get_Name_Index(name));`); however, with the future
introduction of {xe,lua}tex support, this kind of type checking becomes
inconvenient, because {xe,lua}tex's "dvi glyph indices", which are
directly equal to FreeType glyph indices (i.e. they would be loaded with
`FT_Load_Glyph(index);`), would normally also be converted to ints.
This PR introduces a new API (`Text.index`) that performs this mapping
(via the new `DviFont._index_dvi_to_freetype`), always mapping to
FreeType glyph indices (i.e. one can always just call `FT_Load_Glyph`
on the result). To do so, in case (A) it loads itself the native
charmap (something the end user needed to do by themselves previously)
and performs the cmap-to-index conversion (`FT_Get_Char_Index`)
previously implicit in `FT_Load_Char`; in case (B) it performs itself
the name-to-index conversion (`FT_Get_Name_Index`). When {xe,lua}tex
support is introduced in the future, `_index_dvi_to_freetype` will
just return the index as is.
The old APIs are not deprecated yet, as other changes will occur for
{xe,lua}tex support (e.g. font_effects will go away and be replaced by
`.font.effects`, as {xe,lua}tex don't store that info in the pdftexmap
entry), and grouping all API changes together seems nicer (to only
require a single adaptation step by the API consumer).
Comment on lines
+649
to
+651
|
|
# - if no ".enc" file is specified, then the font must be a Type 1
|
|
|
# font, and dvi indices directly index into the font's CharStrings
|
|
|
# vector.
|
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.
just checking but is this why this step is no longer needed: index = t1_encodings[font][glyph_name_or_index]
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
This still happens at the bottom: self._encoding = face._get_t1_encoding_vector() (likewise caching the value of the encoding vector, but this now occurs on the DviFont object) then return self._encoding[idx].
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
if self._encoding is None:
|
|
|
psfont = PsfontsMap(find_tex_file("pdftex.map"))[self.texname]
|
|
|
if psfont.filename is None:
|
|
|
raise ValueError("No usable font file found for {} ({}); "
|
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.
Can you please add a test for coverage?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
anntzer
May 7, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
QuLogic
QuLogic approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|