Merge pull request #471 from SixLabors/js/fix-469

Ensure glyphs are not overwritten by fallbacks.

Author: JimBobSquarePants

SixLabors.Fonts.sln

@@ -73,6 +73,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "UnicodeTestData", "UnicodeT
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SixLabors.Fonts.Benchmarks", "tests\SixLabors.Fonts.Benchmarks\SixLabors.Fonts.Benchmarks\SixLabors.Fonts.Benchmarks.csproj", "{FB8FDC5F-7FEB-4132-9133-C25E05C0B3D9}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "DrawWithImageSharp", "samples\DrawWithImageSharp\DrawWithImageSharp.csproj", "{01863664-6C7E-61F2-F74B-7D451FFDC3C2}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -99,6 +101,10 @@ Global
 		{FB8FDC5F-7FEB-4132-9133-C25E05C0B3D9}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{FB8FDC5F-7FEB-4132-9133-C25E05C0B3D9}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{FB8FDC5F-7FEB-4132-9133-C25E05C0B3D9}.Release|Any CPU.Build.0 = Release|Any CPU
+		{01863664-6C7E-61F2-F74B-7D451FFDC3C2}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{01863664-6C7E-61F2-F74B-7D451FFDC3C2}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{01863664-6C7E-61F2-F74B-7D451FFDC3C2}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{01863664-6C7E-61F2-F74B-7D451FFDC3C2}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -113,6 +119,7 @@ Global
 		{ABB6E111-672F-4846-88D6-C49C6CD01606} = {249327CF-1415-428B-8EEA-8C7705B1DE8F}
 		{654DD381-B93D-4459-B669-296F5D9172ED} = {56801022-D71A-4FBE-BC5B-CBA08E2284EC}
 		{FB8FDC5F-7FEB-4132-9133-C25E05C0B3D9} = {56801022-D71A-4FBE-BC5B-CBA08E2284EC}
+		{01863664-6C7E-61F2-F74B-7D451FFDC3C2} = {71A3911C-D6B9-4EBE-9691-2FE28BDF462E}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {38F4B47F-4F74-40F5-8707-C0EF1D0BDF92}

samples/DrawWithImageSharp/DrawWithImageSharp.csproj

@@ -46,8 +46,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="SixLabors.ImageSharp.Drawing" Version="2.1.4" />
-    <PackageReference Include="System.ValueTuple" Version="4.5.0" />
+    <PackageReference Include="SixLabors.ImageSharp.Drawing" Version="3.0.0-alpha.0.6" />
   </ItemGroup>
 
 </Project>

samples/DrawWithImageSharp/Program.cs

@@ -9,6 +9,7 @@
 using SixLabors.ImageSharp.Drawing;
 using SixLabors.ImageSharp.Drawing.Processing;
 using SixLabors.ImageSharp.Drawing.Processing.Processors.Text;
+using SixLabors.ImageSharp.Drawing.Text;
 using SixLabors.ImageSharp.PixelFormats;
 using SixLabors.ImageSharp.Processing;
 using IOPath = System.IO.Path;
@@ -218,7 +219,7 @@ public static void RenderText(Font font, string text, int width, int height)
         using var img = new Image<Rgba32>(width, height);
         img.Mutate(x => x.Fill(Color.White));
 
-        IPathCollection shapes = TextBuilder.GenerateGlyphs(text, new RichTextOptions(font) { Origin = new Vector2(50f, 4f) });
+        IPathCollection shapes = TextBuilder.GeneratePaths(text, new RichTextOptions(font) { Origin = new Vector2(50f, 4f) });
         img.Mutate(x => x.Fill(Color.Black, shapes));
 
         Directory.CreateDirectory(IOPath.GetDirectoryName(fullPath));

src/SixLabors.Fonts/GlyphPositioningCollection.cs

@@ -17,7 +17,7 @@ internal sealed class GlyphPositioningCollection : IGlyphShapingCollection
     /// <summary>
     /// Contains a map the index of a map within the collection, non-sequential codepoint offsets, and their glyph ids, point size, and mtrics.
     /// </summary>
-    private readonly List<GlyphPositioningData> glyphs = new();
+    private readonly List<GlyphPositioningData> glyphs = [];
 
     /// <summary>
     /// Initializes a new instance of the <see cref="GlyphPositioningCollection"/> class.
@@ -149,6 +149,11 @@ public bool TryUpdate(Font font, GlyphSubstitutionCollection collection)
         ColorFontSupport colorFontSupport = this.TextOptions.ColorFontSupport;
         bool hasFallBacks = false;
         List<int> orphans = [];
+
+        Tag vert = FeatureTags.VerticalAlternates;
+        Tag vrt2 = FeatureTags.VerticalAlternatesAndRotation;
+        Tag vrtr = FeatureTags.VerticalAlternatesForRotation;
+
         for (int i = 0; i < this.glyphs.Count; i++)
         {
             GlyphPositioningData current = this.glyphs[i];
@@ -173,6 +178,15 @@ public bool TryUpdate(Font font, GlyphSubstitutionCollection collection)
                     // cache the original in the font metrics and only update our collection.
                     TextAttributes textAttributes = shape.TextRun.TextAttributes;
                     TextDecorations textDecorations = shape.TextRun.TextDecorations;
+
+                    bool isVertical = AdvancedTypographicUtils.IsVerticalGlyph(codePoint, layoutMode);
+                    foreach (Tag feature in shape.AppliedFeatures)
+                    {
+                        isVertical |= feature == vert;
+                        isVertical |= feature == vrt2;
+                        isVertical |= feature == vrtr;
+                    }
+
                     GlyphMetrics metrics = fontMetrics.GetGlyphMetrics(codePoint, id, textAttributes, textDecorations, layoutMode, colorFontSupport);
                     {
                         // If the glyphs are fallbacks we don't want them as
@@ -183,16 +197,20 @@ public bool TryUpdate(Font font, GlyphSubstitutionCollection collection)
                         }
                     }
 
-                    if (!hasFallBacks)
+                    if (metrics.GlyphType != GlyphType.Fallback)
                     {
                         if (j == 0)
                         {
                             // There should only be a single fallback glyph at this position from the previous collection.
                             this.glyphs.RemoveAt(i);
                         }
 
+                        // We only want a single dimensional advance for positioning.
+                        GlyphShapingBounds bounds = isVertical
+                            ? new(0, 0, 0, metrics.AdvanceHeight)
+                            : new(0, 0, metrics.AdvanceWidth, 0);
+
                         // Track the number of inserted glyphs at the offset so we can correctly increment our position.
-                        GlyphShapingBounds bounds = new(0, 0, metrics.AdvanceWidth, metrics.AdvanceHeight);
                         this.glyphs.Insert(i += replacementCount, new(offset, new(shape, true) { Bounds = bounds }, pointSize, metrics.CloneForRendering(shape.TextRun)));
                         replacementCount++;
                     }
@@ -259,6 +277,7 @@ public bool TryAdd(Font font, GlyphSubstitutionCollection collection)
                 hasFallBacks = true;
             }
 
+            // We only want a single dimensional advance for positioning.
             GlyphShapingBounds bounds = isVertical
                 ? new(0, 0, 0, metrics.AdvanceHeight)
                 : new(0, 0, metrics.AdvanceWidth, 0);

src/SixLabors.Fonts/Tables/AdvancedTypographic/GPosTable.cs

@@ -125,7 +125,13 @@ public bool TryUpdatePositions(FontMetrics fontMetrics, GlyphPositioningCollecti
                 // 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.
-                GlyphShapingData nextData = collection[i + 1];
+                int ni = i + 1;
+                GlyphShapingData nextData = collection[ni];
+                if (!collection.ShouldProcess(fontMetrics, ni))
+                {
+                    break;
+                }
+
                 ScriptClass next = CodePoint.GetScriptClass(nextData.CodePoint);
                 if (next != current &&
                     current is not ScriptClass.Common and not ScriptClass.Unknown and not ScriptClass.Inherited &&

src/SixLabors.Fonts/Tables/General/HorizontalMetricsTable.cs

@@ -19,7 +19,10 @@ public ushort GetAdvancedWidth(int glyphIndex)
     {
         if (glyphIndex >= this.advancedWidths.Length)
         {
-            return this.advancedWidths[0];
+            // Records are indexed by glyph ID. As an optimization, the number of records can
+            // be less than the number of glyphs, in which case the advance width value of the
+            // last record applies to all remaining glyph IDs.
+            return this.advancedWidths[^1];
         }
 
         return this.advancedWidths[glyphIndex];
@@ -29,7 +32,7 @@ internal short GetLeftSideBearing(int glyphIndex)
     {
         if (glyphIndex >= this.leftSideBearings.Length)
         {
-            return this.leftSideBearings[0];
+            return this.leftSideBearings[^1];
         }
 
         return this.leftSideBearings[glyphIndex];