🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@anntzer
Copy link
Contributor

@anntzer anntzer commented May 16, 2025

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:

import matplotlib.pyplot as plt
import matplotlib.animation as manim

fig = plt.figure(figsize=(2, 2))
ts = [
    fig.text(0, 0, '$ABC123$'),
    fig.text(0, 0.2, 'ABC123'),
]
change = 0.001

def update(*args, **kwargs):
    global change
    for t in ts:
        x, y = t.get_position()
        if not (0 <= x <= 1 and 0 <= y <= 1):
            change *= -1
        t.set_x(x + change)
        t.set_y(y + change)
    return ts

ani = manim.FuncAnimation(fig, update, interval=20, frames=int(2/change),
                          cache_frame_data=False)
ani.save('text.gif')

PR summary

PR checklist

@story645
Copy link
Member

story645 commented May 16, 2025

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
Copy link
Contributor Author

anntzer commented May 16, 2025

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 not fully 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.

Edit: see the wiki; attn @QuLogic as well too.

@story645
Copy link
Member

story645 commented May 16, 2025

I'm sorry for the mess, but

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
Copy link
Contributor Author

anntzer commented May 16, 2025

link fixed

@anntzer anntzer force-pushed the ft-direct-render branch 3 times, most recently from d44360f to 66fdae0 Compare May 17, 2025 09:55
@anntzer
Copy link
Contributor Author

anntzer commented May 17, 2025

Edit: fraction bar rendering has been added too.

@QuLogic
Copy link
Member

QuLogic commented May 22, 2025

A few warnings from compiling locally:

[2/4] Compiling C++ object src/ft2font.cpython-313-x86_64-linux-gnu.so.p/ft2font_wrapper.cpp.o
../../src/ft2font_wrapper.cpp: In lambda function:
../../src/ft2font_wrapper.cpp:1785:22: warning: unused variable ‘error’ [-Wunused-variable]
 1785 |             if (auto error = FT_Load_Glyph(face, idx, static_cast<FT_Int32>(flags))) {
      |                      ^~~~~
../../src/ft2font_wrapper.cpp:1788:22: warning: unused variable ‘error’ [-Wunused-variable]
 1788 |             if (auto error = FT_Render_Glyph(face->glyph, FT_RENDER_MODE_NORMAL)) {
      |                      ^~~~~
../../src/ft2font_wrapper.cpp: In lambda function:
../../src/ft2font_wrapper.cpp:1804:26: warning: unused variable ‘error’ [-Wunused-variable]
 1804 |                 if (auto error = FT_Glyph_To_Bitmap(&g, FT_RENDER_MODE_NORMAL, &origin, 1)) {
      |                          ^~~~~

@anntzer
Copy link
Contributor Author

anntzer commented May 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).
I left the warning as is just to keep this as a side discussion point, but it can trivially be suppressed by not capturing the error value too.

@QuLogic
Copy link
Member

QuLogic commented May 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
Copy link
Member

QuLogic commented May 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
Copy link
Contributor Author

anntzer commented May 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)?

Sure, I don't really mind either way? Should the branch live on this repo and be ultimately pruned out, or on yours?

@anntzer anntzer force-pushed the ft-direct-render branch from e980d97 to 383e594 Compare May 30, 2025 08:37
@anntzer
Copy link
Contributor Author

anntzer commented May 30, 2025

Fixed #30059 (comment) by using FT_CHECK (thus also needs #30123).

@QuLogic QuLogic changed the base branch from main to text-overhaul June 5, 2025 01:47
@QuLogic QuLogic added this to the v3.11.0 milestone Jun 5, 2025
@QuLogic
Copy link
Member

QuLogic commented Oct 25, 2025

I think you may have dropped the rebase when doing that... I've rebased and fixed the conflict. However, I've not pushed the test images, because it appears there might be a bug with antialiasing disabled:
antialiased
(this is test_text/antialiased.png)

@anntzer anntzer force-pushed the ft-direct-render branch 2 times, most recently from 3df2b9b to cd6530d Compare October 27, 2025 15:54
Copy link
Member

@QuLogic QuLogic left a 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:

import itertools
import string
import sys

import matplotlib.pyplot as plt

fig = plt.figure()

for i, fontsize in enumerate([None, 20, 32]):
    for j, text in enumerate(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] if len(sys.argv) > 1 else 'bar.png')

in RendererAgg.draw_text, the x/y are almost always integers (or 3e-14 close to one) so fractional positioning shouldn't matter much. But when I compare results with the text-overhaul branch, almost every string has a difference in only 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)
for bitmap in font._render_glyphs(
x, self.height - y,
Copy link
Member

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 for boxarrow_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?

Copy link
Contributor Author

@anntzer anntzer Oct 29, 2025

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

QuLogic commented Oct 28, 2025

test modified from #29551 to also include mathtext:

If I extend this test to include \frac{abc123}{def456}, then the fraction bar is jumpy as well, because in the angle=0 case, we draw the bars with a NumPy array at a rounded position.

text

This could be fixed by always using the non-0-angle implementation which draws a Path instead, so long as we also call gc1.set_snap(False). I'm not sure whether we want to do that? It of course causes the bars to instead be smeared across two pixels periodically instead, so I don't know if we want to optimize for that over better static images. Or perhaps we should just let the user decide by setting snap on the Text object (which may or may not need additional threading through as I don't know if it cared about snapping before)?

@timhoffm
Copy link
Member

How do fonts handle dashes and underscores? Would that be a reasonable precedent behavior?

@anntzer
Copy link
Contributor Author

anntzer commented Oct 28, 2025

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
Copy link
Contributor Author

anntzer commented Oct 29, 2025

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:

import itertools
import string
import sys

import matplotlib.pyplot as plt

fig = plt.figure()

for i, fontsize in enumerate([None, 20, 32]):
    for j, text in enumerate(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] if len(sys.argv) > 1 else 'bar.png')

in RendererAgg.draw_text, the x/y are almost always integers (or 3e-14 close to one) so fractional positioning shouldn't matter much. But when I compare results with the text-overhaul branch, almost every string has a difference in only the first character vs the rest. Do you have any idea why it might only affect that one advance?

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.cpp
index 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, bitmap->left - (bbox.xMin * (1. / 64.)) is negative and thus the cast to FT_Int rounds up, whereas for the others, the cast rounds down, so the first glyph ends up shifted one pixel to the right relative to the others. (I didn't actually check for y but a similar issue seems likely.)

@QuLogic
Copy link
Member

QuLogic commented Oct 29, 2025

I figured that out. There's a bug in draw_glyphs_to_bitmap, which is fixed by

OK, so then it's an additional fix here; it did seem better in several cases (e.g., for This, the Th did seem to hit each other before).

@QuLogic
Copy link
Member

QuLogic commented Oct 29, 2025

How do fonts handle dashes and underscores? Would that be a reasonable precedent behavior?

This depends on the internal hinting and FreeType settings. Depending on the hinter, it can 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
Copy link
Contributor Author

anntzer commented Oct 29, 2025

I tested the positioning of the fraction bar a bit more:

import sys
import matplotlib.pyplot as plt

fig = plt.figure(figsize=(10, 2))
fig.suptitle(sys.argv[1])
for i in range(100):
    fig.text(0.01 * i, .1 + .001 * i, r'$\frac{A}{B}$', snap=False),
    fig.text(0.01 * i, .2 + .001 * i, r'$\frac{A}{B}$', snap=True),
plt.show()
image image image

Both text-overhaul and direct-render are clearly better than main. direct-render looks blurry when snapping is disabled, but that's intentional. However, it looks like direct-render also draws the fraction bar slightly too low, whereas text-overhaul draws it slightly too long to the right?

@QuLogic
Copy link
Member

QuLogic commented Oct 30, 2025

Comparing main with text-overhaul, the difference looks like the glyphs only; the bars stay in the same spot and are the same width. Comparing text-overhaul with direct-render, it looks like the bar is a bit narrower, but based on the not-snapped line, it's actually a partial pixel on the right side, which seems to have snapped to a whole one those lines. WRT the line vertical position, there seems to be some finagling with +/- 1 in the old code:

height = max(int(y2 - y1) - 1, 0)
if height == 0:
center = (y2 + y1) / 2
y = int(center - (height + 1) / 2)
else:
y = int(y1)
x1 = math.floor(x1)
x2 = math.ceil(x2)
image[y:y+height+1, x1:x2+1] = 0xff

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.

Directly render FT glyphs to the Agg buffer.  In particular, this
naturally provides, with no extra work, subpixel positioning of glyphs
(which could also have been implemented in the old framework, but would
have required careful tracking of subpixel offets).

Note that all baseline images should be regenerated.  The new APIs added
to FT2Font are also up to bikeshedding (but they are all private).
@QuLogic
Copy link
Member

QuLogic commented Nov 25, 2025

I forget, was there more to be done here, or just reviewing the images changes?

@anntzer
Copy link
Contributor Author

anntzer commented Nov 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
Copy link
Member

I am 👍🏻 on using snapping for this.

@tacaswell
Copy link
Member

#22852 is also related to this.

so

Throwing latex into this as well, I think it looks like our bar is a bit too low.

@story645 story645 moved this from In Progress to Ready for Review in Font and text overhaul Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

5 participants