Skip Skrifa lookup for cached outlines#543
Skip Skrifa lookup for cached outlines#543valadaptive wants to merge 3 commits intolinebender:mainfrom
Conversation
There was a problem hiding this comment.
Nice find! I didn't realise the Skrifa glyph lookup was that slow.
A concern I have is that there is a behavioural change here too; I think we are selecting outline glyphs even if fonts have both outline and COLR, or outline and bitmap. The recommended behaviour seems to be selecting bitmap before outline.
This order of precedence matters in a few cases:
- Bitmaps being used to express glyphs when font size is very small (ref)
- Fonts which ship with outline as a backup for renderers that don't support COLR or Bitmap (you'd still want to render the bitmap if you can, and we can)
I don't think we can avoid doing the Skrifa lookup to see if a glyph has bitmap data because of this (I could be missing something though!). I do wonder if it's something that could be optimised within Skrifa though (like marking a font as being purely outline, or bitmap, or COLR once it's first loaded), but that would require considerably more work than what's in this PR.
Alternately, I think we could we treat the outline cache as an indicator that, given a font ID and glyph ID, we should be using the outline variant of that glyph (COLR/bitmap do not exist), as we would have only populated it in the prepare_outline_glyph path. So, we could skip the Skrifa lookup IFF we have a cache hit, and go straight to prepare_outline_glyph, and still get the performance win for what I imagine would be most iterations (outline glyph, cache hit).
Stacked on #542; the last commit is the new one.
Previously, drawing a glyph performed the following lookups, in order:
skrifa::BitmapStrikeslookupskrifa::ColorGlyphCollectionlookupskrifa::OutlineGlyphCollectionlookupThe Skrifa lookups are somewhat slow. I believe they perform a binary search on the raw font data internally.
We can optimize this by:
To do this, I refactored the
prepare_[kind]_glyphfunctions to return anOption; this allows them to perform the requisite Skrifa "does the glyph exist" lookups themselves. Theprepare_outline_glyphfunction can then check the outline cache itself, before doing a potentially-expensive Skrifa lookup. I also changed the outline cache to storeOption<OutlinePath>, so we can store absent glyphs in the cache too.Combined with the changes from #542, this is a ~10% speedup above current main, at least on my machine.