perf: cache calculations for 'ch' and 'ex' units #1587
Conversation
|
Thanks (again!) for this pull request.
Yes, I suppose that the cache should be stored in the Moreover, the 1ch / 1em and 1ex / 1em ratios could probably be calculated once for each font face, and maybe without even using Pango (that’s needlessly expensive for what we want to achieve). But it’s not easy to do. What do you think? |
|
Attaching the configuration to a It wasn't immediately clear to me what could potentially affect the ratios for each type: certainly the features could (tabular figures could affect the 1ch size), style will, stretch will, weight will. I know that OpenType hinting can change with font size, but I can't think of anything that would actually modify the glyphs themselves. (But, truthfully, since I couldn't rule it out, I included it in the cache.) If we're fairly sure that the ratio doesn't change for a given font configuration across different sizes, it may be easy enough to simply cache that instead, as this is already doing it for 1ex/1em. Doing the calculation without Pango seems like it could potentially be nice from a philosophical point of view, but with the caching it would be unlikely to be a huge time savings, so I'd probably wait until more things are shifted over to HarfBuzz rather than spend a bunch more effort on it here, since we already have nearly-working code, if that seems reasonable. (I could simply modify the 1ex/1em calculation to also handle 1ch/1em.) |
This commit adds many small improvements in the way fonts are managed. 1. Most of the font deduplication logic has been put in add_font(), instead of draw_first_line(). 2. Font objects now have only one (reproducible) hash, based on the font content and the face index. Before this commit, we had 3 (!) different hashes. Fixes #1553. 3. Font fields are calculated during initialization, to avoid useless parameters storage. 4. Harfbuzz face is not read twice from Pango font when a new Font is added. 5. Font face deduplication is now done using the Pango face pointer, instead of the Harfbuzz face hash extracted from the Pango font. This could save some time, especially for very long documents always using the same font face. 6. Font name displayed in PDF now includes weight and style.
|
Digging a bit further, I'm not sure I can actually attach this to a FontConfiguration object the way styles are handled right now. I think the current style doesn't actually have a Do you see a good way to get one in? If not, I can push the refactor to just cache a ratio for the ex and ch sizes for now, and get to the other refactor later Presumably eventually you'll want to plumb through the context or at least a FontConfiguration object to the style, but it's not clear that this PR is necessary the right place to do that unless you see an easy way. |
Based on review comments. Also modifies inline_box_verticality to use the cache rather than recalculate every time.
|
This now has the ratio calculation code that lets our cache be independent of font size. @liZe: does this seem like something you'd be willing to merge as-is, or do we need to plumb FontConfiguration / context in to style handling first? |
It’s OK, thanks for your second commit. We don’t need |
It avoids the impression that the cache is global. And it actually was, because "copy" on the dictionary doesn’t copy children dictionaries. Related to #1587.
Also fixes the ex value used for vertical-align: middle. Related to #1587.
I get a ~30% speedup from caching this calculation on a relatively large (~300 page) test document. We use a fair number of ch-based lengths, but not an absurd amount. Regardless, this seems to provide a significant speedup as measured by hyperfine:
It wasn't immediately clear to me where a cache for these would be best, but this has it anchored at any style root rather than globally, as arguably the fonts might change between documents. (That said, actually handling
@font-facefonts seems to be a TODO here at the moment anyway.)