From 201c95a76ea69fb5c6652b657b175db12bcd5e80 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Mon, 11 Jul 2022 23:56:35 +0200
Subject: [PATCH] Handle multiple spaces at row break

In order to work around the Qt row breaking algorithm, which considers
multiple spaces as one at QTextLine break, we insert word_joiner unicode
characters beteween each pair of spaces.

The TextLayoutHelper class makes it easy to handle that.

Update Row::Element::rtrim() to only remove one space at row end.

Update support::countExpanders() to count all spaces, without special
handling for consecutive ones.

Fixes bug #10117.
---
 src/Row.cpp                         | 10 +++++-----
 src/frontends/qt/GuiFontMetrics.cpp | 30 ++++++++++++++---------------
 src/support/lstrings.cpp            | 13 +++----------
 3 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/src/Row.cpp b/src/Row.cpp
index 1c17fb2992..6edde19e75 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -27,6 +27,7 @@
 #include "support/lassert.h"
 #include "support/lstrings.h"
 #include "support/lyxlib.h"
+#include "support/textutils.h"
 
 #include <algorithm>
 #include <ostream>
@@ -203,14 +204,13 @@ bool Row::Element::splitAt(int const width, int next_width, bool force,
 
 void Row::Element::rtrim()
 {
-	if (type != STRING)
+	if (type != STRING || str.empty() || !isSpace(str.back()))
 		return;
 	/* This is intended for strings that have been created by splitAt.
-	 * They may have trailing spaces, but they are not counted in the
-	 * string length (QTextLayout feature, actually). We remove them,
-	 * and decrease endpos, since spaces at row break are invisible.
+	 * If There is a trailing space, we remove it and decrease endpos,
+	 * since spaces at row break are invisible.
 	 */
-	str = support::rtrim(str);
+	str.pop_back();
 	endpos = pos + str.length();
 	dim.wid = nspc_wid;
 }
diff --git a/src/frontends/qt/GuiFontMetrics.cpp b/src/frontends/qt/GuiFontMetrics.cpp
index 7fe067b431..3ee27024eb 100644
--- a/src/frontends/qt/GuiFontMetrics.cpp
+++ b/src/frontends/qt/GuiFontMetrics.cpp
@@ -20,8 +20,8 @@
 #include "support/convert.h"
 #include "support/debug.h"
 #include "support/lassert.h"
-#include "support/lstrings.h" // for breakString_helper with qt4
 #include "support/lyxlib.h"
+#include "support/textutils.h"
 
 #define DISABLE_PMPROF
 #include "support/pmprof.h"
@@ -411,7 +411,13 @@ TextLayoutHelper::TextLayoutHelper(docstring const & s, bool isrtl, bool naked)
 #endif
 
 	// Now translate the string character-by-character.
+	bool was_space = false;
 	for (char_type const c : s) {
+		// insert a word joiner character between consecutive spaces
+		bool const is_space = isSpace(c);
+		if (!naked && is_space && was_space)
+			qstr += word_joiner;
+		was_space = is_space;
 		// Remember the QString index at this point
 		pos2qpos_.push_back(qstr.size());
 		// Performance: UTF-16 characters are easier
@@ -599,27 +605,19 @@ GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid,
 		// If the line is not the last one, trailing space is always omitted.
 		int nspc_wid = wid;
 		// For the last line, compute the width without trailing space
-		if (i + 1 == tl.lineCount()) {
-			// trim_pos points to the last character that is not a space
-			auto trim_pos = s.find_last_not_of(from_ascii(" "));
-			if (trim_pos == docstring::npos)
-				nspc_wid = 0;
-			else if (trim_pos + 1 < s.length()) {
-				int const num_spaces = s.length() - trim_pos - 1;
-				// find the position on the line before trailing
-				// spaces. Remove 1 to account for the ending
-				// non-breaking space of qs.
-				nspc_wid = iround(line.cursorToX(line_epos - num_spaces - 1));
-			}
-		}
+		if (i + 1 == tl.lineCount() && !s.empty() && isSpace(s.back())
+		    && line.textStart() <= tlh.pos2qpos(s.size() - 1))
+			nspc_wid = iround(line.cursorToX(tlh.pos2qpos(s.size() - 1)));
 #else
 		// With some monospace fonts, the value of horizontalAdvance()
 		// can be wrong with Qt4. One hypothesis is that the invisible
 		// characters that we use are given a non-null width.
 		// FIXME: this is slower than it could be but we'll get rid of Qt4 anyway
-		docstring const ss = s.substr(pos, epos - pos);
+		docstring ss = s.substr(pos, epos - pos);
 		int const wid = width(ss);
-		int const nspc_wid = i + 1 < tl.lineCount() ? width(rtrim(ss)) : wid;
+		if (!ss.empty() && isSpace(ss.back()))
+			ss.pop_back();
+		int const nspc_wid = i + 1 < tl.lineCount() ? width(ss) : wid;
 #endif
 		breaks.emplace_back(epos - pos, wid, nspc_wid);
 		pos = epos;
diff --git a/src/support/lstrings.cpp b/src/support/lstrings.cpp
index 77c01151f5..d5b6dea588 100644
--- a/src/support/lstrings.cpp
+++ b/src/support/lstrings.cpp
@@ -1507,18 +1507,11 @@ int countExpanders(docstring const & str)
 	// Numbers of characters that are expanded by inter-word spacing.  These
 	// characters are spaces, except for characters 09-0D which are treated
 	// specially.  (From a combination of testing with the notepad found in qt's
-	// examples, and reading the source code.)  In addition, consecutive spaces
-	// only count as one expander.
-	bool wasspace = false;
+	// examples, and reading the source code.)
 	int nexp = 0;
 	for (char_type c : str)
-		if (c > 0x0d && isSpace(c)) {
-			if (!wasspace) {
-				++nexp;
-				wasspace = true;
-			}
-		} else
-			wasspace = false;
+		if (c > 0x0d && isSpace(c))
+			++nexp;
 	return nexp;
 }