Skip to content

Conversation

@QuLogic
Copy link
Member

@QuLogic QuLogic commented Sep 26, 2025

PR summary

This PR is a followup to #30000 which did raster backends, now for vector backends. I'm not 100% on the kerning calculation, but the resulting differences are quite small; I think this may because of using a 16.16 value vs a 26.6 creating minor rounding differences.

This is based on #29695 to reduce conflicts.

PR checklist

const auto load_flags = static_cast<FT_Int32>(flags);

FT2Font::LanguageType languages;
if (auto value = std::get_if<FT2Font::LanguageType>(&languages_or_str)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually not really related to this PR, but is there a reason not to use the std::visit(variant, overloaded {...}) pattern? (see example 4 of https://en.cppreference.com/w/cpp/utility/variant/visit2.html, or grep for overloaded in the mplcairo codebase)

Copy link
Member Author

@QuLogic QuLogic Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not tried std::visit, but at least for std::get_if, one reason why we stuck with it instead of something else is that it wouldn't compile on macOS with the current deployment target we have. It's possible that it's fine now, but I don't believe we've bumped the target in a while.

.. warning::
This API uses the fallback list and is both private and provisional: do not use
it directly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the long term it would be nice if mplcairo could directly call this (instead of having to reimplement the whole thing itself as well); but I suspect the issue is the fact that this has to expose FT2Font objects and mplcairo would thus need to also use the same ft2Library instance as matplotlib, which... well I don't really know if that's something matplotlib wants to expose publically. (I'm mostly just thinking aloud here.)
(mplcairo could also just grab the filename of the FT2Font objects and reopen them, which is not too bad with caching, but I'm not sure this would work with variable fonts at least?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There would need to be some way to share the FreeType library itself, I think. One issue I had initially was between sharing the internal FreeType structures with another when I accidentally had libraqm link to an external one. I'm not sure how easy this would be, unless we moved to some common provider.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 27, 2025

I realized tests were not fully enabled here, and found some issues with CharacterTracker, so I've opened #30608 to fix those before this one.

@QuLogic
Copy link
Member Author

QuLogic commented Sep 30, 2025

Pushed two corrections: 1) SVG had lost the font features/languages setting and 2) fixed vertical offsets in PDF. Now we can do ridiculous accent stacks like this:
image

@QuLogic
Copy link
Member Author

QuLogic commented Sep 30, 2025

I've expanded the new feature tests to include .eps extension, since that's not there by default, and fixed issues in that format.

@QuLogic
Copy link
Member Author

QuLogic commented Oct 3, 2025

I've rebased this and added the image changes for review now that there are no other PR dependencies. Most of the changes are pretty much invisible, as the glyph locations shift by <1/64 of a pixel due to #30000 (comment) Sometimes this causes the rasterization for test comparison to shift a glyph by a whole pixel, since the DPI is rather low, but it's not too noticeable at a normal size. I don't know if we want to round to nearest 1/64th, or just run with this as is.

The main ones you should look at are test_text/complex.*, test_text/features.*, and test_text/language.*, which should all be consistent now with the PNG.

There are fortunately not too many image changes, but please don't merge with them included, as I'll move them to the other branch before doing so like the other PRs.

@tacaswell
Copy link
Member

Holding off merging in case @QuLogic wants to do image management.

@QuLogic QuLogic merged commit 950fa0f into matplotlib:text-overhaul Nov 20, 2025
36 checks passed
@github-project-automation github-project-automation bot moved this from Ready for Review to Done in Font and text overhaul Nov 20, 2025
@QuLogic QuLogic deleted the libraqm-vector branch November 20, 2025 20:07
@anntzer
Copy link
Contributor

anntzer commented Nov 21, 2025

Sorry for not reviewing further, and congrats on getting this in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants