Restore glyph hinting support #544
Conversation
Updates skrifa to 0.19.0 and restores hinting support. Our previous glyph scaler came with its own instance cache but skrifa does not, so this also adds a very simple LRU cache to manage instances. There will likely be adjustments to this when we do "real" glyph caching and start retaining our caches across frames but this should be sufficient for now and will hopefully improve text quality in xilem on low-resolution displays.
| } | ||
|
|
||
| impl GlyphCache { | ||
| pub fn clear(&mut self) { | ||
| self.encoding.reset(); | ||
| self.glyphs.clear(); | ||
| // No need to clear the hinting cache |
There was a problem hiding this comment.
Why? Is it because it's already bounded in size?
There was a problem hiding this comment.
Exactly. In reality, none of this is retained right now so it’s mostly irrelevant. This module will need some substantial rework when we do rasterized glyph caching.
| @@ -55,9 +58,18 @@ impl GlyphCache { | |||
| encoding_cache.encode_fill_style(fill); | |||
| let mut path = encoding_cache.encode_path(true); | |||
| let outline = outlines.get(GlyphId::new(key.glyph_id as u16))?; | |||
| // FIXME: Re-add hinting when skrifa supports it | |||
| } | ||
|
|
||
| const HINTING_MODE: HintingMode = HintingMode::Smooth { | ||
| lcd_subpixel: Some(LcdLayout::Horizontal), |
There was a problem hiding this comment.
I'm guessing this will need to be wired through at some point?
There was a problem hiding this comment.
That’s an open question. The skrifa hinter supports a lot of modes because we have a requirement of matching FreeType. There’s an argument to be made that vello should always just use a modern hinting mode but that’s open to discussion. Ultimately, most fonts these days have hints generated by ttfautohint which ignores these things. Only the MS ClearType fonts really take advantage of them and we can potentially choose a sane default for those.
| @@ -92,6 +92,7 @@ impl SimpleText { | |||
| .glyph_transform(glyph_transform) | |||
| .normalized_coords(var_loc.coords()) | |||
| .brush(brush) | |||
| .hint(false) | |||
There was a problem hiding this comment.
I would expect us to want to test this?
Is it because SimpleText is also used for the headless example?
I see in Zulip that you've mentioned that text should be on a pixel baseline, which we don't enforce in our current examples.
I still would imagine that hinting could be productively used in e.g. the stats displays?
There was a problem hiding this comment.
This is disabled for the test scenes because we have some animated text which doesn’t work well with hinting (it snaps to pixels as it animates causing annoying jitter) and everything currently uses the same code path. We can definitely change this but I wanted to get the functionality in for xilem where hinting is more important.
Updates skrifa to 0.19.0 and restores hinting support. Our previous glyph scaler came with its own instance cache but skrifa does not, so this also adds a very simple LRU cache to manage instances.
There will likely be adjustments to this when we do "real" glyph caching and start retaining our caches across frames but this should be sufficient for now and will hopefully improve text quality in xilem on low-resolution displays.