Merge pull request #440 from SixLabors/js/additional-linebreak-fixes

JimBobSquarePants · web-flow · commit 24871a8b7dd9 · 2025-01-10T22:53:10.000+10:00
Fix calculation of wrapping advance
diff --git a/src/SixLabors.Fonts/TextLayout.cs b/src/SixLabors.Fonts/TextLayout.cs
@@ -894,6 +894,8 @@ private static TextBox BreakLines(
         LayoutMode layoutMode)
     {
         bool shouldWrap = options.WrappingLength > 0;
+
+        // Wrapping length is always provided in pixels. Convert to inches for comparison.
         float wrappingLength = shouldWrap ? options.WrappingLength / options.Dpi : float.MaxValue;
         bool breakAll = options.WordBreaking == WordBreaking.BreakAll;
         bool keepAll = options.WordBreaking == WordBreaking.KeepAll;
@@ -904,7 +906,6 @@ private static TextBox BreakLines(
 
         int graphemeIndex;
         int codePointIndex = 0;
-        float lineAdvance = 0;
         List<TextLine> textLines = new();
         TextLine textLine = new();
         int stringIndex = 0;
@@ -1073,7 +1074,7 @@ VerticalOrientationType.Rotate or
                     }
                 }
 
-                // Now scale the advance.
+                // Now scale the advance. We use inches for comparison.
                 if (isHorizontalLayout || shouldRotate)
                 {
                     float scaleAX = pointSize / glyph.ScaleFactor.X;
@@ -1115,7 +1116,6 @@ VerticalOrientationType.Rotate or
                     descender -= delta;
 
                     // Add our metrics to the line.
-                    lineAdvance += decomposedAdvance;
                     textLine.Add(
                         isDecomposed ? new GlyphMetrics[] { metric } : metrics,
                         pointSize,
@@ -1153,58 +1153,60 @@ VerticalOrientationType.Rotate or
         int maxLineBreakIndex = lineBreaks.Count - 1;
         LineBreak lastLineBreak = lineBreaks[lineBreakIndex];
         LineBreak currentLineBreak = lineBreaks[lineBreakIndex];
+        float lineAdvance = 0;
 
-        lineAdvance = 0;
         for (int i = 0; i < textLine.Count; i++)
         {
             int max = textLine.Count - 1;
             TextLine.GlyphLayoutData glyph = textLine[i];
             codePointIndex = glyph.CodePointIndex;
             int graphemeCodePointIndex = glyph.GraphemeCodePointIndex;
-            float glyphAdvance = glyph.ScaledAdvance;
-            lineAdvance += glyphAdvance;
 
             if (graphemeCodePointIndex == 0 && textLine.Count > 0)
             {
+                lineAdvance += glyph.ScaledAdvance;
+
                 if (codePointIndex == currentLineBreak.PositionWrap && currentLineBreak.Required)
                 {
                     // Mandatory line break at index.
                     TextLine remaining = textLine.SplitAt(i);
                     textLines.Add(textLine.Finalize(options));
                     textLine = remaining;
-                    i = 0;
+                    i = -1;
                     lineAdvance = 0;
                 }
                 else if (shouldWrap)
                 {
-                    float currentAdvance = lineAdvance + glyphAdvance;
-                    if (currentAdvance >= wrappingLength)
+                    if (lineAdvance >= wrappingLength)
                     {
                         if (breakAll)
                         {
-                            // Insert a forced break at this index.
+                            // Insert a forced break.
                             TextLine remaining = textLine.SplitAt(i);
-                            textLines.Add(textLine.Finalize(options));
-                            textLine = remaining;
-                            i = 0;
-                            lineAdvance = 0;
+                            if (remaining != textLine)
+                            {
+                                textLines.Add(textLine.Finalize(options));
+                                textLine = remaining;
+                                i = -1;
+                                lineAdvance = 0;
+                            }
                         }
                         else if (codePointIndex == currentLineBreak.PositionWrap || i == max)
                         {
-                            LineBreak lineBreak = currentAdvance == wrappingLength
+                            LineBreak lineBreak = lineAdvance == wrappingLength
                                 ? currentLineBreak
                                 : lastLineBreak;
 
                             if (i > 0)
                             {
                                 // If the current break is a space, and the line minus the space
                                 // is less than the wrapping length, we can break using the current break.
-                                float positionAdvance = lineAdvance;
+                                float previousAdvance = lineAdvance - (float)glyph.ScaledAdvance;
                                 TextLine.GlyphLayoutData lastGlyph = textLine[i - 1];
                                 if (CodePoint.IsWhiteSpace(lastGlyph.CodePoint))
                                 {
-                                    positionAdvance -= lastGlyph.ScaledAdvance;
-                                    if (positionAdvance <= wrappingLength)
+                                    previousAdvance -= lastGlyph.ScaledAdvance;
+                                    if (previousAdvance <= wrappingLength)
                                     {
                                         lineBreak = currentLineBreak;
                                     }
@@ -1220,7 +1222,7 @@ VerticalOrientationType.Rotate or
                             {
                                 if (breakWord)
                                 {
-                                    // If the line is too long, insert a forced line break.
+                                    // If the line is too long, insert a forced break.
                                     if (textLine.ScaledLineAdvance > wrappingLength)
                                     {
                                         TextLine overflow = textLine.SplitAt(wrappingLength);
@@ -1233,7 +1235,7 @@ VerticalOrientationType.Rotate or
 
                                 textLines.Add(textLine.Finalize(options));
                                 textLine = remaining;
-                                i = 0;
+                                i = -1;
                                 lineAdvance = 0;
                             }
                         }
@@ -1326,7 +1328,11 @@ public void Add(
         {
             // Reset metrics.
             // We track the maximum metrics for each line to ensure glyphs can be aligned.
-            this.ScaledLineAdvance += scaledAdvance;
+            if (graphemeIndex == 0)
+            {
+                this.ScaledLineAdvance += scaledAdvance;
+            }
+
             this.ScaledMaxLineHeight = MathF.Max(this.ScaledMaxLineHeight, scaledLineHeight);
             this.ScaledMaxAscender = MathF.Max(this.ScaledMaxAscender, scaledAscender);
             this.ScaledMaxDescender = MathF.Max(this.ScaledMaxDescender, scaledDescender);
diff --git a/tests/Images/ReferenceOutput/CountLinesWrappingLength_100-3.png b/tests/Images/ReferenceOutput/CountLinesWrappingLength_100-3.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:9c28b19171f3bf75e7eb259492b213dac0132f3664bd29a217bacb08a93c5b6a
+size 2929
diff --git a/tests/Images/ReferenceOutput/CountLinesWrappingLength_100-4.png b/tests/Images/ReferenceOutput/CountLinesWrappingLength_100-4.png
diff --git a/tests/Images/ReferenceOutput/CountLinesWrappingLength_50-4.png b/tests/Images/ReferenceOutput/CountLinesWrappingLength_50-4.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:08378706fcf31fa5efaea158bde3b6350f9e96c5721364f7a1b88ac138f4aaca
+size 3021
diff --git a/tests/Images/ReferenceOutput/CountLinesWrappingLength_50-5.png b/tests/Images/ReferenceOutput/CountLinesWrappingLength_50-5.png
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordBreakMatchesMDN_238-_layoutMode_HorizontalBottomTop-wordBreaking_BreakAll_.png b/tests/Images/ReferenceOutput/MeasureTextWordBreakMatchesMDN_238-_layoutMode_HorizontalBottomTop-wordBreaking_BreakAll_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:e13708fcaf702a91540b1f68803cc2cff14de1a97cef771ca0bcc69e414a706e
-size 13446
+oid sha256:2a23777abd77520f78d269148cee286e23941fea273a7f7a73fcd3a16a724c19
+size 13365
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordBreakMatchesMDN_238-_layoutMode_HorizontalTopBottom-wordBreaking_BreakAll_.png b/tests/Images/ReferenceOutput/MeasureTextWordBreakMatchesMDN_238-_layoutMode_HorizontalTopBottom-wordBreaking_BreakAll_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:1896cd5f5d0521261c4b9e366ae56baa89f48cfd21b7d1a5d4a4e4c50b3f8542
-size 13485
+oid sha256:f0ba4662e9b8a24d4bd852d712d5a2a5f34b9aeadf332f12fdff172d3536b4e8
+size 13396
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordBreak_500-_layoutMode_HorizontalBottomTop-wordBreaking_BreakAll_.png b/tests/Images/ReferenceOutput/MeasureTextWordBreak_500-_layoutMode_HorizontalBottomTop-wordBreaking_BreakAll_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:66b5c1046ab68824efdd3af54c7753a18d7bb12b7a2960c956395b0d20bf19e2
-size 17444
+oid sha256:6d7e8d7d4d63cfe3082b571d7bb156497dd738af448126f319f7be6870317050
+size 17604
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordBreak_500-_layoutMode_HorizontalTopBottom-wordBreaking_BreakAll_.png b/tests/Images/ReferenceOutput/MeasureTextWordBreak_500-_layoutMode_HorizontalTopBottom-wordBreaking_BreakAll_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:a99e819c0a8d7380e7afbdd289ebe0a2ee6f8c62b51ab7d10b06cfddf0729c55
-size 17437
+oid sha256:1fc37c69f57e01534abac8d6ae2c441f88dca4f7111f6b3209f8769ba2a3ac37
+size 17502
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordWrappingHorizontalBottomTop_350-_height_62.625-width_318.86_.png b/tests/Images/ReferenceOutput/MeasureTextWordWrappingHorizontalBottomTop_350-_height_62.625-width_318.86_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:bcf9ccad091fde6016070afcaa24e9040d6a81672f04350e56aa446802675bff
-size 9272
+oid sha256:e272b2bd807c9137d620115fe9f656cb5c810f967a8b2e85d43a0c87c6f62f56
+size 9240
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordWrappingHorizontalTopBottom_350-_height_62.625-width_318.86_.png b/tests/Images/ReferenceOutput/MeasureTextWordWrappingHorizontalTopBottom_350-_height_62.625-width_318.86_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:afd34328b9e945944d90f8e56c33cff7d796a5ab22a55a4b1be5eeb629fb2f5a
-size 9268
+oid sha256:c6a65e60c84cb26ad6cdf4241be47cf00ebdc5ad425da1278a683c7dd709f785
+size 9256
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordWrappingVerticalLeftRight_350-_height_318.563-width_62.813_.png b/tests/Images/ReferenceOutput/MeasureTextWordWrappingVerticalLeftRight_350-_height_318.563-width_62.813_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:57b1755082a3e82879b28ff20f2e4e2cd017aa8b0b236678a8dbf5cc6ddab17d
-size 10317
+oid sha256:6eb2b3a67d130574b75e917cd37f161a07b74fe7ce79e5b2c42abdf9de96e1c0
+size 10300
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordWrappingVerticalMixedLeftRight_350-_height_318.563-width_62.813_.png b/tests/Images/ReferenceOutput/MeasureTextWordWrappingVerticalMixedLeftRight_350-_height_318.563-width_62.813_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:57b1755082a3e82879b28ff20f2e4e2cd017aa8b0b236678a8dbf5cc6ddab17d
-size 10317
+oid sha256:6eb2b3a67d130574b75e917cd37f161a07b74fe7ce79e5b2c42abdf9de96e1c0
+size 10300
diff --git a/tests/Images/ReferenceOutput/MeasureTextWordWrappingVerticalRightLeft_350-_height_318.563-width_62.813_.png b/tests/Images/ReferenceOutput/MeasureTextWordWrappingVerticalRightLeft_350-_height_318.563-width_62.813_.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:b4b0bc75fa8b2ff464b38401741a58ce0fd7095767883d8fcd227e336a8f9c08
-size 10241
+oid sha256:7161d913d93c3e50cb67dcb57d42afcee5a117b10aa65ff7b8d03ec8a9f9bc93
+size 10313
diff --git a/tests/Images/ReferenceOutput/ShouldNotInsertExtraLineBreaks_2__WrappingLength_400_.png b/tests/Images/ReferenceOutput/ShouldNotInsertExtraLineBreaks_2__WrappingLength_400_.png
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:18b14563c798925aaeee959bbeb12bd392af681ab620fb15a0264f7f2904f2a6
+size 14829
diff --git a/tests/SixLabors.Fonts.Tests/Issues/Issues_431.cs b/tests/SixLabors.Fonts.Tests/Issues/Issues_431.cs
@@ -21,13 +21,37 @@ public void ShouldNotInsertExtraLineBreaks()
                 WrappingLength = 400,
             };
 
+            TextLayoutTestUtilities.TestLayout(text, options);
+
             int lineCount = TextMeasurer.CountLines(text, options);
             Assert.Equal(4, lineCount);
 
             IReadOnlyList<GlyphLayout> layout = TextLayout.GenerateLayout(text, options);
             Assert.Equal(46, layout.Count);
+        }
+    }
+
+    [Fact]
+    public void ShouldNotInsertExtraLineBreaks_2()
+    {
+        if (SystemFonts.TryGet("Arial", out FontFamily family))
+        {
+            Font font = family.CreateFont(60);
+            const string text = "- Lorem ipsullll dolor sit amet\n-consectetur elit";
+
+            TextOptions options = new(font)
+            {
+                Origin = new Vector2(50, 20),
+                WrappingLength = 400,
+            };
 
             TextLayoutTestUtilities.TestLayout(text, options);
+
+            int lineCount = TextMeasurer.CountLines(text, options);
+            Assert.Equal(4, lineCount);
+
+            IReadOnlyList<GlyphLayout> layout = TextLayout.GenerateLayout(text, options);
+            Assert.Equal(46, layout.Count);
         }
     }
 }
diff --git a/tests/SixLabors.Fonts.Tests/TextLayoutTests.cs b/tests/SixLabors.Fonts.Tests/TextLayoutTests.cs
@@ -3,6 +3,7 @@
 
 using System.Globalization;
 using System.Numerics;
+using System.Text;
 using SixLabors.Fonts.Tests.Fakes;
 using SixLabors.Fonts.Unicode;
 using SixLabors.ImageSharp.Drawing.Processing;
@@ -289,11 +290,11 @@ public void MeasureTextWordWrappingHorizontalTopBottom(string text, float height
                 LayoutMode = LayoutMode.HorizontalTopBottom
             };
 
-            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
+            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
 
+            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
             Assert.Equal(width, size.Width, 4F);
             Assert.Equal(height, size.Height, 4F);
-            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
         }
     }
 
@@ -315,11 +316,12 @@ public void MeasureTextWordWrappingHorizontalBottomTop(string text, float height
                 LayoutMode = LayoutMode.HorizontalBottomTop
             };
 
-            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
+            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
+
 
+            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
             Assert.Equal(width, size.Width, 4F);
             Assert.Equal(height, size.Height, 4F);
-            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
         }
     }
 
@@ -338,11 +340,11 @@ public void MeasureTextWordWrappingVerticalLeftRight(string text, float height,
                 LayoutMode = LayoutMode.VerticalLeftRight
             };
 
-            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
+            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
 
+            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
             Assert.Equal(width, size.Width, 4F);
             Assert.Equal(height, size.Height, 4F);
-            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
         }
     }
 
@@ -361,11 +363,11 @@ public void MeasureTextWordWrappingVerticalRightLeft(string text, float height,
                 LayoutMode = LayoutMode.VerticalRightLeft
             };
 
-            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
+            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
 
+            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
             Assert.Equal(width, size.Width, 4F);
             Assert.Equal(height, size.Height, 4F);
-            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
         }
     }
 
@@ -384,11 +386,11 @@ public void MeasureTextWordWrappingVerticalMixedLeftRight(string text, float hei
                 LayoutMode = LayoutMode.VerticalMixedLeftRight
             };
 
-            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
+            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
 
+            FontRectangle size = TextMeasurer.MeasureBounds(text, options);
             Assert.Equal(width, size.Width, 4F);
             Assert.Equal(height, size.Height, 4F);
-            TextLayoutTestUtilities.TestLayout(text, options, properties: new { height, width });
         }
     }
 
@@ -416,14 +418,11 @@ public void MeasureTextWordBreakMatchesMDN(string text, LayoutMode layoutMode, W
                 FallbackFontFamilies = new[] { jhengHei }
             };
 
-            FontRectangle size = TextMeasurer.MeasureAdvance(
-                text,
-                options);
+            TextLayoutTestUtilities.TestLayout(text, options, properties: new { layoutMode, wordBreaking });
 
+            FontRectangle size = TextMeasurer.MeasureAdvance(text, options);
             Assert.Equal(width, size.Width, 4F);
             Assert.Equal(height, size.Height, 4F);
-
-            TextLayoutTestUtilities.TestLayout(text, options, properties: new { layoutMode, wordBreaking });
         }
     }
 
@@ -455,10 +454,10 @@ public void MeasureTextWordBreak(string text, LayoutMode layoutMode, WordBreakin
                 text,
                 options);
 
+            TextLayoutTestUtilities.TestLayout(text, options, properties: new { layoutMode, wordBreaking });
+
             Assert.Equal(width, size.Width, 4F);
             Assert.Equal(height, size.Height, 4F);
-
-            TextLayoutTestUtilities.TestLayout(text, options, properties: new { layoutMode, wordBreaking });
         }
     }
 
@@ -550,8 +549,8 @@ public void CountLinesWithSpan()
 
     [Theory]
     [InlineData("This is a long and Honorificabilitudinitatibus califragilisticexpialidocious", 25, 6)]
-    [InlineData("This is a long and Honorificabilitudinitatibus califragilisticexpialidocious", 50, 5)]
-    [InlineData("This is a long and Honorificabilitudinitatibus califragilisticexpialidocious", 100, 4)]
+    [InlineData("This is a long and Honorificabilitudinitatibus califragilisticexpialidocious", 50, 4)]
+    [InlineData("This is a long and Honorificabilitudinitatibus califragilisticexpialidocious", 100, 3)]
     [InlineData("This is a long and Honorificabilitudinitatibus califragilisticexpialidocious", 200, 3)]
     public void CountLinesWrappingLength(string text, int wrappingLength, int usedLines)
     {
@@ -561,9 +560,10 @@ public void CountLinesWrappingLength(string text, int wrappingLength, int usedLi
             WrappingLength = wrappingLength
         };
 
+        TextLayoutTestUtilities.TestLayout(text, options, properties: usedLines);
+
         int count = TextMeasurer.CountLines(text, options);
         Assert.Equal(usedLines, count);
-        TextLayoutTestUtilities.TestLayout(text, options, properties: usedLines);
     }
 
     [Fact]
@@ -1217,10 +1217,11 @@ public void BreakWordEnsuresSingleCharacterPerLine()
         };
 
         const string text = "Hello World!";
-        int lineCount = TextMeasurer.CountLines(text, options);
-        Assert.Equal(text.Length - 1, lineCount);
 
         TextLayoutTestUtilities.TestLayout(text, options);
+
+        int lineCount = TextMeasurer.CountLines(text, options);
+        Assert.Equal(text.Length - 1, lineCount);
     }
 
     private class CaptureGlyphBoundBuilder : IGlyphRenderer
