Cache synthetic/nearest glyph typefaces under the requested family name - #21993
Conversation
FontCollectionBase.TryGetGlyphTypeface only cached the resolved typeface under the
requested family name when synthesis FAILED. When TryCreateSyntheticGlyphTypeface
succeeds it registers the synthetic only under the source font's own family names,
never under the name the caller asked for.
A request arriving through a different name therefore never hits the cache and
re-enters synthesis on every call. Synthesis goes through IPlatformTypeface.TryGetStream,
which reads the entire font file into memory and hands it to SKTypeface.FromStream,
which copies it again natively - so every resolution costs one retained copy of the
whole font file.
Two common shapes reach this path:
- a platform alias, i.e. a family the font manager resolves but that is absent from
GetInstalledFontFamilyNames(). Android declares several in /system/etc/fonts.xml
(arial, helvetica, tahoma, verdana, times, courier);
- a "Family Style" composite decomposed by Typeface.Normalize (e.g. "Arial Black"
-> family arial + FontWeight.Black), which makes the requested key differ from the
key the platform returns.
Windows/DirectWrite is structurally immune: every resolvable family is also an installed
family there, so the case-insensitive family-name search finds the synthetic (registered
under its real name) on the second call and the missing entry repairs itself after one
copy. With a platform alias nothing repairs it.
Measured on Android 13 / arm64: a text widget set to "Arial Black" grew the native heap
by +1187 MB over ~14 redraws (chunks of Roboto-Regular.ttf, identified by byte
comparison), until the platform memory guard killed the process at 2.6 GB.
Adds a regression test with an IFontManagerImpl that models a platform alias: without
the fix the second resolution returns a different instance (re-synthesis), with it the
cached one.
|
You can test this PR using the following package version. |
|
|
@cla-avalonia agree |
| } | ||
|
|
||
| [Win32Fact("Relies on an installed font family to back the alias")] | ||
| public void Should_Cache_Synthetic_Match_Under_Requested_Family_Name() |
There was a problem hiding this comment.
This test should be written platform-independently. There is no need to use Arial.
There was a problem hiding this comment.
Good point, thank you — you are right, and the dependency on Arial was a leftover from how I found the bug in the field rather than anything the test needed.
Rewritten in a3ef02b: the alias is now backed by an embedded test font (resm:Avalonia.Skia.UnitTests.Assets, same pattern as CustomFontManagerImpl) and the test is a plain [Fact], so it runs on every platform. That also matches the defect better, since FontCollectionBase is platform-agnostic — behind [Win32Fact] the Linux and macOS legs were not covering it at all.
The fake font manager is a little more faithful too: it now always resolves the alias to the regular face of the embedded font whatever weight is requested — which is what a platform alias does — instead of delegating to the real font manager.
Verified both ways locally: *FontCollection* is 39/39 with the fix, and the new test is the single failure without it.
Back the alias with an embedded test font instead of an installed Arial, and turn the test into a plain [Fact] so it runs on every platform. The defect being covered lives in FontCollectionBase and is platform-agnostic, so gating the test behind [Win32Fact] left it unexercised on the Linux and macOS legs of CI. Relying on an installed system font also made the test dependent on the build environment. The fake font manager now always resolves the alias to the regular face of an embedded font, whatever weight is asked for - which is what a platform alias actually does - so no system font is involved at all.
|
You can test this PR using the following package version. |
|
Second real-world reproduction of this defect, measured on a production Android 11 kiosk (Avalonia 12.1.1). A Measured with heapprofd on the device: 6 copies of Removing that single Worth noting for anyone hitting this: it only shows up where the platform font set is poor. The same application, same screen, on an Android 13 device (variable Roboto, real 600 available) and on Windows/DirectWrite allocates nothing — no nearest-match, no simulation, no copy. That is probably why this has stayed unnoticed. Caching under |
| } | ||
|
|
||
| private static Stream OpenBackingFont() | ||
| { |
There was a problem hiding this comment.
This picks whichever asset GetAssets happens to enumerate first (currently AdobeBlank2VF.ttf).
Please name the font explicitly resm:Avalonia.Skia.UnitTests.Assets.NotoMono-Regular.ttf?assembly=Avalonia.Skia.UnitTests, matching the style of GlyphTypefaceTests.InterFontUri and add Assert.Equal(FontSimulations.Bold, first.FontSimulations) after the first Black resolution. Without that, a backing font that fails synthesis makes the whole test pass against unfixed code, since the old else branch cached the nearest match.
There was a problem hiding this comment.
Good catch, and the second half is the important one — thank you.
You are right that the test could pass against unfixed code. I checked what GetAssets actually enumerated first: AdobeBlank2VF.ttf, a blank variable font — about the worst possible backing face for a test about bold synthesis. Both points are addressed in c8b03f0:
- the font is now named explicitly,
resm:Avalonia.Skia.UnitTests.Assets.NotoMono-Regular.ttf?assembly=Avalonia.Skia.UnitTests; Assert.Equal(FontSimulations.Bold, first.FontSimulations)is asserted right after the first Black resolution, with a comment explaining precisely why it is there — a font that cannot be emboldened would send the lookup down the old else branch, cache the nearest match, and make everything below vacuous.
I also dropped the now-unused using System.Linq.
Verified both ways locally: FontCollectionTests is 4/4 with the fix, and Should_Cache_Synthetic_Match_Under_Requested_Family_Name is the single failure without it (reverting only FontCollectionBase.cs).
Name the backing font explicitly instead of taking whichever asset GetAssets enumerates first (currently AdobeBlank2VF.ttf, a blank variable font), matching the style used elsewhere for embedded test fonts. Assert that the first resolution really is a synthesised bold. Without it, a backing font that cannot be emboldened makes TryCreateSyntheticGlyphTypeface fail, the old else branch caches the nearest match, and the whole test passes against unfixed code. Verified both ways: FontCollectionTests is 4/4 with the fix, and Should_Cache_Synthetic_Match_Under_Requested_Family_Name is the single failure without it.
|
You can test this PR using the following package version. |
|
Gentle bump — this has been sitting since 16 August and has had no reviewer yet. Recap of what it is, since the thread is long: on a font collection whose platform resolves a Two independent real-world reproductions are in this thread, both on production kiosks, one of them Happy to rebase or split if that helps it move. |
|
You can test this PR using the following package version. |
Gillibald
left a comment
There was a problem hiding this comment.
LGTM
There are still some issues related to synthetic font lookup but we can fix them later
What does the pull request do?
FontCollectionBase.TryGetGlyphTypefaceonly caches the resolved typeface under the requested family name when synthesis fails. WhenTryCreateSyntheticGlyphTypefacesucceeds, the synthetic typeface is registered byTryCreateSyntheticGlyphTypefaceitself — but only under the source font's own family names, never under the name the caller asked for.As a result, a request that arrives through a different name never hits the cache and re-enters synthesis on every single call. Synthesis goes through
IPlatformTypeface.TryGetStream, which reads the entire font file into memory and hands it toSKTypeface.FromStream, which copies it again natively. Every resolution therefore costs one retained copy of the whole font file.Two common shapes reach this path:
GetInstalledFontFamilyNames(). Android declares several in/system/etc/fonts.xml:<alias name="arial" to="sans-serif"/>, plushelvetica,tahoma,verdana,times,courier;Typeface.Normalize(e.g."Arial Black"→ familyarial+FontWeight.Black), which makes the requested key differ from the key the platform returns.When the bare family has already been resolved at another weight, the lookup finds it as a nearest match, synthesizes, and returns — with the requested key still absent from the cache, so the next call repeats everything.
The fix caches under the requested family name in both branches (the
TryAddGlyphTypefacecall moves out of theelse).Why is it needed / measured impact
Found while investigating unbounded native memory growth on Android digital-signage devices (Avalonia 12.1.1, .NET 10, arm64).
/system/fonts/Roboto-Regular.ttf(2 372 548 bytes = 29 × 81 812), copied inSkDynamicMemoryWStreamblocks — the whole font file, once per resolution call, roughly 90 times per screen redraw."Arial Black"grew the native heap by +1187 MB over ~14 redraws; the same widget set toRoboto,Archivo Blackor bareArialstayed flat; switching back to"Arial Black"restarted the growth (+479 MB in 2 minutes) until the platform memory guard killed the process at 2.6 GB.helvetica(another Android alias) resolved once at normal weight first,"helvetica black"on the same process grew the heap by +570 MB in 2 min 30 — a font that had been flat in every earlier test.TryGetGlyphTypefacefinds the synthetic (registered under its real name) on the second call, and the missing cache entry repairs itself after a single copy. With a platform alias, nothing repairs it.Tested platforms
Avalonia.Skia.UnitTests, filter*FontCollection*: 39/39 pass with the fix; 1 failure (the new test) without it — the second resolution returns a different instance, i.e. re-synthesis.EmbeddedFontCollectionTests.Should_Cache_Synthetic_GlyphTypeface,FontCollectionTests.Should_Cache_Nearest_Match) are unaffected.The added test uses an
IFontManagerImplthat models a platform alias — a family that resolves throughTryCreateGlyphTypefacebut is absent fromGetInstalledFontFamilyNames()— and asserts that repeated resolutions return the same instance and perform no further stream-based typeface creation.Breaking changes
None. The change only adds a cache entry that the non-synthetic branch already added.