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 — Rework mapping of dvi glyph indices to freetype indices. by anntzer · Pull Request #29829 · matplotlib/matplotlib

Conversation

@anntzer

@anntzer anntzer commented Mar 29, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

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

  • "closes #0000" is in the body of the PR description to link the related issue
  • new and changed code is tested
  • 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
@anntzer anntzer added the topic: text/usetex label Mar 29, 2025
@github-actions github-actions Bot added topic: text topic: text/fonts labels Mar 29, 2025
@anntzer
anntzer force-pushed the dvi2ft branch 4 times, most recently from 7a760d1 to 45eae37 Compare April 1, 2025 07:51
@anntzer anntzer mentioned this pull request Apr 1, 2025
Multivariate plotting in imshow, pcolor and pcolormesh #29221
Closed
2 of 5 tasks
@anntzer
anntzer marked this pull request as draft April 6, 2025 08:21
@QuLogic QuLogic added the status: waiting for other PR label Apr 8, 2025
@github-actions github-actions Bot added the status: needs rebase label Apr 15, 2025
@anntzer
anntzer force-pushed the dvi2ft branch from 45eae37 to f24a241 Compare April 15, 2025 06:26
@anntzer
anntzer marked this pull request as ready for review April 15, 2025 06:26
@github-actions github-actions Bot removed topic: text/fonts status: needs rebase labels Apr 15, 2025
@anntzer anntzer removed the status: waiting for other PR label Apr 15, 2025
@anntzer
anntzer force-pushed the dvi2ft branch 2 times, most recently from 99185ed to 48be1ca Compare April 17, 2025 10:06
QuLogic
QuLogic reviewed Apr 18, 2025
View reviewed changes
Comment thread lib/matplotlib/dviread.py Resolved

Uh oh!

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

QuLogic
QuLogic approved these changes Apr 18, 2025
View reviewed changes
@anntzer anntzer mentioned this pull request Apr 18, 2025
Parse {lua,xe}tex-generated dvi in dviread. #29939
Merged
5 tasks
@anntzer
Rework mapping of dvi glyph indices to freetype indices.
a3c14cf
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).
@anntzer
anntzer force-pushed the dvi2ft branch from 48be1ca to a3c14cf Compare April 29, 2025 13:20
@story645 story645 added this to the v3.11.0 milestone May 1, 2025
story645
story645 reviewed May 6, 2025
View reviewed changes
Comment thread lib/matplotlib/dviread.py
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.

@story645 story645 May 6, 2025

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.

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.

All reactions

@anntzer anntzer May 7, 2025

Copy link
Copy Markdown
Contributor Author

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.

All reactions
story645
story645 reviewed May 7, 2025
View reviewed changes
Comment thread lib/matplotlib/dviread.py
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 {} ({}); "

@story645 story645 May 7, 2025

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.

Can you please add a test for coverage?

Sorry, something went wrong.

Uh oh!

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

All reactions

@anntzer anntzer May 7, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

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 gets further refactored in the followup PR #29939 into DviFont.path, which does have test coverage (because it is also used elsewhere -- see https://app.codecov.io/gh/matplotlib/matplotlib/pull/29939#4c703021fcf83cd901550e2c525d78ae-R733). I can move code around so that the intermediate stage also has full coverage if you prefer, but this split seems more logical to me.

Sorry, something went wrong.

Uh oh!

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

All reactions
@story645
story645 merged commit 3da6ea9 into matplotlib:main May 8, 2025
@anntzer
anntzer deleted the dvi2ft branch May 8, 2025 16:41
@QuLogic QuLogic added this to Font and text overhaul Jun 5, 2025
@QuLogic QuLogic moved this to Done in Font and text overhaul Jun 5, 2025
@anntzer anntzer mentioned this pull request Sep 5, 2025
Use glyph indices for font tracking in vector formats #30335
Merged
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@story645 story645 story645 left review comments
@QuLogic QuLogic QuLogic approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

topic: text/usetex topic: text

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

@anntzer @QuLogic @story645
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 01402520 — 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.