Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Implement libraqm for vector outputs#30607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:text-overhaul
Are you sure you want to change the base?
Conversation
constauto load_flags =static_cast<FT_Int32>(flags); | ||
FT2Font::LanguageType languages; | ||
if (auto value = std::get_if<FT2Font::LanguageType>(&languages_or_str)) { |
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.
Actually not really related to this PR, but is there a reason not to use thestd::visit(variant, overloaded {...})
pattern? (see example 4 ofhttps://en.cppreference.com/w/cpp/utility/variant/visit2.html, or grep foroverloaded
in the mplcairo codebase)
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've not triedstd::visit
, but at least forstd::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. |
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.
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?)
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.
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 hadlibraqm
link to an external one. I'm not sure how easy this would be, unless we moved to some common provider.
I realized tests were not fully enabled here, and found some issues with |
I've expanded the new feature tests to include |
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 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. |
Uh oh!
There was an error while loading.Please reload this page.
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