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

Fix calculation of wrapping advance
SixLabors · Jan 10, 2025 · 24871a8 · 24871a8
2 parents 6de8c5c + 371e95a
commit 24871a8
Show file tree

Hide file tree

Showing 17 changed files with 101 additions and 67 deletions.
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
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
diff --git a/tests/Images/ReferenceOutput/CountLinesWrappingLength_50-5.png b/tests/Images/ReferenceOutput/CountLinesWrappingLength_50-5.png
diff --git a/...dBreakMatchesMDN_238-_layoutMode_HorizontalBottomTop-wordBreaking_BreakAll_.png b/...dBreakMatchesMDN_238-_layoutMode_HorizontalBottomTop-wordBreaking_BreakAll_.png
diff --git a/...dBreakMatchesMDN_238-_layoutMode_HorizontalTopBottom-wordBreaking_BreakAll_.png b/...dBreakMatchesMDN_238-_layoutMode_HorizontalTopBottom-wordBreaking_BreakAll_.png
diff --git a/...ureTextWordBreak_500-_layoutMode_HorizontalBottomTop-wordBreaking_BreakAll_.png b/...ureTextWordBreak_500-_layoutMode_HorizontalBottomTop-wordBreaking_BreakAll_.png
diff --git a/...ureTextWordBreak_500-_layoutMode_HorizontalTopBottom-wordBreaking_BreakAll_.png b/...ureTextWordBreak_500-_layoutMode_HorizontalTopBottom-wordBreaking_BreakAll_.png
diff --git a/...MeasureTextWordWrappingHorizontalBottomTop_350-_height_62.625-width_318.86_.png b/...MeasureTextWordWrappingHorizontalBottomTop_350-_height_62.625-width_318.86_.png
diff --git a/...MeasureTextWordWrappingHorizontalTopBottom_350-_height_62.625-width_318.86_.png b/...MeasureTextWordWrappingHorizontalTopBottom_350-_height_62.625-width_318.86_.png
diff --git a/.../MeasureTextWordWrappingVerticalLeftRight_350-_height_318.563-width_62.813_.png b/.../MeasureTextWordWrappingVerticalLeftRight_350-_height_318.563-width_62.813_.png
diff --git a/...ureTextWordWrappingVerticalMixedLeftRight_350-_height_318.563-width_62.813_.png b/...ureTextWordWrappingVerticalMixedLeftRight_350-_height_318.563-width_62.813_.png
diff --git a/.../MeasureTextWordWrappingVerticalRightLeft_350-_height_318.563-width_62.813_.png b/.../MeasureTextWordWrappingVerticalRightLeft_350-_height_318.563-width_62.813_.png
diff --git a/...mages/ReferenceOutput/ShouldNotInsertExtraLineBreaks_2__WrappingLength_400_.png b/...mages/ReferenceOutput/ShouldNotInsertExtraLineBreaks_2__WrappingLength_400_.png
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