diff --git a/src/Avalonia.Base/Media/FontManager.cs b/src/Avalonia.Base/Media/FontManager.cs index 481a69eaf7f..517f30308aa 100644 --- a/src/Avalonia.Base/Media/FontManager.cs +++ b/src/Avalonia.Base/Media/FontManager.cs @@ -34,12 +34,39 @@ public FontManager(IFontManagerImpl platformImpl) var options = AvaloniaLocator.Current.GetService(); _fontFallbacks = options?.FontFallbacks; - _fontFamilyMappings = options?.FontFamilyMappings; + _fontFamilyMappings = CreateFontFamilyMappings(options?.FontFamilyMappings); var defaultFontFamilyName = GetDefaultFontFamilyName(options); DefaultFontFamily = new FontFamily(defaultFontFamilyName); } + /// + /// Copies the configured mappings into a case-insensitive dictionary. Family names are matched + /// case-insensitively everywhere else, so a mapping must apply whatever casing the requested + /// name arrives in - and the dictionary handed to carries + /// whichever comparer its author happened to give it. + /// + private static IReadOnlyDictionary? CreateFontFamilyMappings( + IReadOnlyDictionary? fontFamilyMappings) + { + if (fontFamilyMappings is null || fontFamilyMappings.Count == 0) + { + return null; + } + + var mappings = new Dictionary(fontFamilyMappings.Count, StringComparer.OrdinalIgnoreCase); + + foreach (var mapping in fontFamilyMappings) + { + // Names that collide only by casing were distinct entries before the copy. Take the + // last rather than throwing: a mapping table is configuration, and failing here would + // bring the application down at startup. + mappings[mapping.Key] = mapping.Value; + } + + return mappings; + } + /// /// Get the current font manager instance. /// diff --git a/src/Avalonia.Base/Media/Fonts/FontCollectionBase.cs b/src/Avalonia.Base/Media/Fonts/FontCollectionBase.cs index e8ba3cb9a4a..b0e34f1f4a9 100644 --- a/src/Avalonia.Base/Media/Fonts/FontCollectionBase.cs +++ b/src/Avalonia.Base/Media/Fonts/FontCollectionBase.cs @@ -18,7 +18,8 @@ public abstract class FontCollectionBase : IFontCollection Comparer.Create((a, b) => string.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); // Make this internal for testing purposes - internal readonly ConcurrentDictionary> _glyphTypefaceCache = new(); + internal readonly ConcurrentDictionary> _glyphTypefaceCache = + new(StringComparer.OrdinalIgnoreCase); // Cache of resolved script/culture fallback family names. A non-null value is the preferred // fallback family for that script bucket: a Tier B hint that is still re-checked for coverage @@ -501,7 +502,26 @@ public virtual bool TryCreateSyntheticGlyphTypeface( fontSimulations |= FontSimulations.Bold; } - if (fontSimulations != FontSimulations.None && glyphTypeface.PlatformTypeface.TryGetStream(out var stream)) + if (fontSimulations == FontSimulations.None) + { + return false; + } + + // A synthetic for this key may already be cached under the source family, reached + // through another of its names or by another thread. Building a second one copies the + // whole font file through TryGetStream, then loses the slot below to the instance + // already there, so nothing caches it, nothing disposes it, and its native typeface is + // never released. + if (glyphTypefaces.TryGetValue(key, out var cachedGlyphTypeface) && + cachedGlyphTypeface is not null && + cachedGlyphTypeface.FontSimulations == fontSimulations) + { + syntheticGlyphTypeface = cachedGlyphTypeface; + + return true; + } + + if (glyphTypeface.PlatformTypeface.TryGetStream(out var stream)) { using (stream) { @@ -853,13 +873,10 @@ protected bool TryGetGlyphTypeface( glyphTypeface = syntheticGlyphTypeface; } - // Cache the resolved typeface under the REQUESTED family name, whether it is - // the nearest match or a synthetic one. TryCreateSyntheticGlyphTypeface only - // registers the synthetic under the *source font's own* family names, so a - // request coming through a different name (an alias, or a "Family Style" - // composite normalized by Typeface.Normalize) never hits the cache and - // re-enters synthesis on every single call — and synthesis copies the whole - // font file through TryGetStream. + // TryCreateSyntheticGlyphTypeface registers the synthetic only under the + // source font's own family names, so a request arriving through a different + // name would otherwise miss the cache and re-synthesise on every call, + // copying the whole font file each time. TryAddGlyphTypeface(familyName, key, glyphTypeface); } diff --git a/src/Avalonia.Base/Media/Fonts/SystemFontCollection.cs b/src/Avalonia.Base/Media/Fonts/SystemFontCollection.cs index 390f4f47193..e076c143574 100644 --- a/src/Avalonia.Base/Media/Fonts/SystemFontCollection.cs +++ b/src/Avalonia.Base/Media/Fonts/SystemFontCollection.cs @@ -69,7 +69,7 @@ public override bool TryGetGlyphTypeface(string familyName, FontStyle style, Fon TryAddGlyphTypeface(platformTypeface.FamilyName, key, glyphTypeface); // Then the requested family name - if (familyName != platformTypeface.FamilyName) + if (!string.Equals(familyName, platformTypeface.FamilyName, StringComparison.OrdinalIgnoreCase)) TryAddGlyphTypeface(familyName, key, glyphTypeface); //Add to cache diff --git a/tests/Avalonia.Skia.UnitTests/Media/FontCollectionTests.cs b/tests/Avalonia.Skia.UnitTests/Media/FontCollectionTests.cs index c5ba33cb118..479bc7c5e42 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/FontCollectionTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/FontCollectionTests.cs @@ -193,9 +193,95 @@ public void Should_Cache_Synthetic_Match_Under_Requested_Family_Name() } } + [Fact] + public void Should_Ignore_Family_Name_Casing_When_Resolving_A_Synthetic_Match() + { + var fontManager = new AliasFontManagerImpl(alias: "MyAlias"); + + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface.With(fontManagerImpl: fontManager))) + { + var fontCollection = new TestSystemFontCollection(fontManager); + + Assert.True(fontCollection.TryGetGlyphTypeface( + "MyAlias", FontStyle.Normal, FontWeight.Normal, FontStretch.Normal, out _)); + + // Casing must not decide whether a request gets a synthesised bold. A cache keyed + // ordinally sends this lookup past the synthesis branch and down the family-name + // search, which returns the nearest match raw - so the very same family renders + // faux-bold under one casing and regular weight under another. + Assert.True(fontCollection.TryGetGlyphTypeface( + "MYALIAS", FontStyle.Normal, FontWeight.Black, FontStretch.Normal, out var upperCase)); + + Assert.Equal(FontSimulations.Bold, upperCase.FontSimulations); + + var creationsAfterFirstCall = fontManager.StreamTypefaceCreations; + + Assert.True(fontCollection.TryGetGlyphTypeface( + "MyAlias", FontStyle.Normal, FontWeight.Black, FontStretch.Normal, out var mixedCase)); + + // One shared cache entry, so the other casing neither re-synthesises nor gets a + // second instance of the same face. + Assert.Same(upperCase, mixedCase); + Assert.Equal(creationsAfterFirstCall, fontManager.StreamTypefaceCreations); + } + } + + [Fact] + public void Should_Not_Cache_A_Family_Twice_When_The_Platform_Returns_Another_Casing() + { + // The platform reports the family as "Noto Mono"; the caller asks in lower case, as any + // XAML author may. + var fontManager = new AliasFontManagerImpl(alias: "Noto Mono"); + + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface.With(fontManagerImpl: fontManager))) + { + var fontCollection = new TestSystemFontCollection(fontManager); + + Assert.True(fontCollection.TryGetGlyphTypeface( + "noto mono", FontStyle.Normal, FontWeight.Normal, FontStretch.Normal, out _)); + + // A cache keyed ordinally stores the requested casing beside the platform's own, but + // AddFontFamily de-duplicates case-insensitively and publishes only the first of the + // two, leaving the second bucket unreachable from every family-name search. + Assert.Single(fontCollection.GlyphTypefaceCache); + Assert.Equal(fontCollection.GlyphTypefaceCache.Count, fontCollection.Count); + } + } + + [Fact] + public void Should_Reuse_An_Already_Cached_Synthetic_Glyph_Typeface() + { + var fontManager = new AliasFontManagerImpl(alias: "MyAlias"); + + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface.With(fontManagerImpl: fontManager))) + { + var fontCollection = new TestSystemFontCollection(fontManager); + + Assert.True(fontCollection.TryGetGlyphTypeface( + "MyAlias", FontStyle.Normal, FontWeight.Normal, FontStretch.Normal, out var regular)); + + Assert.True(fontCollection.TryCreateSyntheticGlyphTypeface( + regular, FontStyle.Normal, FontWeight.Black, FontStretch.Normal, out var first)); + + Assert.Equal(FontSimulations.Bold, first.FontSimulations); + + var creationsAfterFirstCall = fontManager.StreamTypefaceCreations; + + Assert.True(fontCollection.TryCreateSyntheticGlyphTypeface( + regular, FontStyle.Normal, FontWeight.Black, FontStretch.Normal, out var second)); + + // A second synthesis builds a GlyphTypeface that then loses the cache slot to the + // first one, so it is returned to the caller but never cached and never disposed - + // and GlyphTypeface has no finalizer, so its native typeface is retained until the + // process exits. + Assert.Same(first, second); + Assert.Equal(creationsAfterFirstCall, fontManager.StreamTypefaceCreations); + } + } + /// /// Font manager whose MyAlias family resolves through the platform but is absent from - /// the installed family list — the shape of a platform alias (for instance Android's + /// the installed family list, the shape of a platform alias (for instance Android's /// <alias name="arial" to="sans-serif"/> in /system/etc/fonts.xml). /// Such a family cannot be found again by the family-name search, so nothing repairs a /// missing cache entry. @@ -219,7 +305,9 @@ public AliasFontManagerImpl(string alias) _alias = alias; } - /// Number of typefaces created from a stream, i.e. of synthetic emboldenings. + /// Number of typefaces created from a stream: both the alias resolution and every + /// synthetic emboldening go through this overload, so the counter also proves that a cached + /// result short-circuits the platform call. public int StreamTypefaceCreations { get; private set; } public string GetDefaultFontFamilyName() => _inner.GetDefaultFontFamilyName(); @@ -231,7 +319,7 @@ public bool TryCreateGlyphTypeface(string familyName, FontStyle style, FontWeigh FontStretch stretch, [NotNullWhen(true)] out IPlatformTypeface? platformTypeface) { // The alias always resolves to the regular face of the backing font, never to the - // requested weight — exactly what a platform alias does. + // requested weight, exactly what a platform alias does. if (string.Equals(familyName, _alias, StringComparison.OrdinalIgnoreCase)) { using var stream = OpenBackingFont(); diff --git a/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs b/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs index 4f171b29ad3..f579f2134ea 100644 --- a/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs +++ b/tests/Avalonia.Skia.UnitTests/Media/FontManagerTests.cs @@ -396,6 +396,40 @@ public void Should_Map_FontFamily() } } + [Fact] + public void Should_Map_FontFamily_Regardless_Of_Casing() + { + using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface.With(fontManagerImpl: new FontManagerImpl()))) + { + using (AvaloniaLocator.EnterScope()) + { + AvaloniaLocator.CurrentMutable.BindToSelf(new FontManagerOptions + { + DefaultFamilyName = s_fontUri, + FontFamilyMappings = new Dictionary + { + { "Segoe UI", new FontFamily("fonts:Inter#Inter") } + } + }); + + FontManager.Current.AddFontCollection(new InterFontCollection()); + + // The mapping table is configuration; its casing has nothing to do with the casing + // a control asks in. Both the composite and the plain family path must still find + // the mapping. + Assert.True(FontManager.Current.TryGetGlyphTypeface( + new Typeface("Abc, segoe ui"), out var fromComposite)); + + Assert.Equal("Inter", fromComposite.FamilyName); + + Assert.True(FontManager.Current.TryGetGlyphTypeface( + new Typeface("SEGOE UI"), out var fromPlainFamily)); + + Assert.Equal("Inter", fromPlainFamily.FamilyName); + } + } + } + [Fact] public void Should_Get_FamilyTypefaces() {