Merge pull request #405 from SixLabors/js/fix-kerning

Apply kerning table offset to correct glyph.

samples/DrawWithImageSharp/DrawWithImageSharp.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <DebugType>portable</DebugType>
@@ -46,7 +46,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="SixLabors.ImageSharp.Drawing" Version="2.0.1" />
+    <PackageReference Include="SixLabors.ImageSharp.Drawing" Version="2.1.3" />
     <PackageReference Include="System.ValueTuple" Version="4.5.0" />
   </ItemGroup>
 

src/SixLabors.Fonts/FileFontMetrics.cs

@@ -148,8 +148,8 @@ internal override void ApplySubstitution(GlyphSubstitutionCollection collection)
         => this.fontMetrics.Value.ApplySubstitution(collection);
 
     /// <inheritdoc/>
-    internal override bool TryGetKerningOffset(ushort previousId, ushort currentId, out Vector2 vector)
-        => this.fontMetrics.Value.TryGetKerningOffset(previousId, currentId, out vector);
+    internal override bool TryGetKerningOffset(ushort currentId, ushort nextId, out Vector2 vector)
+        => this.fontMetrics.Value.TryGetKerningOffset(currentId, nextId, out vector);
 
     /// <inheritdoc/>
     internal override void UpdatePositions(GlyphPositioningCollection collection)

src/SixLabors.Fonts/Font.cs

@@ -253,11 +253,11 @@ public bool TryGetGlyphs(
     }
 
     /// <summary>
-    /// Gets the amount, in px units, the <paramref name="current"/> glyph should be offset if it is proceeded by
-    /// the <paramref name="previous"/> glyph.
+    /// Gets the amount, in px units, the <paramref name="current"/> glyph should be offset if it is followed by
+    /// the <paramref name="next"/> glyph.
     /// </summary>
-    /// <param name="previous">The previous glyph.</param>
     /// <param name="current">The current glyph.</param>
+    /// <param name="next">The next glyph.</param>
     /// <param name="dpi">The DPI (Dots Per Inch) to render/measure the kerning offset at.</param>
     /// <param name="vector">
     /// When this method returns, contains the offset, in font units, that should be applied to the
@@ -267,12 +267,12 @@ public bool TryGetGlyphs(
     /// <returns>
     /// <see langword="true"/> if the face contains and offset for the glyph combination; otherwise, <see langword="false"/>.
     /// </returns>
-    public bool TryGetKerningOffset(Glyph previous, Glyph current, float dpi, out Vector2 vector)
+    public bool TryGetKerningOffset(Glyph current, Glyph next, float dpi, out Vector2 vector)
     {
-        if (this.FontMetrics.TryGetKerningOffset(previous.GlyphMetrics.GlyphId, current.GlyphMetrics.GlyphId, out vector))
+        if (this.FontMetrics.TryGetKerningOffset(current.GlyphMetrics.GlyphId, next.GlyphMetrics.GlyphId, out vector))
         {
             // Scale the result
-            Vector2 scale = new Vector2(this.Size * dpi) / current.GlyphMetrics.ScaleFactor;
+            Vector2 scale = new Vector2(this.Size * dpi) / next.GlyphMetrics.ScaleFactor;
             vector *= scale;
             return true;
         }

src/SixLabors.Fonts/FontMetrics.cs

@@ -223,11 +223,11 @@ internal abstract IReadOnlyList<GlyphMetrics> GetGlyphMetrics(
     internal abstract void ApplySubstitution(GlyphSubstitutionCollection collection);
 
     /// <summary>
-    /// Gets the amount, in font units, the <paramref name="currentId"/> glyph should be offset if it is proceeded by
-    /// the <paramref name="previousId"/> glyph.
+    /// Gets the amount, in font units, the <paramref name="currentId"/> glyph should be offset if it is followed by
+    /// the <paramref name="nextId"/> glyph.
     /// </summary>
-    /// <param name="previousId">The previous glyph id.</param>
     /// <param name="currentId">The current glyph id.</param>
+    /// <param name="nextId">The next glyph id.</param>
     /// <param name="vector">
     /// When this method returns, contains the offset, in font units, that should be applied to the
     /// <paramref name="currentId"/> glyph, if the offset is found; otherwise the default vector value.
@@ -236,7 +236,7 @@ internal abstract IReadOnlyList<GlyphMetrics> GetGlyphMetrics(
     /// <returns>
     /// <see langword="true"/> if the face contains and offset for the glyph combination; otherwise, <see langword="false"/>.
     /// </returns>
-    internal abstract bool TryGetKerningOffset(ushort previousId, ushort currentId, out Vector2 vector);
+    internal abstract bool TryGetKerningOffset(ushort currentId, ushort nextId, out Vector2 vector);
 
     /// <summary>
     /// Applies any available positioning updates to the collection of glyphs.

src/SixLabors.Fonts/StreamFontMetrics.cs

@@ -272,7 +272,7 @@ internal override void ApplySubstitution(GlyphSubstitutionCollection collection)
     }
 
     /// <inheritdoc/>
-    internal override bool TryGetKerningOffset(ushort previousId, ushort currentId, out Vector2 vector)
+    internal override bool TryGetKerningOffset(ushort currentId, ushort nextId, out Vector2 vector)
     {
         bool isTTF = this.outlineType == OutlineType.TrueType;
         KerningTable? kern = isTTF
@@ -285,7 +285,7 @@ internal override bool TryGetKerningOffset(ushort previousId, ushort currentId,
             return false;
         }
 
-        return kern.TryGetKerningOffset(previousId, currentId, out vector);
+        return kern.TryGetKerningOffset(currentId, nextId, out vector);
     }
 
     /// <inheritdoc/>
@@ -312,14 +312,14 @@ internal override void UpdatePositions(GlyphPositioningCollection collection)
             {
                 // Set max constraints to prevent OutOfMemoryException or infinite loops from attacks.
                 int maxCount = AdvancedTypographicUtils.GetMaxAllowableShapingCollectionCount(collection.Count);
-                for (int index = 1; index < collection.Count; index++)
+                for (int index = 0; index < collection.Count - 1; index++)
                 {
                     if (index >= maxCount)
                     {
                         break;
                     }
 
-                    kern.UpdatePositions(this, collection, index - 1, index);
+                    kern.UpdatePositions(this, collection, index, index + 1);
                 }
             }
         }

src/SixLabors.Fonts/Tables/General/Kern/KerningTable.cs

@@ -42,10 +42,10 @@ public static KerningTable Load(BigEndianBinaryReader reader)
         ushort version = reader.ReadUInt16();
         ushort subTableCount = reader.ReadUInt16();
 
-        var tables = new List<KerningSubTable>(subTableCount);
+        List<KerningSubTable> tables = new(subTableCount);
         for (int i = 0; i < subTableCount; i++)
         {
-            var t = KerningSubTable.Load(reader); // returns null for unknown/supported table format
+            KerningSubTable? t = KerningSubTable.Load(reader); // returns null for unknown/supported table format
             if (t != null)
             {
                 tables.Add(t);
@@ -62,27 +62,27 @@ public void UpdatePositions(FontMetrics fontMetrics, GlyphPositioningCollection
             return;
         }
 
-        ushort previous = collection[left].GlyphId;
-        ushort current = collection[right].GlyphId;
+        ushort current = collection[left].GlyphId;
+        ushort next = collection[right].GlyphId;
 
-        if (this.TryGetKerningOffset(previous, current, out Vector2 result))
+        if (this.TryGetKerningOffset(current, next, out Vector2 result))
         {
-            collection.Advance(fontMetrics, right, current, (short)result.X, (short)result.Y);
+            collection.Advance(fontMetrics, left, current, (short)result.X, (short)result.Y);
         }
     }
 
-    public bool TryGetKerningOffset(ushort previous, ushort current, out Vector2 result)
+    public bool TryGetKerningOffset(ushort current, ushort next, out Vector2 result)
     {
         result = Vector2.Zero;
-        if (this.Count == 0 || previous == 0 || current == 0)
+        if (this.Count == 0 || current == 0 || next == 0)
         {
             return false;
         }
 
         bool kerned = false;
         foreach (KerningSubTable sub in this.kerningSubTable)
         {
-            kerned |= sub.TryApplyOffset(previous, current, ref result);
+            kerned |= sub.TryApplyOffset(current, next, ref result);
         }
 
         return kerned;

tests/SixLabors.Fonts.Tests/Issues/Issues_403.cs

@@ -0,0 +1,26 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+namespace SixLabors.Fonts.Tests.Issues;
+
+public class Issues_403
+{
+    // https://github.com/SixLabors/Fonts/discussions/403
+    [Fact]
+    public void DoesNotKernIncorrectly()
+    {
+        Font font = new FontCollection().Add(TestFonts.KellySlabFile).CreateFont(2048);
+
+        TextOptions options = new(font) { KerningMode = KerningMode.Auto };
+        FontRectangle kerned = TextMeasurer.MeasureSize("AY", options);
+
+        options.KerningMode = KerningMode.None;
+
+        FontRectangle unkerned = TextMeasurer.MeasureSize("AY", options);
+
+        Assert.Equal(kerned.Left, unkerned.Left);
+        Assert.Equal(kerned.Top, unkerned.Top);
+        Assert.True(kerned.Right < unkerned.Right);
+        Assert.Equal(kerned.Bottom, unkerned.Bottom);
+    }
+}

tests/SixLabors.Fonts.Tests/TestFonts.cs

@@ -255,6 +255,8 @@ public static class TestFonts
 
     public static string CourierPrimeFile => GetFullPath("courier-prime.woff2");
 
+    public static string KellySlabFile => GetFullPath("KellySlab-Regular.ttf");
+
     public static Stream TwemojiMozillaData() => OpenStream(TwemojiMozillaFile);
 
     public static Stream SegoeuiEmojiData() => OpenStream(SegoeuiEmojiFile);