From d794fac44c6281bc0af612169e299cd2abffc04e Mon Sep 17 00:00:00 2001
From: attila <attila@juce.com>
Date: Wed, 8 May 2024 12:50:50 +0200
Subject: [PATCH] SimpleShapedText: Fix line break behaviour with text trailing
 whitespaces

This fixes an issue with the text wrapping logic for text chunks ending
in a whitespace.

When trying to fit a text chunk, the logic works with two values: width
with trailing whitespace, and width without trailing whitespace.

When the trailingWhitespacesShouldFit option is false, the logic
checks if "withoutTrailingWhitespace" can still fit inside the remaining
width.

Prior to this fix, it then decremented the remaining width with
"withoutTrailingWhitespace", but it should have used
"withTrailingWhitespace" for the decrement operation, always, regardless
of the value of the withTrailingWhitespacesShouldFit option.

This mistake only caused an observable issue when multiple fonts were
used for the shaping operation, and a different font would be used
immediately after a whitespace falling at the end of a line.
---
 .../fonts/juce_SimpleShapedText.cpp           | 37 +++++++++++++------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/modules/juce_graphics/fonts/juce_SimpleShapedText.cpp b/modules/juce_graphics/fonts/juce_SimpleShapedText.cpp
index c76421321f..6965f6e58c 100644
--- a/modules/juce_graphics/fonts/juce_SimpleShapedText.cpp
+++ b/modules/juce_graphics/fonts/juce_SimpleShapedText.cpp
@@ -148,6 +148,9 @@ public:
     SimpleShapedText (const String* data,
                       const ShapedTextOptions& options);
 
+    /*  The returned container associates line numbers with the range of glyphs (not input codepoints)
+        that make up the line.
+    */
     const auto& getLineNumbers() const { return lineNumbers; }
 
     const auto& getResolvedFonts() const { return resolvedFonts; }
@@ -615,6 +618,12 @@ struct ShapingParams
     Font resolvedFont;
 };
 
+struct LineAdvance
+{
+    float includingTrailingWhitespace;
+    float maybeIgnoringWhitespace;
+};
+
 struct ConsumableGlyphs
 {
 public:
@@ -683,12 +692,9 @@ public:
     /*  If this function returns a value that also means that it's safe to break before the provided
         codepoint. Otherwise, we couldn't meaningfully calculate the requested value.
     */
-    std::optional<float> getAdvanceXUpToBreakPointIfSafe (int64 breakBefore,
-                                                          bool whitespaceShouldFitInLine) const
+    std::optional<LineAdvance> getAdvanceXUpToBreakPointIfSafe (int64 breakBefore,
+                                                                bool whitespaceShouldFitInLine) const
     {
-        if (breakBefore == range.getEnd())
-            return cumulativeAdvanceX.back();
-
         const auto breakBeforeGlyphIndex = [&]() -> std::optional<size_t>
         {
             if (breakBefore == range.getEnd())
@@ -703,16 +709,18 @@ public:
         if (! breakBeforeGlyphIndex.has_value())
             return std::nullopt;
 
+        const auto includingTrailingWhitespace = cumulativeAdvanceX [*breakBeforeGlyphIndex];
+
         if (! whitespaceShouldFitInLine)
         {
             for (auto i = (int64) *breakBeforeGlyphIndex; --i >= 0;)
             {
                 if (! glyphs[(size_t) i].whitespace)
-                    return cumulativeAdvanceX[(size_t) i + 1];
+                    return LineAdvance { includingTrailingWhitespace, cumulativeAdvanceX[(size_t) i + 1] };
             }
         }
 
-        return cumulativeAdvanceX [*breakBeforeGlyphIndex];
+        return LineAdvance { includingTrailingWhitespace, includingTrailingWhitespace };
     }
 
     auto isEmpty() const
@@ -996,7 +1004,12 @@ void SimpleShapedText::shape (const String& data,
             struct BestMatch
             {
                 int64 breakBefore{};
-                float advance{};
+
+                // We need to use maybeIgnoringWhitespace in comparisions, but
+                // includingTrailingWhitespace when using subtraction to calculate the remaining
+                // space.
+                LineAdvance advance{};
+
                 bool unsafe{};
                 std::vector<ShapedGlyph> unsafeGlyphs;
             };
@@ -1011,7 +1024,7 @@ void SimpleShapedText::shape (const String& data,
                 if (auto safeAdvance = glyphsToConsume.getAdvanceXUpToBreakPointIfSafe (*breakBefore,
                                                                                         options.getTrailingWhitespacesShouldFit()))
                 {
-                    if (*safeAdvance < remainingWidth || ! bestMatch.has_value())
+                    if (safeAdvance->maybeIgnoringWhitespace < remainingWidth || ! bestMatch.has_value())
                         bestMatch = BestMatch { *breakBefore, *safeAdvance, false, std::vector<ShapedGlyph> {} };
                     else
                         break;  // We found a safe break that is too large to fit. Anything beyond
@@ -1048,7 +1061,7 @@ void SimpleShapedText::shape (const String& data,
                                                           [] (auto acc, const auto& elem) { return acc + elem.advance.getX(); });
 
                     if (advance < remainingWidth || ! bestMatch.has_value())
-                        bestMatch = BestMatch { *breakBefore, advance, true, std::move (glyphs) };
+                        bestMatch = BestMatch { *breakBefore, { advance, advance }, true, std::move (glyphs) };
                 }
             }
 
@@ -1086,12 +1099,12 @@ void SimpleShapedText::shape (const String& data,
                 numGlyphsInLine += (int64) glyphs.size();
 
                 if (remainingWidth.has_value())
-                    remainingWidth = *remainingWidth - bestMatch->advance;
+                    remainingWidth = *remainingWidth - bestMatch->advance.includingTrailingWhitespace;
 
                 glyphsToConsume.breakBeforeAndConsume (bestMatch->breakBefore);
             };
 
-            if (bestMatch->advance >= remainingWidth)
+            if (bestMatch->advance.maybeIgnoringWhitespace >= remainingWidth)
             {
                 // Even an empty line is too short to fit any of the text
                 if (numGlyphsInLine == 0 && exactlyEqual (remainingWidth, options.getMaxWidth()))