Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix newline handling and whitespace trimming #442

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix newline handling and whitespace trimming #442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/SixLabors.Fonts/GlyphMetrics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,8 @@ void SetDecoration(TextDecorations decorations, float thickness, float position)
/// <returns>The <see cref="bool"/>.</returns>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
protected internal static bool ShouldSkipGlyphRendering(CodePoint codePoint)
=> UnicodeUtility.IsDefaultIgnorableCodePoint((uint)codePoint.Value) && !UnicodeUtility.ShouldRenderWhiteSpaceOnly(codePoint);
=> CodePoint.IsNewLine(codePoint) ||
(UnicodeUtility.IsDefaultIgnorableCodePoint((uint)codePoint.Value) && !UnicodeUtility.ShouldRenderWhiteSpaceOnly(codePoint));

/// <summary>
/// Returns the size to render/measure the glyph based on the given size and resolution in px units.
Expand Down
87 changes: 72 additions & 15 deletions src/SixLabors.Fonts/TextLayout.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,12 +391,23 @@ private static IEnumerable<GlyphLayout> LayoutLineHorizontal(
TextLine.GlyphLayoutData data = textLine[i];
if (data.IsNewLine)
{
glyphs.Add(new GlyphLayout(
new Glyph(data.Metrics[0], data.PointSize),
boxLocation,
penLocation,
Vector2.Zero,
data.ScaledAdvance,
yLineAdvance,
GlyphLayoutMode.Horizontal,
true,
data.GraphemeIndex,
data.StringIndex));

penLocation.X = originX;
penLocation.Y += yLineAdvance;

boxLocation.X = originX;
boxLocation.Y += advanceY;
continue;
goto end;
}

int j = 0;
Expand Down Expand Up @@ -429,6 +440,7 @@ private static IEnumerable<GlyphLayout> LayoutLineHorizontal(
boxLocation.Y += advanceY;
}

end:
return glyphs;
}

Expand Down Expand Up @@ -524,12 +536,23 @@ private static IEnumerable<GlyphLayout> LayoutLineVertical(
TextLine.GlyphLayoutData data = textLine[i];
if (data.IsNewLine)
{
glyphs.Add(new GlyphLayout(
new Glyph(data.Metrics[0], data.PointSize),
boxLocation,
penLocation,
Vector2.Zero,
xLineAdvance,
data.ScaledAdvance,
GlyphLayoutMode.Vertical,
true,
data.GraphemeIndex,
data.StringIndex));

boxLocation.X += advanceX;
boxLocation.Y = originY;

penLocation.X += xLineAdvance;
penLocation.Y = originY;
continue;
goto end;
JimBobSquarePants marked this conversation as resolved.
Show resolved Hide resolved
}

int j = 0;
Expand Down Expand Up @@ -576,6 +599,7 @@ private static IEnumerable<GlyphLayout> LayoutLineVertical(
penLocation.X += xLineAdvance;
}

end:
return glyphs;
}

Expand Down Expand Up @@ -671,12 +695,23 @@ private static IEnumerable<GlyphLayout> LayoutLineVerticalMixed(
TextLine.GlyphLayoutData data = textLine[i];
if (data.IsNewLine)
{
glyphs.Add(new GlyphLayout(
new Glyph(data.Metrics[0], data.PointSize),
boxLocation,
penLocation,
Vector2.Zero,
xLineAdvance,
data.ScaledAdvance,
GlyphLayoutMode.Vertical,
true,
data.GraphemeIndex,
data.StringIndex));

boxLocation.X += advanceX;
boxLocation.Y = originY;

penLocation.X += xLineAdvance;
penLocation.Y = originY;
continue;
goto end;
JimBobSquarePants marked this conversation as resolved.
Show resolved Hide resolved
}

if (data.IsTransformed)
Expand Down Expand Up @@ -752,6 +787,7 @@ private static IEnumerable<GlyphLayout> LayoutLineVerticalMixed(
penLocation.X += xLineAdvance;
}

end:
return glyphs;
}

Expand Down Expand Up @@ -1170,10 +1206,30 @@ VerticalOrientationType.Rotate or
{
// Mandatory line break at index.
TextLine remaining = textLine.SplitAt(i);
textLines.Add(textLine.Finalize(options));
textLine = remaining;
i = -1;
lineAdvance = 0;

if (shouldWrap && textLine.ScaledLineAdvance - glyph.ScaledAdvance > wrappingLength)
{
// We've overshot the wrapping length so we need to split the line
// at the previous break and add both lines.
TextLine overflow = textLine.SplitAt(lastLineBreak, keepAll);
if (overflow != textLine)
{
textLines.Add(textLine.Finalize(options));
textLine = overflow;
}

textLines.Add(textLine.Finalize(options));
textLine = remaining;
i = -1;
lineAdvance = 0;
}
else
{
textLines.Add(textLine.Finalize(options));
textLine = remaining;
i = -1;
lineAdvance = 0;
}
}
else if (shouldWrap)
{
Expand Down Expand Up @@ -1201,7 +1257,7 @@ VerticalOrientationType.Rotate or
{
// 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 previousAdvance = lineAdvance - (float)glyph.ScaledAdvance;
float previousAdvance = lineAdvance - glyph.ScaledAdvance;
TextLine.GlyphLayoutData lastGlyph = textLine[i - 1];
if (CodePoint.IsWhiteSpace(lastGlyph.CodePoint))
{
Expand Down Expand Up @@ -1463,8 +1519,9 @@ public TextLine SplitAt(LineBreak lineBreak, bool keepAll)

private void TrimTrailingWhitespace()
{
int index = this.data.Count;
while (index > 0)
int count = this.data.Count;
int index = count;
while (index > 1)
{
// Trim trailing breaking whitespace.
CodePoint point = this.data[index - 1].CodePoint;
Expand All @@ -1476,9 +1533,9 @@ private void TrimTrailingWhitespace()
index--;
}

if (index < this.data.Count && index != 0)
if (index < count)
{
this.data.RemoveRange(index, this.data.Count - index);
this.data.RemoveRange(index, count - index);
}
}

Expand Down
3 changes: 3 additions & 0 deletions ...Images/ReferenceOutput/LineWrappingWithExplicitNewLine__WrappingLength_800_.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions ...Images/ReferenceOutput/LineWrappingWithImplicitNewLine__WrappingLength_800_.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions ...Images/ReferenceOutput/RenderingTextIncludesAllGlyphs__WrappingLength_1900_.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
0 ...ut/ShouldInsertExtraLineBreaksA_400-4.png → ...ut/ShouldInsertExtraLineBreaksA_400-5.png
View file
Open in desktop
File renamed without changes
0 ...ut/ShouldInsertExtraLineBreaksB_400-4.png → ...ut/ShouldInsertExtraLineBreaksB_400-6.png
View file
Open in desktop
File renamed without changes
2 changes: 1 addition & 1 deletion tests/SixLabors.Fonts.Tests/Issues/Issues_27.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public void ThrowsMeasuringWhitespace()
Font font = new FontCollection().Add(TestFonts.WendyOneFile).CreateFont(12);
FontRectangle size = TextMeasurer.MeasureBounds(" ", new TextOptions(new Font(font, 30)));

Assert.Equal(60, size.Width, 1F);
Assert.Equal(6, size.Width, 1F);
Assert.Equal(0, size.Height, 1F);
}
}
4 changes: 2 additions & 2 deletions tests/SixLabors.Fonts.Tests/Issues/Issues_33.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ namespace SixLabors.Fonts.Tests.Issues;
public class Issues_33
{
[Theory]
[InlineData("\naaaabbbbccccddddeeee\n\t\t\t3 tabs\n\t\t\t\t\t5 tabs", 580, 70)] // newlines aren't directly measured but it is used for offsetting
[InlineData("\n\tHelloworld", 310, 10)]
[InlineData("\naaaabbbbccccddddeeee\n\t\t\t3 tabs\n\t\t\t\t\t5 tabs", 580, 120)]
[InlineData("\n\tHelloworld", 310, 60)]
[InlineData("\tHelloworld", 310, 10)]
[InlineData(" Helloworld", 340, 10)]
[InlineData("Hell owor ld\t", 340, 10)]
Expand Down
40 changes: 10 additions & 30 deletions tests/SixLabors.Fonts.Tests/Issues/Issues_35.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,10 @@ public class Issues_35
public void RenderingTabAtStartOrLineTooShort()
{
Font font = CreateFont("\t x");
FontRectangle xWidth = TextMeasurer.MeasureBounds("x", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabWidth = TextMeasurer.MeasureBounds("\t", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabWithXWidth = TextMeasurer.MeasureBounds("\tx", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });

Assert.Equal(tabWidth.Width + xWidth.Width, tabWithXWidth.Width, 2F);
}

[Fact]
public void Rendering2TabsAtStartOfLineTooShort()
{
Font font = CreateFont("\t x");
FontRectangle xWidth = TextMeasurer.MeasureBounds("x", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabWidth = TextMeasurer.MeasureBounds("\t\t", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabWithXWidth = TextMeasurer.MeasureBounds("\t\tx", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
TextOptions options = new(font) { Dpi = font.FontMetrics.ScaleFactor };
FontRectangle xWidth = TextMeasurer.MeasureBounds("x", options);
FontRectangle tabWidth = TextMeasurer.MeasureBounds("\t", options);
FontRectangle tabWithXWidth = TextMeasurer.MeasureBounds("\tx", options);

Assert.Equal(tabWidth.Width + xWidth.Width, tabWithXWidth.Width, 2F);
}
Expand All @@ -34,27 +24,17 @@ public void Rendering2TabsAtStartOfLineTooShort()
public void TwoTabsAreDoubleWidthOfOneTab()
{
Font font = CreateFont("\t x");
FontRectangle xWidth = TextMeasurer.MeasureBounds("x", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabWidth = TextMeasurer.MeasureBounds("\t", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle twoTabWidth = TextMeasurer.MeasureBounds("\t\t", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });

Assert.Equal(twoTabWidth.Width, tabWidth.Width * 2, 2F);
}

[Fact]
public void TwoTabsAreDoubleWidthOfOneTabMinusXWidth()
{
Font font = CreateFont("\t x");
FontRectangle xWidth = TextMeasurer.MeasureBounds("x", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabWidth = TextMeasurer.MeasureBounds("\tx", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle twoTabWidth = TextMeasurer.MeasureBounds("\t\tx", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
TextOptions options = new(font) { Dpi = font.FontMetrics.ScaleFactor };
FontRectangle xWidth = TextMeasurer.MeasureBounds("x", options);
FontRectangle tabWithXWidth = TextMeasurer.MeasureBounds("\tx", options);
FontRectangle tabTabWithXWidth = TextMeasurer.MeasureBounds("\t\tx", options);

Assert.Equal(twoTabWidth.Width - xWidth.Width, (tabWidth.Width - xWidth.Width) * 2, 2F);
Assert.Equal(tabTabWithXWidth.Width - xWidth.Width, 2 * (tabWithXWidth.Width - xWidth.Width), 2F);
}

public static Font CreateFont(string text)
{
var fc = (IFontMetricsCollection)new FontCollection();
IFontMetricsCollection fc = new FontCollection();
Font d = fc.AddMetrics(new FakeFontInstance(text), CultureInfo.InvariantCulture).CreateFont(12);
return new Font(d, 1F);
}
Expand Down
20 changes: 11 additions & 9 deletions tests/SixLabors.Fonts.Tests/Issues/Issues_36.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@ public class Issues_36
[InlineData(3)]
[InlineData(4)]
[InlineData(5)]
public void TextWidthForTabOnlyTextShouldBeSingleTabWidthMultipliedByTabCount(int tabCount)
public void TextWidthForTabOnlyTextShouldBeSingleTabWidth(int tabCount)
{
Font font = CreateFont("\t x");
Font font = CreateFont("\t");
TextOptions options = new(font) { Dpi = font.FontMetrics.ScaleFactor };

FontRectangle tabWidth = TextMeasurer.MeasureBounds("\t", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabWidth = TextMeasurer.MeasureBounds("\t", options);
string tabString = string.Empty.PadRight(tabCount, '\t');
FontRectangle tabCountWidth = TextMeasurer.MeasureBounds(tabString, new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabCountWidth = TextMeasurer.MeasureBounds(tabString, options);

Assert.Equal(tabWidth.Width * tabCount, tabCountWidth.Width, 2F);
Assert.Equal(tabWidth.Width, tabCountWidth.Width, 2F);
}

[Theory]
Expand All @@ -35,10 +36,11 @@ public void TextWidthForTabOnlyTextShouldBeSingleTabWidthMultipliedByTabCountMin
{
Font font = CreateFont("\t x");

FontRectangle xWidth = TextMeasurer.MeasureBounds("x", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabWidth = TextMeasurer.MeasureBounds("\tx", new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
TextOptions options = new(font) { Dpi = font.FontMetrics.ScaleFactor };
FontRectangle xWidth = TextMeasurer.MeasureBounds("x", options);
FontRectangle tabWidth = TextMeasurer.MeasureBounds("\tx", options);
string tabString = "x".PadLeft(tabCount + 1, '\t');
FontRectangle tabCountWidth = TextMeasurer.MeasureBounds(tabString, new TextOptions(font) { Dpi = font.FontMetrics.ScaleFactor });
FontRectangle tabCountWidth = TextMeasurer.MeasureBounds(tabString, options);

float singleTabWidth = tabWidth.Width - xWidth.Width;
float finalTabWidth = tabCountWidth.Width - xWidth.Width;
Expand All @@ -47,7 +49,7 @@ public void TextWidthForTabOnlyTextShouldBeSingleTabWidthMultipliedByTabCountMin

public static Font CreateFont(string text)
{
var fc = (IFontMetricsCollection)new FontCollection();
IFontMetricsCollection fc = new FontCollection();
Font d = fc.AddMetrics(new FakeFontInstance(text), CultureInfo.InvariantCulture).CreateFont(12);
return new Font(d, 1F);
}
Expand Down
4 changes: 3 additions & 1 deletion tests/SixLabors.Fonts.Tests/Issues/Issues_400.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ public void RenderingTextIncludesAllGlyphs()
.AppendLine(" NEWS_CATEGORY=EWF&NEWS_HASH=4b298ff9277ef9fdf515356be95ea3caf57cd36&OFFSET=0&SEARCH_VALUE=CA88105E1088&ID_NEWS")
.Append(" ");

TextLayoutTestUtilities.TestLayout(stringBuilder.ToString(), options);

int lineCount = TextMeasurer.CountLines(stringBuilder.ToString(), options);
Assert.Equal(2, lineCount);
Assert.Equal(4, lineCount);
#endif
}
}
20 changes: 8 additions & 12 deletions tests/SixLabors.Fonts.Tests/Issues/Issues_434.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace SixLabors.Fonts.Tests.Issues;
public class Issues_434
{
[Theory]
[InlineData("- Lorem ipsullll\n\ndolor sit amet\n-consectetur elit", 4)]
[InlineData("- Lorem ipsullll\n\ndolor sit amet\n-consectetur elit", 5)]
public void ShouldInsertExtraLineBreaksA(string text, int expectedLineCount)
{
if (SystemFonts.TryGet("Arial", out FontFamily family))
Expand All @@ -20,20 +20,18 @@ public void ShouldInsertExtraLineBreaksA(string text, int expectedLineCount)
WrappingLength = 400,
};

// Line count includes rendered lines only.
// Line breaks cause offsetting of subsequent lines.
TextLayoutTestUtilities.TestLayout(text, options, properties: expectedLineCount);

int lineCount = TextMeasurer.CountLines(text, options);
Assert.Equal(expectedLineCount, lineCount);

IReadOnlyList<GlyphLayout> layout = TextLayout.GenerateLayout(text, options);
Assert.Equal(46, layout.Count);

TextLayoutTestUtilities.TestLayout(text, options, properties: expectedLineCount);
Assert.Equal(47, layout.Count);
}
}

[Theory]
[InlineData("- Lorem ipsullll\n\n\ndolor sit amet\n-consectetur elit", 4)]
[InlineData("- Lorem ipsullll\n\n\ndolor sit amet\n-consectetur elit", 6)]
public void ShouldInsertExtraLineBreaksB(string text, int expectedLineCount)
{
if (SystemFonts.TryGet("Arial", out FontFamily family))
Expand All @@ -45,15 +43,13 @@ public void ShouldInsertExtraLineBreaksB(string text, int expectedLineCount)
WrappingLength = 400,
};

// Line count includes rendered lines only.
// Line breaks cause offsetting of subsequent lines.
TextLayoutTestUtilities.TestLayout(text, options, properties: expectedLineCount);

int lineCount = TextMeasurer.CountLines(text, options);
Assert.Equal(expectedLineCount, lineCount);

IReadOnlyList<GlyphLayout> layout = TextLayout.GenerateLayout(text, options);
Assert.Equal(46, layout.Count);

TextLayoutTestUtilities.TestLayout(text, options, properties: expectedLineCount);
Assert.Equal(48, layout.Count);
}
}
}
Loading
Loading