SixLabors / Fonts

:black_nib: Font loading and layout library.
https://sixlabors.com/products/fonts
Other
313 stars 72 forks source link

Issue 367 introduced lingering whitespaces #393

Closed batwomankt closed 8 months ago

batwomankt commented 8 months ago

Discussed in https://github.com/SixLabors/Fonts/discussions/392

Originally posted by **batwomankt** March 15, 2024 Before the fix for issue 367 was merged, if the character at the end of a split for word wrap was whitespace, it would be trimmed. However, with the 367 fix introduced, that particular bit of trimming is missed in `SplitAt` and images have lingering whitespaces at the end of the line that are not trimmed properly. After some fiddling, I have a suggestion on how to have both the desired line split behavior and whitespace trimming. This still passes the 3 line count desired by the opener of issue 367, while also removing any trailing whitespaces from text lines! In the `TextLine` class, add a method something like: ``` private void TrimTrailingWhitespaceAndRecalculateMetrics() { int index = this.data.Count; while (index > 0) { if (!CodePoint.IsWhiteSpace(this.data[index - 1].CodePoint)) { break; } index--; } if (index < this.data.Count) { this.data.RemoveRange(index, this.data.Count - index); } // Lastly recalculate this line metrics. float advance = 0; float ascender = 0; float descender = 0; float lineHeight = 0; foreach (GlyphLayoutData glyph in this.data) { advance += glyph.ScaledAdvance; ascender = MathF.Max(ascender, glyph.ScaledAscender); descender = MathF.Max(descender, glyph.ScaledDescender); lineHeight = MathF.Max(lineHeight, glyph.ScaledLineHeight); } this.ScaledLineAdvance = advance; this.ScaledMaxAscender = ascender; this.ScaledMaxDescender = descender; this.ScaledMaxLineHeight = lineHeight; } ``` and have the `SplitAt` function adjusted to something like: ``` public TextLine SplitAt(LineBreak lineBreak, bool keepAll) { int index = this.data.Count; GlyphLayoutData glyphWrap = default; while (index > 0) { glyphWrap = this.data[--index]; if (glyphWrap.Offset == lineBreak.PositionWrap) { break; } } if (index == 0) { // Now trim trailing whitespace from this line. this.TrimTrailingWhitespaceAndRecalculateMetrics(); return this; } // Word breaks should not be used for Chinese/Japanese/Korean (CJK) text // when word-breaking mode is keep-all. if (keepAll && UnicodeUtility.IsCJKCodePoint((uint)glyphWrap.CodePoint.Value)) { // Loop through previous glyphs to see if there is // a non CJK codepoint we can break at. while (index > 0) { glyphWrap = this.data[--index]; if (!UnicodeUtility.IsCJKCodePoint((uint)glyphWrap.CodePoint.Value)) { index++; break; } } if (index == 0) { return this; } } // Create a new line ensuring we capture the initial metrics. TextLine result = new(); result.data.AddRange(this.data.GetRange(index, this.data.Count - index)); float advance = 0; float ascender = 0; float descender = 0; float lineHeight = 0; for (int i = 0; i < result.data.Count; i++) { GlyphLayoutData glyph = result.data[i]; advance += glyph.ScaledAdvance; ascender = MathF.Max(ascender, glyph.ScaledAscender); descender = MathF.Max(descender, glyph.ScaledDescender); lineHeight = MathF.Max(lineHeight, glyph.ScaledLineHeight); } result.ScaledLineAdvance = advance; result.ScaledMaxAscender = ascender; result.ScaledMaxDescender = descender; result.ScaledMaxLineHeight = lineHeight; // Remove those items from this line. this.data.RemoveRange(index, this.data.Count - index); // Now trim trailing whitespace from this line. this.TrimTrailingWhitespaceAndRecalculateMetrics(); return result; } ``` Desired behavior: ![CircleOverlay Bottom Center Right jpg](https://github.com/SixLabors/Fonts/assets/13904946/88eb736b-d5f8-4127-a3b5-6801e162517d) Current behavior: ![CircleOverlay Bottom Center Right jpg-broken](https://github.com/SixLabors/Fonts/assets/13904946/3c5421f7-3120-4615-aa1b-5510a0564524)
batwomankt commented 8 months ago

Resolved in PR 395