Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Drop the FT2Font intermediate buffer.#30059
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
story645 commentedMay 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Would you be open to using a tracking issue or project board for the font work (sample categories: draft, waiting on other PRs, ready for review, please review first)? I think breaking everything up into small PRs is fantastic but I'm finding it a bit overwhelming to keep track of the order for reviewing things (and figure I'm not alone here) and how these PRs relate to each other. |
anntzer commentedMay 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sure, I'll do that. I'm sorry for the mess, but this is basically me trying to remember all the patches or even just ideas I have accumulated over the years that went nowhere because they would thrash all the baseline images, and that I now try to present given that I see an opportunity with the FreeType upgrade. Also, some of them are notfully complete patches but things that I believe can be made complete with relatively little effort (for some value of "little effort"...) simply because I do not have the time now to finalize them but want to put them up for discussion as the underlying idea should be clear. |
story645 commentedMay 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
No apologies needed, I commend you for taking advantage of the freetype upgrade. And thanks for the wiki, though some reason can't link cleanly |
anntzer commentedMay 16, 2025
link fixed |
d44360f to66fdae0Compareanntzer commentedMay 17, 2025
Edit: fraction bar rendering has been added too. |
QuLogic commentedMay 22, 2025
A few warnings from compiling locally: |
anntzer commentedMay 22, 2025
This is semi-intentional: if these checks fail (error > 0) then the error should really be raised using throw_ft_error, not with a plain raise as I currently do, but throw_ft_error is not visible from ft2font_wrapper.cpp so some code needs to be moved around, e.g. make throw_ft_error an inline function defined in ft2font.h, or move the new implementations into real C++ FT2Font methods in ft2font.cpp and add wrapper lambdas in ft2font_wrapper.cpp, or (would be my preferred choice) switch error checking to the FT_CHECK macro used in mplcairo (see mplcairo's macros.h). |
QuLogic commentedMay 24, 2025
On a side note, I'm in favour of this in general as removing the intermediate buffer will make emoji/colour glyphs easier since they won't need to go through it. |
QuLogic commentedMay 30, 2025
On the call, we were discussing whether it made sense to target all these font-related (and requiring full image rebuild) PRs to a separate feature branch, which would contain the final test image updates. Do you have any thoughts on retargetting this PR (instead of folding it in to#29816)? |
anntzer commentedMay 30, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sure, I don't really mind either way? Should the branch live on this repo and be ultimately pruned out, or on yours? |
anntzer commentedMay 30, 2025
Fixed#30059 (comment) by using FT_CHECK (thus also needs#30123). |
QuLogic left a comment
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.
So I've experimented a little bit to try and see where the changes arise; I've added a note below of one that I think I see.
Using the following small script:
importitertoolsimportstringimportsysimportmatplotlib.pyplotaspltfig=plt.figure()fori,fontsizeinenumerate([None,20,32]):forj,textinenumerate(itertools.batched(string.ascii_lowercase,3)):fig.text(0.1+0.2*i,0.1+0.1*j,''.join(text),fontsize=fontsize)fig.savefig(sys.argv[1]iflen(sys.argv)>1else'bar.png')
inRendererAgg.draw_text, thex/y are almost always integers (or 3e-14 close to one) so fractional positioning shouldn't matter much. But when I compare results with thetext-overhaul branch, almost every string has a difference inonly the first character vs the rest. Do you have any idea why it might only affect that one advance?
| self._renderer.draw_text_image(font,x,y+1,angle,gc) | ||
| forbitmapinfont._render_glyphs( | ||
| x,self.height-y, |
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.
If this were trimmed to integers (int(x), int(self.height - y)) then forboxarrow_test_image, all but the first letter in each string remain the same as the old images. I guess keeping the fractional part (i.e., as its written now) gives us the proper sub-pixel positioning?
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.
That's the intent, at least. Looks like something similar to#30059 (comment) was happening as well?
QuLogic commentedOct 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If I extend this test to include This could be fixed by always using the non-0- |
timhoffm commentedOct 28, 2025
How do fonts handle dashes and underscores? Would that be a reasonable precedent behavior? |
anntzer commentedOct 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Controlling based on Artist.get_snap() sounds like a good idea (as you mention, manually inserting gc.set_snap(False) works); I pushed that (and checked that it works). (However, from a quick look, maybe the exact way rounding works in snapping is different? The fraction bar looks shifted down?) Edit: Likely the proper way to rely on path snapping requires changing the fraction bar from being a filled rectangle (with zero-linewidth) to being a single line with fixed linewidth; still todo, and maybe a problem for the rare case of drawing vertical bars. Fonts likely control that based on hinting (https://en.wikipedia.org/wiki/Font_hinting), which could also be the switch, but I thought having separate controls for fraction bars and for the rest of hinting would be better. |
anntzer commentedOct 29, 2025
I figured that out. There's a bug in draw_glyphs_to_bitmap, which is fixed by diff --git i/src/ft2font.cpp w/src/ft2font.cppindex 8838f68ee5..96ccafe1b0 100644--- i/src/ft2font.cpp+++ w/src/ft2font.cpp@@ -711,8 +711,8 @@ void FT2Font::draw_glyphs_to_bitmap(bool antialiased) // now, draw to our target surface (convert position) // bitmap left and top in pixel, string bbox in subpixel- FT_Int x = (FT_Int)(bitmap->left - (bbox.xMin * (1. / 64.)));- FT_Int y = (FT_Int)((bbox.yMax * (1. / 64.)) - bitmap->top + 1);+ FT_Int x = (FT_Int)std::floor(bitmap->left - (bbox.xMin * (1. / 64.)));+ FT_Int y = (FT_Int)std::floor((bbox.yMax * (1. / 64.)) - bitmap->top + 1); draw_bitmap(image, &bitmap->bitmap, x, y); } The problem being that for the very first glyph only, |
QuLogic commentedOct 29, 2025
OK, so then it's an additional fix here; it did seem better in several cases (e.g., for |
QuLogic commentedOct 29, 2025
This depends on the internal hinting and FreeType settings. Depending on the hinter, itcan align vertical and horizontal lines to the pixel grid (which is why scaling up or down text isn't always correct), but it could just as easily not do that if the font doesn't implement it. |
anntzer commentedOct 29, 2025
QuLogic commentedOct 30, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Comparing matplotlib/lib/matplotlib/_mathtext.py Lines 167 to 175 incd3685f
I'm not sure how relevant that would be, but also note the floor/ceil on the x positions which is likely related to the width changing. |
QuLogic commentedNov 25, 2025
I forget, was there more to be done here, or just reviewing the images changes? |
anntzer commentedNov 25, 2025
We still need to agree that text.get_snap() is the correct API to control fraction bar blurring (that seems reasonable enough to me), and maybe try to figure out whether there's any off-by-1 issue with the fraction bar positioning. Some help on that would be welcome. |
tacaswell commentedNov 28, 2025
I am 👍🏻 on using snapping for this. |
tacaswell commentedNov 30, 2025
#22852 is also related to this. ![]() Throwing latex into this as well, I think it looks like our bar is a bit too low. |
Directly render FT glyphs to the Agg buffer. In particular, thisnaturally provides, with no extra work, subpixel positioning of glyphs(which could also have been implemented in the old framework, but wouldhave required careful tracking of subpixel offets).Note that all baseline images should be regenerated. The new APIs addedto FT2Font are also up to bikeshedding (but they are all private).
QuLogic commentedDec 18, 2025
I've rebased again and added the test images. This only includes the images that have changed here and not in the |
greglucas commentedDec 18, 2025
Looking at |
QuLogic commentedDec 18, 2025
Ah sorry, I just realized that GitHub gave me the "base" comparison link, so it actually shows the full diff between If I'm not mistaken, t looks like the epsilon was changed in an earlier PR. |
greglucas commentedDec 18, 2025
That looks like a more minimal change in the epsilon, so I agree with you that it was updated in a different PR. 👍 Still has my approval. |





Uh oh!
There was an error while loading.Please reload this page.
Directly render FT glyphs to the Agg buffer. In particular, this naturally provides, with no extra work, subpixel positioning of glyphs (closing#29551) (this could also have been implemented in the old framework, but would have required careful tracking of subpixel offets).
Note that rendering of mathtext boxes (e.g., fraction bars) is currently missing, but should be "easy" (the main question being whether/how to ensure proper hinting/alignment to pixel edges in the horizontal or vertical output cases).Likewise, all baseline images should be regenerated. Finally, the new APIs added to FT2Font are likely up to bikeshedding (but they are all private).In practice this should get included (if we agree to go this way) into the FreeType update (#29816). This would also supersede the patch advertised at#29816 (comment).
test modified from#29551 to also include mathtext:
PR summary
PR checklist