matplotlib icon indicating copy to clipboard operation
matplotlib copied to clipboard

Implement text shaping with libraqm

Open QuLogic opened this issue 9 months ago • 6 comments

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

QuLogic avatar May 02 '25 10:05 QuLogic

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.

QuLogic avatar May 03 '25 09:05 QuLogic

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.

anntzer avatar May 17 '25 10:05 anntzer

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.

QuLogic avatar May 17 '25 10:05 QuLogic

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.

anntzer avatar May 21 '25 05:05 anntzer

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).

QuLogic avatar May 24 '25 09:05 QuLogic

[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?)

anntzer avatar May 24 '25 09:05 anntzer

Vector formats still need a bit of refactoring (in other open PRs), so I've decided to drop that from here so that this can be reviewed quicker.

QuLogic avatar Jul 24 '25 21:07 QuLogic

The failure on 3.12 on ubuntu is the c-coverage tool. I have a vague memory of fixing this before, but none of the details of how we fixed it?

tacaswell avatar Aug 21 '25 19:08 tacaswell

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

QuLogic avatar Aug 22 '25 00:08 QuLogic

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

tacaswell avatar Aug 22 '25 14:08 tacaswell

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.

QuLogic avatar Aug 23 '25 09:08 QuLogic

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 :+1:

anntzer avatar Aug 24 '25 10:08 anntzer

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.

QuLogic avatar Aug 26 '25 21:08 QuLogic