Page MenuHome

BLF: Do Not Preload Glyph Cache
ClosedPublic

Authored by Harley Acheson (harley) on Aug 14 2021, 3:27 AM.

Details

Summary

This patch turns off the preloading of ascii glyphs and instead caches
each glyph the first time it is actually used.


Rendering font glyphs to bitmaps that we can draw with is not a trivial process and takes measurable time. Therefore we cache them. Once we have processed a character all subsequent uses are very fast. However, we currently preload this cache with about 95 glyphs in the ascii range. As in before we can print any characters for a new font size or style we render out these characters first.

I'm not able to see any advantage to doing this. There isn't a performance advantage in that rendering these 95 in a row takes the same total time as rendering each at any time. And there is a very real chance that you might not even use some of these. Or any at all. So on balance it would be best to cache only as needed, immediately after rending a glyph the first time.

This is what this patch does. First use of a character adds it to the cache. Therefore if your only use of a font size or style is to print out "漢" that is the only character rendered, with no need to wait for 95 others first.

This simplifies the code. It is no longer a requirement to call blf_font_ensure_ascii_table() before any use of BLF_UTF8_NEXT_FAST. And the macro BLF_UTF8_NEXT_FAST can be simplified a bit because we no longer have to pass glyph_ascii_table to it since we can get to that via the passed GlyphCacheBLF.

Diff Detail

Repository
rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Aug 14 2021, 3:27 AM
Harley Acheson (harley) created this revision.
Harley Acheson (harley) retitled this revision from BLF: Do Not Preload Glyphs to BLF: Do Not Preload Glyph Cache.Aug 14 2021, 3:49 AM

Looks OK, no need for another review iteration, noted an issue inline though.

source/blender/blenfont/intern/blf_font.c
460–463

This no longer continues if g is NULL.

That should be added back.

This revision is now accepted and ready to land.Aug 14 2021, 3:34 PM
Harley Acheson (harley) updated this revision to Diff 40699.EditedAug 14 2021, 9:05 PM

@Campbell Barton (campbellbarton) - This no longer continues if g is NULL. That should be added back.

I do agree and did put this back in, but it is an interesting thing to mention...

Current code would crap out if the glyph requested is not in the glyph_ascii_table, but with this change g will not be NULL in almost any normal case. FT_Get_Char_Index() returns zero on failure, with deliberate design so that FT_Load_Glyph will then get the glyph at position zero which is .notdef - the "tofu" character.

However, this is not obvious, hard to explain easily, or we could change our wrapper of FT_Load_Glyph to behave differently. And it is theoretically possible to create a bad font that does not have .notdef at position zero, so best to have this there, although the FreeType notes say they are handling this situation specifically since 2.11.0.

This revision was automatically updated to reflect the committed changes.

@Harley Acheson (harley) tested with a font generated from font-forge with one character defined, and this doesn't crash, so in practice these NULL checks probably aren't triggered.

This is more of a general correctness issue then, as all users of blf_glyph_add currently check for a NULL.
If this is not needed those checks should be removed everywhere with the cases it would have returned null replaced with BLI_assert_unreachable(..).

Although I think it's best to leave as-is, since as you mention it's theoretically possible FT_Load_Glyph fails.