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 — Implement text shaping with libraqm by QuLogic · Pull Request #30000 · matplotlib/matplotlib

Conversation

@QuLogic

@QuLogic QuLogic commented May 2, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

PR summary

This is based on #29816 but doesn't yet include #29794 and #29695. I'm mostly opening it to see how CI will cope.

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
@QuLogic QuLogic added topic: text status: waiting for other PR topic: text/fonts status: needs documentation labels May 2, 2025
@QuLogic QuLogic added this to the v3.11.0 milestone May 2, 2025
@github-actions github-actions Bot added topic: mplot3d topic: text/mathtext topic: styles and removed topic: text labels May 2, 2025
@QuLogic
QuLogic force-pushed the libraqm branch 3 times, most recently from f8dea40 to d1780a2 Compare May 3, 2025 07:46
@QuLogic

QuLogic commented May 3, 2025

Copy link
Copy Markdown
Member Author

This was useful in pointing out some cross-platform incompatibilities. I may check cibuildwheel as well later to ensure bundling is working correctly.

The failing test is a relatively recent one; I will have to check if that is from here or the FreeType change.

All reactions

Sorry, something went wrong.

Uh oh!

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

@QuLogic
QuLogic force-pushed the libraqm branch from d1780a2 to dd82f64 Compare May 7, 2025 11:16
@github-actions github-actions Bot added backend: agg topic: path handling backend: pdf labels May 7, 2025
@QuLogic
QuLogic force-pushed the libraqm branch from dd82f64 to 45a7668 Compare May 8, 2025 19:04
@tacaswell tacaswell mentioned this pull request May 9, 2025
[ENH]: Allow specifying a typeface instead of a font #29866
Open
@github-actions github-actions Bot added the status: needs rebase label May 10, 2025
@anntzer

anntzer commented May 17, 2025

Copy link
Copy Markdown
Contributor

Do you envision that raqm will become the sole text layout method, or do you plan to keep the old manual layouting around (perhaps togglable)? I just noticed that the old layouting method appears to be missing handling of lsb_delta and rsb_delta (see note at https://freetype.org/freetype2/docs/reference/ft2-glyph_retrieval.html#ft_glyphslotrec ("If you use strong auto-hinting, you must apply these delta values! Otherwise you will experience far too large inter-glyph spacing at small rendering sizes in most cases.")) but raqm would likely (hopefully?) take care of that for us. If the manual layouting is going to stay then I'll open a separate issue to implement this (perhaps also to be folded into the FreeType update mega-PR); if not, perhaps we can just forget about it.

All reactions

Sorry, something went wrong.

Uh oh!

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

@QuLogic

QuLogic commented May 17, 2025

Copy link
Copy Markdown
Member Author

I intend for it to replace everything, I think. I have some in-progress work to apply it to PDF/PS/SVG as well.

I did notice the lsb_delta/rsb_delta note and tried to implement it in #29816, though IIRC not much (or even nothing) changed, but I could be misremembering.

anntzer reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@anntzer

anntzer commented May 21, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Also, does _text_helpers.layout() also need to be rewritten to use raqm as well? (likely it would need to also include y information now)
I had in fact more or less written that function as a "poor man's raqm", IIRC...

A nice endpoint would be if the new _text_helpers.LayoutItem, by supporting both x and y, was general enough that draw_text and draw_mathtext (and even draw_tex) could ultimately just all pass a list of LayoutItems (+ rects, in the mathtext/tex case) down to a single glyph rendering method.

All reactions

Sorry, something went wrong.

Uh oh!

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

@github-actions github-actions Bot added topic: text backend: ps labels May 24, 2025
@QuLogic

QuLogic commented May 24, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

OK, I've figured out why I was running into issues before and pushed the vector implementation up now. The issues aren't fixed, but at least in a position to be discussed.

Specifically, _text_helpers.layout used to use glyph.linearHoriAdvance which is a 16.16 fixed-point value, but libraqm returns advances in 26.6 fixed-point values. For example, the string "center Tj" has 16.16 advances:

[3603200, 4032000, 4153600, 2569600, 4032000, 2694400, 2083200, 4003200, 1820800]

which works out to:

>>> orig / 65536
array([54.98046875, 61.5234375 , 63.37890625, 39.20898438, 61.5234375 ,
       41.11328125, 31.78710938, 61.08398438, 27.78320312])

But the libraqm 26.6 advances are (equivalent to the linear advances divided by 1024 and rounded to integers):

[3519, 3938, 4056, 2509, 3938, 2631, 2034, 3909, 1778]

which works out to:

>>> raqm / 64
array([54.984375, 61.53125 , 63.375   , 39.203125, 61.53125 , 41.109375,
       31.78125 , 61.078125, 27.78125 ])

or the small difference of:

[0.00390625, 0.0078125, -0.00390625, -0.00585938, 0.0078125, -0.00390625, -0.00585938, -0.00585938, -0.00195312]

Unfortunately, this touches almost every single SVG in a way that they all fail, but invisibly.

I'm not sure if we should attempt to work around this (by scaling DPI by 1024, or adding a 1024 scaling FT_Matrix, or something else), or accept this change (but then maybe we'd want to roll that into #29816's big test image update).

All reactions

Sorry, something went wrong.

Uh oh!

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

@anntzer

anntzer commented May 24, 2025

Copy link
Copy Markdown
Contributor

[I guess what's really stored in the font file are advances in font design units and then FreeType scales them by (font design units->real space) and then by 2^16 whereas raqm (really, harfbuzz) uses 2^6 for that last scaling, so probably neither of them is really right or wrong?]

If the breakages are only in svg files then I guess this isn't really a problem because it's just a few numbers (the glyph x-positions) that will change and so the diff will not actually be so big? (If you check by git diff rather than the github UI or similar it would just be a few small changes?)

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 5, 2025 01:39
@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 commented Aug 22, 2025

Copy link
Copy Markdown
Member Author

Rebased on latest text-overhaul branch which merged the latest main, so hopefully we'll see if the test coverage is working again.

All reactions

Sorry, something went wrong.

Uh oh!

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

@QuLogic QuLogic added the CI: Run cibuildwheel Run wheel building tests on a PR label Aug 22, 2025
@tacaswell

tacaswell commented Aug 22, 2025

Copy link
Copy Markdown
Member

the appveyor failures look like we are doing some funny git-work there....

All reactions

Sorry, something went wrong.

Uh oh!

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

@QuLogic

QuLogic commented Aug 23, 2025

Copy link
Copy Markdown
Member Author

Rebased on latest text-overhaul branch which merged the latest main, so hopefully we'll see if the test coverage is working again.

Unfortunately, it's still a problem, but I've figured out that it's specifically about Harfbuzz and that I can reproduce it locally. I'm not sure what is buggy to cause the problem with only a few files, but as we don't care about subproject code coverage, I'll see if it can be disabled.

the appveyor failures look like we are doing some funny git-work there....

Yes, this was set up in #30231 / #30274 though I'm not sure why it's complaining on AppVeyor only.

All reactions

Sorry, something went wrong.

Uh oh!

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

anntzer
anntzer reviewed Aug 24, 2025
View reviewed changes
Comment thread src/ft2font.cpp Resolved

Uh oh!

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

Comment thread extern/meson.build Resolved

Uh oh!

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

Comment thread src/ft2font.cpp Outdated
throw std::runtime_error("failed to set text face for layout");
}
for (auto [cluster, face] : face_substitutions) {
raqm_set_freetype_face_range(rq, face, cluster, 1);

@anntzer anntzer Aug 24, 2025

Copy link
Copy Markdown
Contributor

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.

I think the last argument may need to be greater than 1, because the cluster may correspond to more than a single byte on the bytestring. I guess what needs to be done is that when you record face_substitutions.emplace_back(rglyph.cluster, fallback->face) you need actually apply the fallback face onto the entire span from rq_glyphs[i].cluster to rq_glyphs[i+1].cluster?

Adding test for font fallback may be enough to check that.

Sorry, something went wrong.

Uh oh!

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

All reactions

@QuLogic QuLogic Aug 26, 2025

Copy link
Copy Markdown
Member 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 turns out to be a good point. I have implemented your suggestion for now, but unfortunately, I'm not certain that libraqm exposes enough information to correctly implement things yet.

Specifically, if you have mixed LTR and RTL text (as in the existing complex layout test with text Arabic: رَقْم), then glyphs are returned in LTR display order. So you get glyphs for clusters 0, 1, 2, 3, 4, 5, 6, 7, 12, 10, 10, 8, 8. The run from 0-7 is in English and 8-12 in Arabic, but we get the latter in reverse order. In some cases, multiple glyphs are used for a single cluster and other times a single one for multiple indices.

Using rq_glyphs[i].cluster to rq_glyphs[i+1].cluster (with a fallback to the end of the string) happens to work because in that RTL section, we get the lowest index last and so set fallback on all indices above it even though we didn't "know" about the other in-between indices. So I think that somewhere in that run, if some character did exist, we might accidentally switch it to the fallback, but I haven't come up with a good way to test that.

Sorry, something went wrong.

Uh oh!

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

All reactions

@anntzer anntzer Aug 26, 2025

Copy link
Copy Markdown
Contributor

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.

Good point; perhaps that can be fixed by doing something along the lines of

cluster_starts = sorted(rq_glyph.cluster for rq_glyph in rq_glyphs)
span_end = cluster_starts[searchsorted(cluster_starts, span_start) + 1]

?

Sorry, something went wrong.

Uh oh!

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

All reactions

@QuLogic QuLogic Aug 26, 2025

Copy link
Copy Markdown
Member 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.

Yes, doing some kind of sorting does seem to work as expected (at least based on my debugging), though I haven't thought up a good test to be sure there aren't any mixups. I've extended the test to include multiple fallbacks as well, just to be sure that is working.

Sorry, something went wrong.

Uh oh!

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

All reactions
@anntzer

anntzer commented Aug 24, 2025

Copy link
Copy Markdown
Contributor

I have some doubts about the correctness of the font fallback implementation (per the review above), which can probably be addressed by a test; other than that, looks good to me 👍

All reactions

Sorry, something went wrong.

Uh oh!

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

QuLogic added 2 commits August 26, 2025 04:18
@QuLogic
Remove version from FreeType wrap file
584b1fd
Latest Meson (1.9.0 at this time) is now erroring because of the
Harfbuzz contains a differently-named wrap file that provides
`freetype2`. This may be a bug upstream, but we really don't need the
version in the filename.
@QuLogic
Add libraqm and its dependencies to build
0635d3a
Also add some missing license entries in more places.
@QuLogic
QuLogic force-pushed the libraqm branch from 5956859 to d8c9110 Compare August 26, 2025 09:22
@github-actions github-actions Bot removed the CI: Run cibuildwheel Run wheel building tests on a PR label Aug 26, 2025
@QuLogic
QuLogic force-pushed the libraqm branch from d8c9110 to 80517ff Compare August 26, 2025 21:11
@QuLogic

QuLogic commented Aug 26, 2025

Copy link
Copy Markdown
Member Author

I've added an additional commit at the beginning to rename freetype-2.13.3.wrap to freetype2.wrap because Meson 1.9.0 seems to find some conflict with Harfbuzz's freetype2.wrap This may be a bug upstream, but we don't really need the version in the name (it made some sense with FreeType 2.6.1 because that was a custom copy, but this freetype2.wrap is from Meson's wrapdb.)

Then also corrected the fallback code for multi-codepoint clusters as pointed out by @anntzer.

All reactions

Sorry, something went wrong.

Uh oh!

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

@QuLogic
QuLogic force-pushed the libraqm branch 2 times, most recently from 146bd6b to 25f50ac Compare August 26, 2025 22:00
@QuLogic QuLogic mentioned this pull request Aug 27, 2025
Add language parameter to Text objects #29794
Merged
1 of 4 tasks
anntzer
anntzer approved these changes Sep 2, 2025
View reviewed changes

@anntzer anntzer left a comment

Copy link
Copy Markdown
Contributor

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.

A few minor things left only. Feel free to self-merge with or without.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread src/ft2font.cpp Resolved

Uh oh!

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

Comment thread src/ft2font.cpp Resolved

Uh oh!

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

Comment thread src/ft2font.cpp
std::unique_ptr<std::remove_pointer_t<raqm_t>, decltype(&raqm_destroy)>(
rq, raqm_destroy);

if (!raqm_set_text(rq, reinterpret_cast<const uint32_t *>(text.data()),

@anntzer anntzer Sep 2, 2025

Copy link
Copy Markdown
Contributor

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.

It doesn't have to be done here (indeed, a separate PR may keep things more self-contained), but maybe PyFT2Font_set_text could change the type of text to std::string (utf8, per pybind11) and you can use raqm_set_text_utf8 here? Mostly for conciseness; but I also suspect that most inputs are not internally represented as 4byte-wide PyUNICODEs so conversion to utf32 is required by the current code anyways.

Sorry, something went wrong.

Uh oh!

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

All reactions
anntzer
anntzer reviewed Sep 2, 2025
View reviewed changes
Comment thread src/ft2font.cpp Resolved

Uh oh!

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

QuLogic added 4 commits September 2, 2025 19:01
@QuLogic
Implement text shaping with libraqm
b0ded3a
@QuLogic
Implement font fallback for libraqm
9813523
@QuLogic
DOC: Add What's New entry for complex text layout
b36b97f
@QuLogic
ci: Ignore coverage data from subprojects and generated files
04c8eef
We only care about our own source files, so anything in `subprojects` or
the `build` directory can be ignored, thus fixing the bugginess with
Harfbuzz headers.
@QuLogic
QuLogic force-pushed the libraqm branch from 25f50ac to 04c8eef Compare September 2, 2025 23:06
@github-actions github-actions Bot removed the topic: mplot3d label Sep 2, 2025
@QuLogic
QuLogic merged commit 04c8eef into matplotlib:text-overhaul Sep 3, 2025
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Font and text overhaul Sep 3, 2025
@story645 story645 mentioned this pull request Sep 14, 2025
[Bug]: Add native RTL (right-to-left) text rendering support for Hebrew and Arabic #30557
Closed
@QuLogic QuLogic mentioned this pull request Sep 26, 2025
Implement libraqm for vector outputs #30607
Merged
1 of 5 tasks
@QuLogic QuLogic mentioned this pull request Apr 10, 2026
Font and text overhaul #30161
Merged
4 tasks done
This was referenced Aug 30, 2026
[Doc]: 3.11 text shaping silently reverses the arabic_reshaper + python-bidi workaround #32262
Closed
Doc: tell 3.11 upgraders to remove the pre-shaping workaround #32263
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anntzer anntzer anntzer approved these changes
@tacaswell tacaswell tacaswell approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

topic: text/fonts

Projects

Font and text overhaul
Status: Done

Milestone

v3.11.0

Development

Successfully merging this pull request may close these issues.

[Feature Request] Proper Arabic Language Support in Matplotlib Plots [Bug]: Incorrect rendering of Abugida fonts on Matplotlib visualization [Bug]: Font rendering bug for Devanagari text Indic Script labels not rendered correctly

Uh oh!

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

3 participants

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