Merge pull request #478 from SixLabors/js/fix-475

JimBobSquarePants · web-flow · commit aefb9674fa75 · 2025-11-26T14:27:16.000+10:00
Choose correct shaper and avoid infinite loop
diff --git a/src/SixLabors.Fonts/GlyphPositioningCollection.cs b/src/SixLabors.Fonts/GlyphPositioningCollection.cs
@@ -199,7 +199,7 @@ public bool TryUpdate(Font font, GlyphSubstitutionCollection collection)
 
                     if (metrics.GlyphType != GlyphType.Fallback)
                     {
-                        if (j == 0)
+                        if (replacementCount == 0)
                         {
                             // There should only be a single fallback glyph at this position from the previous collection.
                             this.glyphs.RemoveAt(i);
diff --git a/src/SixLabors.Fonts/Tables/AdvancedTypographic/GPosTable.cs b/src/SixLabors.Fonts/Tables/AdvancedTypographic/GPosTable.cs
@@ -89,11 +89,11 @@ internal static GPosTable Load(BigEndianBinaryReader reader)
         uint featureVariationsOffset = (minorVersion == 1) ? reader.ReadOffset32() : 0;
 
         // TODO: Optimization. Allow only reading the scriptList.
-        var scriptList = ScriptList.Load(reader, scriptListOffset);
+        ScriptList? scriptList = ScriptList.Load(reader, scriptListOffset);
 
-        var featureList = FeatureListTable.Load(reader, featureListOffset);
+        FeatureListTable featureList = FeatureListTable.Load(reader, featureListOffset);
 
-        var lookupList = LookupListTable.Load(reader, lookupListOffset);
+        LookupListTable lookupList = LookupListTable.Load(reader, lookupListOffset);
 
         // TODO: Feature Variations.
         return new GPosTable(scriptList, featureList, lookupList);
@@ -116,7 +116,7 @@ public bool TryUpdatePositions(FontMetrics fontMetrics, GlyphPositioningCollecti
                 continue;
             }
 
-            ScriptClass current = CodePoint.GetScriptClass(collection[i].CodePoint);
+            ScriptClass current = this.GetScriptClass(CodePoint.GetScriptClass(collection[i].CodePoint));
 
             int index = i;
             int count = 1;
@@ -132,7 +132,7 @@ public bool TryUpdatePositions(FontMetrics fontMetrics, GlyphPositioningCollecti
                     break;
                 }
 
-                ScriptClass next = CodePoint.GetScriptClass(nextData.CodePoint);
+                ScriptClass next = this.GetScriptClass(CodePoint.GetScriptClass(nextData.CodePoint));
                 if (next != current &&
                     current is not ScriptClass.Common and not ScriptClass.Unknown and not ScriptClass.Inherited &&
                     next is not ScriptClass.Common and not ScriptClass.Unknown and not ScriptClass.Inherited)
@@ -278,7 +278,7 @@ private Tag GetUnicodeScriptTag(ScriptClass script)
 
     private List<(Tag Feature, ushort Index, LookupTable LookupTable)> GetFeatureLookups(in Tag stageFeature, params LangSysTable[] langSysTables)
     {
-        List<(Tag Feature, ushort Index, LookupTable LookupTable)> lookups = new();
+        List<(Tag Feature, ushort Index, LookupTable LookupTable)> lookups = [];
         for (int i = 0; i < langSysTables.Length; i++)
         {
             ushort[] featureIndices = langSysTables[i].FeatureIndices;
@@ -306,6 +306,32 @@ private Tag GetUnicodeScriptTag(ScriptClass script)
         return lookups;
     }
 
+    private ScriptClass GetScriptClass(ScriptClass current)
+    {
+        if (current is ScriptClass.Common or ScriptClass.Unknown or ScriptClass.Inherited)
+        {
+            return current;
+        }
+
+        if (this.ScriptList is null)
+        {
+            return ScriptClass.Default;
+        }
+
+        Tag[] tags = UnicodeScriptTagMap.Instance[current];
+
+        for (int i = 0; i < tags.Length; i++)
+        {
+            if (this.ScriptList.TryGetValue(tags[i].Value, out ScriptListTable? _))
+            {
+                return current;
+            }
+        }
+
+        // Script for `current` not present in the font: use default shaper.
+        return ScriptClass.Default;
+    }
+
     private static bool HasFeature(List<TagEntry> glyphFeatures, in Tag feature)
     {
         for (int i = 0; i < glyphFeatures.Count; i++)
diff --git a/src/SixLabors.Fonts/Tables/AdvancedTypographic/GSub/LookupType2SubTable.cs b/src/SixLabors.Fonts/Tables/AdvancedTypographic/GSub/LookupType2SubTable.cs
@@ -23,7 +23,7 @@ public static LookupSubTable Load(BigEndianBinaryReader reader, long offset, Loo
     }
 }
 
-internal class LookupType2Format1SubTable : LookupSubTable
+internal sealed class LookupType2Format1SubTable : LookupSubTable
 {
     private readonly SequenceTable[] sequenceTables;
     private readonly CoverageTable coverageTable;
@@ -58,7 +58,7 @@ public static LookupType2Format1SubTable Load(BigEndianBinaryReader reader, long
         Span<ushort> sequenceOffsets = sequenceOffsetsBuffer.GetSpan();
         reader.ReadUInt16Array(sequenceOffsets);
 
-        var sequenceTables = new SequenceTable[sequenceCount];
+        SequenceTable[] sequenceTables = new SequenceTable[sequenceCount];
         for (int i = 0; i < sequenceTables.Length; i++)
         {
             // Sequence Table
@@ -75,7 +75,7 @@ public static LookupType2Format1SubTable Load(BigEndianBinaryReader reader, long
             sequenceTables[i] = new SequenceTable(reader.ReadUInt16Array(glyphCount));
         }
 
-        var coverageTable = CoverageTable.Load(reader, offset + coverageOffset);
+        CoverageTable coverageTable = CoverageTable.Load(reader, offset + coverageOffset);
 
         return new LookupType2Format1SubTable(sequenceTables, coverageTable, lookupFlags);
     }
diff --git a/src/SixLabors.Fonts/Tables/AdvancedTypographic/GSubTable.cs b/src/SixLabors.Fonts/Tables/AdvancedTypographic/GSubTable.cs
@@ -85,11 +85,11 @@ internal static GSubTable Load(BigEndianBinaryReader reader)
         uint featureVariationsOffset = (minorVersion == 1) ? reader.ReadOffset32() : 0;
 
         // TODO: Optimization. Allow only reading the scriptList.
-        var scriptList = ScriptList.Load(reader, scriptListOffset);
+        ScriptList? scriptList = ScriptList.Load(reader, scriptListOffset);
 
-        var featureList = FeatureListTable.Load(reader, featureListOffset);
+        FeatureListTable featureList = FeatureListTable.Load(reader, featureListOffset);
 
-        var lookupList = LookupListTable.Load(reader, lookupListOffset);
+        LookupListTable lookupList = LookupListTable.Load(reader, lookupListOffset);
 
         // TODO: Feature Variations.
         return new GSubTable(scriptList, featureList, lookupList);
@@ -106,7 +106,7 @@ public void ApplySubstitution(FontMetrics fontMetrics, GlyphSubstitutionCollecti
         {
             // Choose a shaper based on the script.
             // This determines which features to apply to which glyphs.
-            ScriptClass current = CodePoint.GetScriptClass(collection[i].CodePoint);
+            ScriptClass current = this.GetScriptClass(CodePoint.GetScriptClass(collection[i].CodePoint));
 
             int index = i;
             int count = 1;
@@ -115,7 +115,7 @@ public void ApplySubstitution(FontMetrics fontMetrics, GlyphSubstitutionCollecti
                 // We want to assign the same feature lookups to individual sections of the text rather
                 // than the text as a whole to ensure that different language shapers do not interfere
                 // with each other when the text contains multiple languages.
-                ScriptClass next = CodePoint.GetScriptClass(collection[i + 1].CodePoint);
+                ScriptClass next = this.GetScriptClass(CodePoint.GetScriptClass(collection[i + 1].CodePoint));
                 if (next != current &&
                     current is not ScriptClass.Common and not ScriptClass.Unknown and not ScriptClass.Inherited &&
                     next is not ScriptClass.Common and not ScriptClass.Unknown and not ScriptClass.Inherited)
@@ -291,7 +291,7 @@ private Tag GetUnicodeScriptTag(ScriptClass script)
 
     private List<(Tag Feature, ushort Index, LookupTable LookupTable)> GetFeatureLookups(in Tag stageFeature, params LangSysTable[] langSysTables)
     {
-        List<(Tag Feature, ushort Index, LookupTable LookupTable)> lookups = new();
+        List<(Tag Feature, ushort Index, LookupTable LookupTable)> lookups = [];
         for (int i = 0; i < langSysTables.Length; i++)
         {
             ushort[] featureIndices = langSysTables[i].FeatureIndices;
@@ -319,6 +319,32 @@ private Tag GetUnicodeScriptTag(ScriptClass script)
         return lookups;
     }
 
+    private ScriptClass GetScriptClass(ScriptClass current)
+    {
+        if (current is ScriptClass.Common or ScriptClass.Unknown or ScriptClass.Inherited)
+        {
+            return current;
+        }
+
+        if (this.ScriptList is null)
+        {
+            return ScriptClass.Default;
+        }
+
+        Tag[] tags = UnicodeScriptTagMap.Instance[current];
+
+        for (int i = 0; i < tags.Length; i++)
+        {
+            if (this.ScriptList.TryGetValue(tags[i].Value, out ScriptListTable? _))
+            {
+                return current;
+            }
+        }
+
+        // Script for `current` not present in the font: use default shaper.
+        return ScriptClass.Default;
+    }
+
     private static bool HasFeature(List<TagEntry> glyphFeatures, in Tag feature)
     {
         for (int i = 0; i < glyphFeatures.Count; i++)
diff --git a/src/SixLabors.Fonts/Tables/AdvancedTypographic/UnicodeScriptTagMap.cs b/src/SixLabors.Fonts/Tables/AdvancedTypographic/UnicodeScriptTagMap.cs
@@ -11,7 +11,7 @@ namespace SixLabors.Fonts.Tables.AdvancedTypographic;
 /// </summary>
 internal sealed class UnicodeScriptTagMap : Dictionary<ScriptClass, Tag[]>
 {
-    private static readonly Lazy<UnicodeScriptTagMap> Lazy = new(() => CreateMap(), true);
+    private static readonly Lazy<UnicodeScriptTagMap> Lazy = new(CreateMap, true);
 
     /// <summary>
     /// Prevents a default instance of the <see cref="UnicodeScriptTagMap"/> class from being created.
@@ -58,6 +58,7 @@ private static UnicodeScriptTagMap CreateMap()
             { ScriptClass.CyproMinoan, new[] { Tag.Parse("cpmn") } },
             { ScriptClass.Cypriot, new[] { Tag.Parse("cprt") } },
             { ScriptClass.Cyrillic, new[] { Tag.Parse("cyrl") } },
+            { ScriptClass.Default, new[] { Tag.Parse("DFLT"), Tag.Parse("dflt"), Tag.Parse("latn") } },
             { ScriptClass.Devanagari, new[] { Tag.Parse("dev2"), Tag.Parse("deva") } },
             { ScriptClass.DivesAkuru, new[] { Tag.Parse("diak") } },
             { ScriptClass.Dogra, new[] { Tag.Parse("dogr") } },
diff --git a/src/SixLabors.Fonts/Unicode/ScriptClass.cs b/src/SixLabors.Fonts/Unicode/ScriptClass.cs
@@ -822,4 +822,9 @@ public enum ScriptClass
     /// Shortcode: Zanb
     /// </summary>
     ZanabazarSquare,
+
+    /// <summary>
+    /// Shortcode: DFLT
+    /// </summary>
+    Default = 999
 }
diff --git a/tests/SixLabors.Fonts.Tests/FontMetricsTests.cs b/tests/SixLabors.Fonts.Tests/FontMetricsTests.cs
@@ -53,7 +53,7 @@ public void FontMetricsVerticalFontMatchesReference()
         // Compared to EveryFonts TTFDump metrics
         // https://everythingfonts.com/ttfdump
         FontCollection collection = new();
-        FontFamily family = collection.Add(TestFonts.NotoSansSCThinFile);
+        FontFamily family = collection.Add(TestFonts.NotoSansSCThinBad);
         Font font = family.CreateFont(12);
 
         Assert.Equal(1000, font.FontMetrics.UnitsPerEm);
@@ -240,7 +240,7 @@ public void GlyphMetricsVerticalMatchesReference()
         // Compared to EveryFonts TTFDump metrics
         // https://everythingfonts.com/ttfdump
         FontCollection collection = new();
-        FontFamily family = collection.Add(TestFonts.NotoSansSCThinFile);
+        FontFamily family = collection.Add(TestFonts.NotoSansSCThinBad);
         Font font = family.CreateFont(12);
 
         CodePoint codePoint = new('A');
diff --git a/tests/SixLabors.Fonts.Tests/Fonts/NotoSans-Regular.ttf b/tests/SixLabors.Fonts.Tests/Fonts/NotoSans-Regular.ttf
diff --git a/tests/SixLabors.Fonts.Tests/Fonts/NotoSansSC-Regular.ttf b/tests/SixLabors.Fonts.Tests/Fonts/NotoSansSC-Regular.ttf
diff --git a/tests/SixLabors.Fonts.Tests/Fonts/NotoSansSC-Thin-Bad.ttf b/tests/SixLabors.Fonts.Tests/Fonts/NotoSansSC-Thin-Bad.ttf
diff --git a/tests/SixLabors.Fonts.Tests/Fonts/NotoSansSC-Thin.ttf b/tests/SixLabors.Fonts.Tests/Fonts/NotoSansSC-Thin.ttf
diff --git a/tests/SixLabors.Fonts.Tests/Issues/Issues_469.cs b/tests/SixLabors.Fonts.Tests/Issues/Issues_469.cs
@@ -32,7 +32,7 @@ public void Test_Issue_469()
         string inconsolata = fontCollection.Add(TestFonts.InconsolataRegular).Name;
         string notoNaskhArabic = fontCollection.Add(TestFonts.NotoNaskhArabicRegular).Name;
         string notoSansJpThin = fontCollection.Add(TestFonts.NotoSansJPRegular).Name;
-        string notoSansScThin = fontCollection.Add(TestFonts.NotoSansSCRegular).Name;
+        string notoSansScThin = fontCollection.Add(TestFonts.NotoSansSCThin).Name;
         string sarabun = fontCollection.Add(TestFonts.SarabunRegular).Name;
 
         FontFamily mainFontFamily = fontCollection.Get(arial);
diff --git a/tests/SixLabors.Fonts.Tests/Issues/Issues_475.cs b/tests/SixLabors.Fonts.Tests/Issues/Issues_475.cs
@@ -0,0 +1,37 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+using SixLabors.Fonts.Rendering;
+
+namespace SixLabors.Fonts.Tests.Issues;
+
+public class Issues_475
+{
+    [Fact]
+    public void Test_Issue_475()
+    {
+        const string text = "වේගවත් දුඹුරු හිවලා කම්මැලි බල්ලා උඩින් පනී";
+
+        FontCollection fontCollection = new();
+        string noto = fontCollection.Add(TestFonts.NotoSansRegular).Name;
+        string sc = fontCollection.Add(TestFonts.NotoSansSCRegular).Name;
+
+        FontFamily mainFontFamily = fontCollection.Get(noto);
+        Font mainFont = mainFontFamily.CreateFont(30, FontStyle.Regular);
+
+        TextOptions options = new(mainFont)
+        {
+            FallbackFontFamilies =
+            [
+                fontCollection.Get(sc),
+            ],
+        };
+
+        // None of the fonts here can actually render the real glyphs in the text, just squares
+        // so just verify that we don't hit any exceptions and get the correct glyph count.
+        GlyphRenderer renderer = new();
+        TextRenderer.RenderTextTo(renderer, text, options);
+
+        Assert.Equal(43, renderer.GlyphRects.Count);
+    }
+}
diff --git a/tests/SixLabors.Fonts.Tests/TestFonts.cs b/tests/SixLabors.Fonts.Tests/TestFonts.cs
@@ -203,7 +203,7 @@ public static class TestFonts
 
     public static string Version1Font => GetFullPath("Font-Version1.ttf");
 
-    public static string NotoSansSCThinFile => GetFullPath("NotoSansSC-Thin.ttf");
+    public static string NotoSansSCThinBad => GetFullPath("NotoSansSC-Thin-Bad.ttf");
 
     public static string NotoSansKRRegular => GetFullPath("NotoSansKR-Regular.otf");
 
@@ -281,6 +281,8 @@ public static class TestFonts
 
     public static string NotoSansJPRegular => GetFullPath("NotoSansJP-Regular.ttf");
 
+    public static string NotoSansSCThin => GetFullPath("NotoSansSC-Thin.ttf");
+
     public static string NotoSansSCRegular => GetFullPath("NotoSansSC-Regular.ttf");
 
     public static string SarabunRegular => GetFullPath("Sarabun-Regular.ttf");

Original file line number	Diff line number	Diff line change
`@@ -199,7 +199,7 @@ public bool TryUpdate(Font font, GlyphSubstitutionCollection collection)`
`199`	`199`
`200`	`200`	`if (metrics.GlyphType != GlyphType.Fallback)`
`201`	`201`	`{`
`202`		`- if (j == 0)`
	`202`	`+ if (replacementCount == 0)`
`203`	`203`	`{`
`204`	`204`	`// There should only be a single fallback glyph at this position from the previous collection.`
`205`	`205`	`this.glyphs.RemoveAt(i);`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ public static LookupSubTable Load(BigEndianBinaryReader reader, long offset, Loo`
`23`	`23`	`}`
`24`	`24`	`}`
`25`	`25`
`26`		`-internal class LookupType2Format1SubTable : LookupSubTable`
	`26`	`+internal sealed class LookupType2Format1SubTable : LookupSubTable`
`27`	`27`	`{`
`28`	`28`	`private readonly SequenceTable[] sequenceTables;`
`29`	`29`	`private readonly CoverageTable coverageTable;`
`@@ -58,7 +58,7 @@ public static LookupType2Format1SubTable Load(BigEndianBinaryReader reader, long`
`58`	`58`	`Span<ushort> sequenceOffsets = sequenceOffsetsBuffer.GetSpan();`
`59`	`59`	`reader.ReadUInt16Array(sequenceOffsets);`
`60`	`60`
`61`		`- var sequenceTables = new SequenceTable[sequenceCount];`
	`61`	`+ SequenceTable[] sequenceTables = new SequenceTable[sequenceCount];`
`62`	`62`	`for (int i = 0; i < sequenceTables.Length; i++)`
`63`	`63`	`{`
`64`	`64`	`// Sequence Table`
`@@ -75,7 +75,7 @@ public static LookupType2Format1SubTable Load(BigEndianBinaryReader reader, long`
`75`	`75`	`sequenceTables[i] = new SequenceTable(reader.ReadUInt16Array(glyphCount));`
`76`	`76`	`}`
`77`	`77`
`78`		`- var coverageTable = CoverageTable.Load(reader, offset + coverageOffset);`
	`78`	`+ CoverageTable coverageTable = CoverageTable.Load(reader, offset + coverageOffset);`
`79`	`79`
`80`	`80`	`return new LookupType2Format1SubTable(sequenceTables, coverageTable, lookupFlags);`
`81`	`81`	`}`