Better handling of trailing spaces in rows.

When a string is broken at the margin by the Qt algorithm, the space
at which breaking occurred is automatically skipped in width
computation. However, the ending space of the string is taken into
account and is visible for example at paragraph end.

When the trailing space is followed by a displayed inset, then the
space should be skipped too, which means that the width of the last
row element has to be recomputed. For the sake of performance, the
width of the element without trailing spaces is computed in advance in
FontMetrics::breakString.

This "no space" width will be used when trimming a row element of its trailing
spaces instead of the original one.

Additionally, do not trim trailing spaces when the row is flushed.

Fixes bug #12449.

src/Row.cpp

@@ -172,6 +172,7 @@ bool Row::Element::splitAt(int const width, int next_width, bool force,
 		e.str = str.substr(i, brk.len);
 		e.endpos = e.pos + brk.len;
 		e.dim.wid = brk.wid;
+		e.nspc_wid = brk.nspc_wid;
 		e.row_flags = CanBreakInside | BreakAfter;
 		if (first) {
 			// this element eventually goes to *this
@@ -218,6 +219,7 @@ void Row::Element::rtrim()
 	 */
 	str = support::rtrim(str);
 	endpos = pos + str.length();
+	dim.wid = nspc_wid;
 }
 
 
@@ -338,7 +340,8 @@ ostream & operator<<(ostream & os, Row const & row)
 	   << " descent: " << row.dim_.des
 	   << " separator: " << row.separator
 	   << " label_hfill: " << row.label_hfill
-	   << " row_boundary: " << row.right_boundary() << "\n";
+	   << " right_boundary: " << row.right_boundary()
+	   << " flushed: " << row.flushed() << "\n";
 	// We cannot use the operator above, unfortunately
 	double x = row.left_margin;
 	for (Row::Element const & e : row.elements_) {

src/Row.h

@@ -119,9 +119,11 @@ public:
 		pos_type pos;
 		// first position after the element in the paragraph
 		pos_type endpos;
-		// The dimension of the chunk (does not contains the
+		// The dimension of the chunk (does not contain the
 		// separator correction)
 		Dimension dim;
+		// The width of the element without trailing spaces
+		int nspc_wid = 0;
 
 		// Non-zero only if element is an inset
 		Inset const * inset = nullptr;

src/TextMetrics.cpp

@@ -1069,7 +1069,7 @@ void cleanupRow(Row & row, bool at_end)
 
 	row.endpos(row.back().endpos);
 	// remove trailing spaces on row break
-	if (!at_end)
+	if (!at_end && !row.flushed())
 		row.back().rtrim();
 	// boundary exists when there was no space at the end of row
 	row.right_boundary(!at_end && row.back().endpos == row.endpos());
@@ -1118,9 +1118,9 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 		                             : fcit->row_flags;
 		if (rows.empty() || needsRowBreak(f1, f2)) {
 			if (!rows.empty()) {
-				cleanupRow(rows.back(), false);
 				// Flush row as requested by row flags
 				rows.back().flushed((f1 & Flush) || (f2 & FlushBefore));
+				cleanupRow(rows.back(), false);
 			}
 			pos_type pos = rows.empty() ? 0 : rows.back().endpos();
 			rows.push_back(newRow(*this, bigrow.pit(), pos, is_rtl));
@@ -1155,9 +1155,9 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 	}
 
 	if (!rows.empty()) {
-		cleanupRow(rows.back(), true);
 		// Last row in paragraph is flushed
 		rows.back().flushed(true);
+		cleanupRow(rows.back(), true);
 	}
 
 	return rows;

src/frontends/FontMetrics.h

@@ -126,9 +126,14 @@ public:
 
 	// The places where to break a string and the width of the resulting lines.
 	struct Break {
-		Break(int l, int w) : len(l), wid(w) {}
+		Break(int l, int w, int nsw) : len(l), wid(w), nspc_wid(nsw) {}
+		// Number of characters
 		int len = 0;
+		// text width
 		int wid = 0;
+		// text width when trailing spaces are removed; only makes a
+		// difference for the last break.
+		int nspc_wid = 0;
 	};
 	typedef std::vector<Break> Breaks;
 	/**

src/frontends/qt/GuiFontMetrics.cpp

@@ -19,7 +19,6 @@
 
 #include "support/convert.h"
 #include "support/lassert.h"
-#include "support/lstrings.h"
 #include "support/lyxlib.h"
 #include "support/debug.h"
 
@@ -579,9 +578,6 @@ GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid,
 	 */
 	to.setWrapMode(force ? QTextOption::WrapAtWordBoundaryOrAnywhere
 	                     : QTextOption::WordWrap);
-	// Let QTextLine::naturalTextWidth() account for trailing spaces
-	// (horizontalAdvance() still does not).
-	to.setFlags(QTextOption::IncludeTrailingSpaces);
 	tl.setTextOption(to);
 
 	bool first = true;
@@ -600,26 +596,43 @@ GuiFontMetrics::breakString_helper(docstring const & s, int first_wid, int wid,
 	int pos = 0;
 	for (int i = 0 ; i < tl.lineCount() ; ++i) {
 		QTextLine const & line = tl.lineAt(i);
-		int const epos = brkstr2str_pos(qs, s, line.textStart() + line.textLength());
+		int const line_epos = line.textStart() + line.textLength();
+		int const epos = brkstr2str_pos(qs, s, line_epos);
 #if QT_VERSION >= 0x050000
-		int const wid = i + 1 < tl.lineCount() ? iround(line.horizontalAdvance())
-		                                       : iround(line.naturalTextWidth());
+		// This does not take trailing spaces into account, except for the last line.
+		int const wid = iround(line.naturalTextWidth());
+		// 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));
+			}
+		}
 #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
-		int const wid = i + 1 < tl.lineCount() ? width(rtrim(s.substr(pos, epos - pos)))
-		                                       : width(s.substr(pos, epos - pos));
+		docstring const ss = s.substr(pos, epos - pos);
+		int const wid = width(ss);
+		int const nspc_wid = i + 1 < tl.lineCount() ? wid : width(trim(ss));
 #endif
-		breaks.emplace_back(epos - pos, wid);
+		breaks.emplace_back(epos - pos, wid, nspc_wid);
 		pos = epos;
 #if 0
 		// FIXME: should it be kept in some form?
 		if ((force && line.textLength() == brkStrOffset) || line_wid > x)
 			return {-1, line_wid};
 #endif
-
 	}
 
 	return breaks;