[LyX features/breakrows] Change Row::Element::splitAt to return a vector of Element

Jean-Marc Lasgouttes lasgouttes at lyx.org
Tue Sep 7 13:50:21 UTC 2021


The branch, breakrows, has been updated.
  discards  5bdc309b1feb6eb9e28914ff0359c3dc78d2bb92 (commit)
  discards  d0e51b0c0812437e69d779f7eaaa2769972b8605 (commit)
  discards  df3e3a065664ac367d3342e19f3468dc31264985 (commit)
  discards  96465d1b778824836179257319945c6b3c4ebdea (commit)
  discards  38798259ed68ff4f8046da517f02d85a335dcbed (commit)
  discards  fb81252e525af49c40198d8407abf909deeb4304 (commit)
  discards  c9de77fee897cca334928ea1287ce9cd522fb706 (commit)
  discards  583b7d98623e284733cbfe53e5e9d07352701d4e (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (5bdc309b1feb6eb9e28914ff0359c3dc78d2bb92)
            \
             N -- N -- N (eec0ec60dd5bec229f3128f956ae937bdc441735)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

- Log -----------------------------------------------------------------

commit eec0ec60dd5bec229f3128f956ae937bdc441735
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Mon Sep 6 14:52:42 2021 +0200

    Change Row::Element::splitAt to return a vector of Element
    
    This is in preparation of splitAt splitting in more than two strings.
    For now, it returns a vector which contains 0 or 1 element, but the
    code that uses it is prepared for a larger vector.
    
    Rename the helper function splitFrom to moveElements and rewrite the
    code to be more efficient.
    
    Remove type of row element INVALID, which is not needed anymore.
    
    The code in TextMetrics::breakParagraph is now much simpler.

diff --git a/src/Row.cpp b/src/Row.cpp
index 8233a96..093f280 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -123,41 +123,45 @@ pos_type Row::Element::x2pos(int &x) const
 			x = 0;
 			i = isRTL();
 		}
-		break;
-	case INVALID:
-		LYXERR0("x2pos: INVALID row element !");
 	}
 	//lyxerr << "=> p=" << pos + i << " x=" << x << endl;
 	return pos + i;
 }
 
 
-Row::Element Row::Element::splitAt(int w, bool force)
+Row::Elements Row::Element::splitAt(int w, bool force)
 {
 	if (type != STRING || !(row_flags & CanBreakInside))
-		return Element();
+		return Elements();
 
 	FontMetrics const & fm = theFontMetrics(font);
 	dim.wid = w;
 	int const i = fm.breakAt(str, dim.wid, isRTL(), force);
-	if (i != -1) {
-		//Create a second row element to return
+
+	// No need to break
+	if (i == -1)
+		return Elements();
+
+	Elements tail;
+	// Is there a second element to return?
+	if (i < int(str.length())) {
 		Element ret(STRING, pos + i, font, change);
 		ret.str = str.substr(i);
 		ret.endpos = ret.pos + ret.str.length();
 		// Copy the after flags of the original element to the second one.
 		ret.row_flags = row_flags & (CanBreakInside | AfterFlags);
+		tail.push_back(ret);
 
 		// Now update ourselves
 		str.erase(i);
 		endpos = pos + i;
-		// Row should be broken after the original element
-		row_flags = (row_flags & ~AfterFlags) | BreakAfter;
-		//LYXERR0("breakAt(" << w << ")  Row element Broken at " << w << "(w(str)=" << fm.width(str) << "): e=" << *this);
-		return ret;
 	}
 
-	return Element();
+	// Row should be broken after the original element
+	row_flags = (row_flags & ~AfterFlags) | BreakAfter;
+	//LYXERR0("splitAt(" << w << ")  Row element Broken at " << w << "(w(str)=" << fm.width(str) << "): e=" << *this);
+
+	return tail;
 }
 
 
@@ -265,10 +269,6 @@ ostream & operator<<(ostream & os, Row::Element const & e)
 		break;
 	case Row::SPACE:
 		os << "SPACE: ";
-		break;
-	case Row::INVALID:
-		os << "INVALID: ";
-		break;
 	}
 	os << "width=" << e.full_width() << ", row_flags=" << e.row_flags;
 	return os;
@@ -463,19 +463,14 @@ void Row::pop_back()
 
 namespace {
 
-// Remove stuff after \c it from \c elts, and return it.
-// if \c init is provided, it will prepended to the rest
-Row::Elements splitFrom(Row::Elements & elts, Row::Elements::iterator const & it,
-                        Row::Element const & init = Row::Element())
+// Move stuff after \c it from \c from and the end of \c to.
+void moveElements(Row::Elements & from, Row::Elements::iterator const & it,
+                  Row::Elements & to)
 {
-	Row::Elements ret;
-	if (init.isValid())
-		ret.push_back(init);
-	ret.insert(ret.end(), it, elts.end());
-	elts.erase(it, elts.end());
-	if (!elts.empty())
-		elts.back().row_flags = (elts.back().row_flags & ~AfterFlags) | BreakAfter;
-	return ret;
+	to.insert(to.end(), it, from.end());
+	from.erase(it, from.end());
+	if (!from.empty())
+		from.back().row_flags = (from.back().row_flags & ~AfterFlags) | BreakAfter;
 }
 
 }
@@ -508,6 +503,7 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 	}
 
 	// Iterate backwards over breakable elements and try to break them
+	Elements tail;
 	Elements::iterator cit_brk = cit;
 	int wid_brk = wid + cit_brk->dim.wid;
 	++cit_brk;
@@ -522,7 +518,8 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 		if (wid_brk <= w && brk.row_flags & CanBreakAfter) {
 			end_ = brk.endpos;
 			dim_.wid = wid_brk;
-			return splitFrom(elements_, cit_brk + 1);
+			moveElements(elements_, cit_brk + 1, tail);
+			return tail;
 		}
 		// assume now that the current element is not there
 		wid_brk -= brk.dim.wid;
@@ -542,7 +539,7 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 		 * - shorter than the natural width of the element, in order to enforce
 		 *   break-up.
 		 */
-		Element remainder = brk.splitAt(min(w - wid_brk, brk.dim.wid - 2), !word_wrap);
+		tail = brk.splitAt(min(w - wid_brk, brk.dim.wid - 2), !word_wrap);
 		if (brk.row_flags & BreakAfter) {
 			/* if this element originally did not cause a row overflow
 			 * in itself, and the remainder of the row would still be
@@ -557,11 +554,8 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 			*cit_brk = brk;
 			dim_.wid = wid_brk + brk.dim.wid;
 			// If there are other elements, they should be removed.
-			// remainder can be empty when splitting at trailing space
-			if (remainder.str.empty())
-				return splitFrom(elements_, next(cit_brk, 1));
-			else
-				return splitFrom(elements_, next(cit_brk, 1), remainder);
+			moveElements(elements_, cit_brk + 1, tail);
+			return tail;
 		}
 	}
 
@@ -577,20 +571,23 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 		// been added. We can cut right here.
 		end_ = cit->pos;
 		dim_.wid = wid;
-		return splitFrom(elements_, cit);
+		moveElements(elements_, cit, tail);
+		return tail;
 	}
 
 	/* If we are here, it means that we have not found a separator to
 	 * shorten the row. Let's try to break it again, but not at word
 	 * boundary this time.
 	 */
-	Element remainder = cit->splitAt(w - wid, true);
+	tail = cit->splitAt(w - wid, true);
 	if (cit->row_flags & BreakAfter) {
 		end_ = cit->endpos;
 		dim_.wid = wid + cit->dim.wid;
 		// If there are other elements, they should be removed.
-		return splitFrom(elements_, next(cit, 1), remainder);
+		moveElements(elements_, cit + 1, tail);
+		return tail;
 	}
+
 	return Elements();
 }
 
diff --git a/src/Row.h b/src/Row.h
index 7466797..552c2fe 100644
--- a/src/Row.h
+++ b/src/Row.h
@@ -50,9 +50,7 @@ public:
 		// An inset
 		INSET,
 		// Some spacing described by its width, not a string
-		SPACE,
-		// Something that should not happen (for error handling)
-		INVALID
+		SPACE
 	};
 
 /**
@@ -61,8 +59,6 @@ public:
  */
 	struct Element {
 		//
-		Element() = default;
-		//
 		Element(Type const t, pos_type p, Font const & f, Change const & ch)
 			: type(t), pos(p), endpos(p + 1), font(f), change(ch) {}
 
@@ -94,13 +90,14 @@ public:
 		pos_type x2pos(int &x) const;
 		/** Break the element in two if possible, so that its width is less
 		 * than \param w.
-		 * \return an element containing the remainder of the text, or
-		 *   an invalid element if nothing happened.
+		 * \return a vector of elements containing the remainder of
+		 *   the text (empty if nothing happened).
 		 * \param w: the desired maximum width
 		 * \param force: if true, the string is cut at any place, otherwise it
 		 *   respects the row breaking rules of characters.
 		 */
-		Element splitAt(int w, bool force);
+		// FIXME: ideally return type should be Elements, but it is not possible.
+		std::vector<Element> splitAt(int w, bool force);
 		// remove trailing spaces (useful for end of row)
 		void rtrim();
 
@@ -108,8 +105,6 @@ public:
 		bool isRTL() const { return font.isVisibleRightToLeft(); }
 		// This is true for virtual elements.
 		bool isVirtual() const { return type == VIRTUAL; }
-		// Invalid element, for error handling
-		bool isValid() const { return type !=INVALID; }
 
 		// Returns the position on left side of the element.
 		pos_type left_pos() const { return isRTL() ? endpos : pos; };
@@ -117,11 +112,11 @@ public:
 		pos_type right_pos() const { return isRTL() ? pos : endpos; };
 
 		// The kind of row element
-		Type type = INVALID;
+		Type type;
 		// position of the element in the paragraph
-		pos_type pos = 0;
+		pos_type pos;
 		// first position after the element in the paragraph
-		pos_type endpos = 0;
+		pos_type endpos;
 		// The dimension of the chunk (does not contains the
 		// separator correction)
 		Dimension dim;
diff --git a/src/RowPainter.cpp b/src/RowPainter.cpp
index 656f89a..400b7b6 100644
--- a/src/RowPainter.cpp
+++ b/src/RowPainter.cpp
@@ -565,10 +565,6 @@ void RowPainter::paintText()
 
 		case Row::SPACE:
 			paintTextDecoration(e);
-			break;
-
-		case Row::INVALID:
-			LYXERR0("Trying to paint INVALID row element.");
 		}
 
 		// The markings of foreign languages
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index f055074..f70f11a 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1058,7 +1058,7 @@ Row newRow(TextMetrics const & tm, pit_type pit, pos_type pos, bool is_rtl)
 }
 
 
-void cleanupRow(Row & row, pos_type real_endpos, bool is_rtl)
+void cleanupRow(Row & row, bool at_end, bool is_rtl)
 {
 	if (row.empty()) {
 		row.endpos(0);
@@ -1066,11 +1066,12 @@ void cleanupRow(Row & row, pos_type real_endpos, bool is_rtl)
 	}
 
 	row.endpos(row.back().endpos);
+	row.flushed(at_end);
 	// remove trailing spaces on row break
-	if (row.endpos() < real_endpos)
+	if (!at_end)
 		row.back().rtrim();
 	// boundary exists when there was no space at the end of row
-	row.right_boundary(row.endpos() < real_endpos && row.back().endpos == row.endpos());
+	row.right_boundary(!at_end && row.back().endpos == row.endpos());
 	// make sure that the RTL elements are in reverse ordering
 	row.reverseRTL(is_rtl);
 }
@@ -1114,7 +1115,7 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 		                             : fcit->row_flags;
 		if (rows.empty() || needsRowBreak(f1, f2)) {
 			if (!rows.empty())
-				cleanupRow(rows.back(), bigrow.endpos(), is_rtl);
+				cleanupRow(rows.back(), false, is_rtl);
 			pos_type pos = rows.empty() ? 0 : rows.back().endpos();
 			rows.push_back(newRow(*this, bigrow.pit(), pos, is_rtl));
 			// the width available for the row.
@@ -1129,45 +1130,30 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 		// Next element to consider is either the top of the temporary
 		// pile, or the place when we were in main row
 		Row::Element elt = *fcit;
-		Row::Element next_elt = elt.splitAt(width - rows.back().width(),
-		                                    !elt.font.language()->wordWrap());
-		if (elt.dim.wid > width - rows.back().width()) {
-			Row & rb = rows.back();
-			rb.push_back(*fcit);
+		Row::Elements tail = elt.splitAt(width - rows.back().width(),
+		                                 !elt.font.language()->wordWrap());
+		Row & rb = rows.back();
+		rb.push_back(elt);
+		rb.finalizeLast();
+		if (rows.back().width() > width) {
+			LATTEST(tail.empty());
 			// if the row is too large, try to cut at last separator. In case
 			// of success, reset indication that the row was broken abruptly.
 			int const next_width = max_width_ - leftMargin(rb.pit(), rb.endpos())
 				- rightMargin(rb.pit());
 
-			Row::Elements next_elts = rb.shortenIfNeeded(width, next_width);
+			tail = rb.shortenIfNeeded(width, next_width);
+		}
 
-			// Go to next element
-			++fcit;
+		// Go to next element
+		++fcit;
 
-			// Handle later the elements returned by shortenIfNeeded.
-			if (!next_elts.empty()) {
-				rb.flushed(false);
-				fcit.put(next_elts);
-			}
-		} else {
-			// a new element in the row
-			rows.back().push_back(elt);
-			rows.back().finalizeLast();
-
-			// Go to next element
-			++fcit;
-
-			// Add a new next element on the pile
-			if (next_elt.isValid()) {
-				// do as if we inserted this element in the original row
-				if (!next_elt.str.empty())
-					fcit.put(next_elt);
-			}
-		}
+		// Handle later the elements returned by splitAt or shortenIfNeeded.
+		fcit.put(tail);
 	}
 
 	if (!rows.empty()) {
-		cleanupRow(rows.back(), bigrow.endpos(), is_rtl);
+		cleanupRow(rows.back(), true, is_rtl);
 		// Last row in paragraph is flushed
 		rows.back().flushed(true);
 	}

commit 92d6917a897c2668c91f373dde2e3f85fa1a3b3d
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Thu Sep 2 15:16:28 2021 +0200

    Fix setting of row pos/endpos (overlapping rows)
    
    In TextMetrics::breakParagraph, get rid of the fragile `pos' local
    variable, which was not correctly updated. Rely on the endpos of the
    last element in row instead.
    
    Rewrite cleanupRow to rely on the endpos of last the row element to
    set row endpos, instead of a `pos' parameter.

diff --git a/src/Row.cpp b/src/Row.cpp
index 7b74694..8233a96 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -463,8 +463,8 @@ void Row::pop_back()
 
 namespace {
 
-// Remove stuff after it from elts, and return it.
-// if init is provided, it will be in front of the rest
+// Remove stuff after \c it from \c elts, and return it.
+// if \c init is provided, it will prepended to the rest
 Row::Elements splitFrom(Row::Elements & elts, Row::Elements::iterator const & it,
                         Row::Element const & init = Row::Element())
 {
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 00ebb55..f055074 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1058,15 +1058,19 @@ Row newRow(TextMetrics const & tm, pit_type pit, pos_type pos, bool is_rtl)
 }
 
 
-void cleanupRow(Row & row, pos_type pos, pos_type real_endpos, bool is_rtl)
+void cleanupRow(Row & row, pos_type real_endpos, bool is_rtl)
 {
-	row.endpos(pos);
+	if (row.empty()) {
+		row.endpos(0);
+		return;
+	}
+
+	row.endpos(row.back().endpos);
 	// remove trailing spaces on row break
-	if (pos < real_endpos && !row.empty())
+	if (row.endpos() < real_endpos)
 		row.back().rtrim();
 	// boundary exists when there was no space at the end of row
-	row.right_boundary(!row.empty() && pos < real_endpos
-	                   && row.back().endpos == pos);
+	row.right_boundary(row.endpos() < real_endpos && row.back().endpos == row.endpos());
 	// make sure that the RTL elements are in reverse ordering
 	row.reverseRTL(is_rtl);
 }
@@ -1094,7 +1098,6 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 	bool const is_rtl = text_->isRTL(bigrow.pit());
 	bool const end_label = text_->getEndLabel(bigrow.pit()) != END_LABEL_NO_LABEL;
 
-	pos_type pos = 0;
 	int width = 0;
 	flexible_const_iterator<Row> fcit = flexible_begin(bigrow);
 	flexible_const_iterator<Row> const end = flexible_end(bigrow);
@@ -1111,7 +1114,8 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 		                             : fcit->row_flags;
 		if (rows.empty() || needsRowBreak(f1, f2)) {
 			if (!rows.empty())
-				cleanupRow(rows.back(), pos, bigrow.endpos(), is_rtl);
+				cleanupRow(rows.back(), bigrow.endpos(), is_rtl);
+			pos_type pos = rows.empty() ? 0 : rows.back().endpos();
 			rows.push_back(newRow(*this, bigrow.pit(), pos, is_rtl));
 			// the width available for the row.
 			width = max_width_ - rows.back().right_margin;
@@ -1149,7 +1153,6 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 			// a new element in the row
 			rows.back().push_back(elt);
 			rows.back().finalizeLast();
-			pos = elt.endpos;
 
 			// Go to next element
 			++fcit;
@@ -1164,7 +1167,7 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 	}
 
 	if (!rows.empty()) {
-		cleanupRow(rows.back(), pos, bigrow.endpos(), is_rtl);
+		cleanupRow(rows.back(), bigrow.endpos(), is_rtl);
 		// Last row in paragraph is flushed
 		rows.back().flushed(true);
 	}

commit 8ab910586808e075f1ec101d1b04585d455c7be7
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Wed Sep 1 16:54:28 2021 +0200

    Get rid of need_new_row boolean in breakParagraph
    
    Instead of having breakParagraph decide when breaking a row is
    necessary, let Row::shortenIfNeeded set the row_flag of the last
    element to request a row break. This was already done in splitAt.
    
    This is in preparation of splitAt splitting in more than two elements.

diff --git a/src/Row.cpp b/src/Row.cpp
index d3529a3..7b74694 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -473,6 +473,8 @@ Row::Elements splitFrom(Row::Elements & elts, Row::Elements::iterator const & it
 		ret.push_back(init);
 	ret.insert(ret.end(), it, elts.end());
 	elts.erase(it, elts.end());
+	if (!elts.empty())
+		elts.back().row_flags = (elts.back().row_flags & ~AfterFlags) | BreakAfter;
 	return ret;
 }
 
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index fbca421..00ebb55 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1041,6 +1041,7 @@ bool operator==(flexible_const_iterator<T> const & t1,
 	return t1.cit_ == t2.cit_ && t1.pile_.empty() && t2.pile_.empty();
 }
 
+
 Row newRow(TextMetrics const & tm, pit_type pit, pos_type pos, bool is_rtl)
 {
 	Row nrow;
@@ -1070,6 +1071,7 @@ void cleanupRow(Row & row, pos_type pos, pos_type real_endpos, bool is_rtl)
 	row.reverseRTL(is_rtl);
 }
 
+
 // Implement the priorities described in RowFlags.h.
 bool needsRowBreak(int f1, int f2)
 {
@@ -1092,14 +1094,12 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 	bool const is_rtl = text_->isRTL(bigrow.pit());
 	bool const end_label = text_->getEndLabel(bigrow.pit()) != END_LABEL_NO_LABEL;
 
-	bool need_new_row = true;
 	pos_type pos = 0;
 	int width = 0;
 	flexible_const_iterator<Row> fcit = flexible_begin(bigrow);
 	flexible_const_iterator<Row> const end = flexible_end(bigrow);
 	while (true) {
-		bool const has_row = !rows.empty();
-		bool const row_empty = !has_row || rows.back().empty();
+		bool const row_empty = rows.empty() || rows.back().empty();
 		// The row flags of previous element, if there is one.
 		// Otherwise we use NoBreakAfter to avoid an empty row before
 		// e.g. a displayed equation.
@@ -1109,14 +1109,12 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 		// paragraph has an end label (for which an empty row is OK).
 		int const f2 = (fcit == end) ? (end_label ? Inline : NoBreakBefore)
 		                             : fcit->row_flags;
-		need_new_row |= needsRowBreak(f1, f2);
-		if (need_new_row) {
+		if (rows.empty() || needsRowBreak(f1, f2)) {
 			if (!rows.empty())
 				cleanupRow(rows.back(), pos, bigrow.endpos(), is_rtl);
 			rows.push_back(newRow(*this, bigrow.pit(), pos, is_rtl));
 			// the width available for the row.
 			width = max_width_ - rows.back().right_margin;
-			need_new_row = false;
 		}
 
 		// The stopping condition is here because we may need a new
@@ -1146,7 +1144,6 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 			if (!next_elts.empty()) {
 				rb.flushed(false);
 				fcit.put(next_elts);
-				need_new_row = true;
 			}
 		} else {
 			// a new element in the row
@@ -1162,7 +1159,6 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 				// do as if we inserted this element in the original row
 				if (!next_elt.str.empty())
 					fcit.put(next_elt);
-				need_new_row = true;
 			}
 		}
 	}

commit e9824398d85f365bc8d3b1d2eef876ecace207f9
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Tue Aug 31 19:23:55 2021 +0200

    Centralize the code that removes trailing spaces from end row element.
    
    Move to Row::Element::rtrim the code in Row::shortenIfNeeded that
    removes trailing spaces from last element in row, so that it can be
    called when actually breaking a row.
    
    Fixes bug found by Kornel.

diff --git a/src/Row.cpp b/src/Row.cpp
index 38dbb85..d3529a3 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -35,7 +35,6 @@ using namespace std;
 
 namespace lyx {
 
-using support::rtrim;
 using frontend::FontMetrics;
 
 
@@ -162,6 +161,20 @@ Row::Element Row::Element::splitAt(int w, bool force)
 }
 
 
+void Row::Element::rtrim()
+{
+	if (type != STRING)
+		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.
+	 */
+	str = support::rtrim(str);
+	endpos = pos + str.length();
+}
+
+
 bool Row::isMarginSelected(bool left, DocIterator const & beg,
 		DocIterator const & end) const
 {
@@ -539,14 +552,6 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 				break;
 			}
 			end_ = brk.endpos;
-			/* after breakAt, there may be spaces at the end of the
-			 * string, but they are not counted in the string length
-			 * (QTextLayout feature, actually). We remove them, but do
-			 * not change the end of the row, since spaces at row
-			 * break are invisible.
-			 */
-			brk.str = rtrim(brk.str);
-			brk.endpos = brk.pos + brk.str.length();
 			*cit_brk = brk;
 			dim_.wid = wid_brk + brk.dim.wid;
 			// If there are other elements, they should be removed.
@@ -578,11 +583,8 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 	 * boundary this time.
 	 */
 	Element remainder = cit->splitAt(w - wid, true);
-	if (remainder.isValid()) {
+	if (cit->row_flags & BreakAfter) {
 		end_ = cit->endpos;
-		// See comment above.
-		cit->str = rtrim(cit->str);
-		cit->endpos = cit->pos + cit->str.length();
 		dim_.wid = wid + cit->dim.wid;
 		// If there are other elements, they should be removed.
 		return splitFrom(elements_, next(cit, 1), remainder);
diff --git a/src/Row.h b/src/Row.h
index b0b1755..7466797 100644
--- a/src/Row.h
+++ b/src/Row.h
@@ -101,6 +101,8 @@ public:
 		 *   respects the row breaking rules of characters.
 		 */
 		Element splitAt(int w, bool force);
+		// remove trailing spaces (useful for end of row)
+		void rtrim();
 
 		//
 		bool isRTL() const { return font.isVisibleRightToLeft(); }
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index fb2a4ae..fbca421 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1060,6 +1060,10 @@ Row newRow(TextMetrics const & tm, pit_type pit, pos_type pos, bool is_rtl)
 void cleanupRow(Row & row, pos_type pos, pos_type real_endpos, bool is_rtl)
 {
 	row.endpos(pos);
+	// remove trailing spaces on row break
+	if (pos < real_endpos && !row.empty())
+		row.back().rtrim();
+	// boundary exists when there was no space at the end of row
 	row.right_boundary(!row.empty() && pos < real_endpos
 	                   && row.back().endpos == pos);
 	// make sure that the RTL elements are in reverse ordering

commit 257db9a5d88a336c9be777a4d4ba2da84012caa5
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Tue Aug 31 15:58:56 2021 +0200

    Handle the case where breakAt cuts after trailing space
    
    In this case, the extra element returned should empty but valid. The
    row flag BreakAfter is set to indicate that we have a break there
    (this principle will be used more generally in a forthcoming commit).
    
    To detect that we cut at the trailing space, it is necessary to rely
    on the difference between QTextLine::horizontalAdvance() and
    QTextLine::naturalTextWidth() when the flag
    QTextOption::IncludeTrailingSpaces is used: the trailing space is
    taken into account in the later, but not in the former.
    
    Somme comments have been added to make code intent clearer.

diff --git a/src/Row.cpp b/src/Row.cpp
index 6e975a5..38dbb85 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -142,13 +142,19 @@ Row::Element Row::Element::splitAt(int w, bool force)
 	dim.wid = w;
 	int const i = fm.breakAt(str, dim.wid, isRTL(), force);
 	if (i != -1) {
+		//Create a second row element to return
 		Element ret(STRING, pos + i, font, change);
 		ret.str = str.substr(i);
 		ret.endpos = ret.pos + ret.str.length();
+		// Copy the after flags of the original element to the second one.
 		ret.row_flags = row_flags & (CanBreakInside | AfterFlags);
+
+		// Now update ourselves
 		str.erase(i);
 		endpos = pos + i;
-		//lyxerr << "breakAt(" << w << ")  Row element Broken at " << x << "(w(str)=" << fm.width(str) << "): e=" << *this << endl;
+		// Row should be broken after the original element
+		row_flags = (row_flags & ~AfterFlags) | BreakAfter;
+		//LYXERR0("breakAt(" << w << ")  Row element Broken at " << w << "(w(str)=" << fm.width(str) << "): e=" << *this);
 		return ret;
 	}
 
@@ -251,7 +257,7 @@ ostream & operator<<(ostream & os, Row::Element const & e)
 		os << "INVALID: ";
 		break;
 	}
-	os << "width=" << e.full_width();
+	os << "width=" << e.full_width() << ", row_flags=" << e.row_flags;
 	return os;
 }
 
@@ -522,7 +528,7 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 		 *   break-up.
 		 */
 		Element remainder = brk.splitAt(min(w - wid_brk, brk.dim.wid - 2), !word_wrap);
-		if (remainder.isValid()) {
+		if (brk.row_flags & BreakAfter) {
 			/* if this element originally did not cause a row overflow
 			 * in itself, and the remainder of the row would still be
 			 * too large after breaking, then we will have issues in
@@ -544,7 +550,11 @@ Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 			*cit_brk = brk;
 			dim_.wid = wid_brk + brk.dim.wid;
 			// If there are other elements, they should be removed.
-			return splitFrom(elements_, next(cit_brk, 1), remainder);
+			// remainder can be empty when splitting at trailing space
+			if (remainder.str.empty())
+				return splitFrom(elements_, next(cit_brk, 1));
+			else
+				return splitFrom(elements_, next(cit_brk, 1), remainder);
 		}
 	}
 
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index f611434..fb2a4ae 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1156,7 +1156,8 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 			// Add a new next element on the pile
 			if (next_elt.isValid()) {
 				// do as if we inserted this element in the original row
-				fcit.put(next_elt);
+				if (!next_elt.str.empty())
+					fcit.put(next_elt);
 				need_new_row = true;
 			}
 		}
diff --git a/src/frontends/qt/GuiFontMetrics.cpp b/src/frontends/qt/GuiFontMetrics.cpp
index f873689..83c2c0a 100644
--- a/src/frontends/qt/GuiFontMetrics.cpp
+++ b/src/frontends/qt/GuiFontMetrics.cpp
@@ -511,6 +511,9 @@ GuiFontMetrics::breakAt_helper(docstring const & s, int const x,
 	QTextOption to;
 	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);
 	tl.beginLayout();
 	QTextLine line = tl.createLine();
@@ -543,8 +546,11 @@ GuiFontMetrics::breakAt_helper(docstring const & s, int const x,
 		--len;
 	LASSERT(len > 0 || qlen == 0, /**/);
 #endif
-	// Do not cut is the string is already short enough
-	if (len == static_cast<int>(s.length())) {
+	// Do not cut is the string is already short enough. We rely on
+	// naturalTextWidth() to catch the case where we cut at the trailing
+	// space.
+	if (len == static_cast<int>(s.length())
+		&& line.naturalTextWidth() <= x) {
 		len = -1;
 #if QT_VERSION < 0x050000
 		// With some monospace fonts, the value of horizontalAdvance()

commit fe01f83bbe813f2e37e7be14fc1bb7c6340ede70
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Mon Aug 30 15:48:44 2021 +0200

    Workaround for Qt 4
    
    At least with Qt 4.8.7 on Ubuntu 16.04, QTextLine::lineWidth() can
    return a bogus value, at least with Courier font. One hypothesis is
    that the invisible characters that we use in breakAt_helper are given
    a non-null width.
    
    Work around it, although the exact bug has not been pinpointed.

diff --git a/src/frontends/qt/GuiFontMetrics.cpp b/src/frontends/qt/GuiFontMetrics.cpp
index ba16266..f873689 100644
--- a/src/frontends/qt/GuiFontMetrics.cpp
+++ b/src/frontends/qt/GuiFontMetrics.cpp
@@ -517,7 +517,7 @@ GuiFontMetrics::breakAt_helper(docstring const & s, int const x,
 	line.setLineWidth(x);
 	tl.createLine();
 	tl.endLayout();
-	int const line_wid = iround(line.horizontalAdvance());
+	int line_wid = iround(line.horizontalAdvance());
 	if ((force && line.textLength() == offset) || line_wid > x)
 		return {-1, line_wid};
 	/* Since QString is UTF-16 and docstring is UCS-4, the offsets may
@@ -543,9 +543,16 @@ GuiFontMetrics::breakAt_helper(docstring const & s, int const x,
 		--len;
 	LASSERT(len > 0 || qlen == 0, /**/);
 #endif
-	// si la chaîne est déjà trop courte, on ne coupe pas
-	if (len == static_cast<int>(s.length()))
+	// Do not cut is the string is already short enough
+	if (len == static_cast<int>(s.length())) {
 		len = -1;
+#if QT_VERSION < 0x050000
+		// 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.
+		line_wid = width(s);
+#endif
+	}
 	return {len, line_wid};
 }
 

commit 31e59c17b74e7742490c0cad817c6a2f0bf1a342
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Tue Jul 20 00:07:13 2021 +0200

    Last step of transition: use sortenIfNeeded again.
    
    Change semantics of Row::shortenIfNeeded: instead of breaking the row
    and returning a boolean, it returns the list of row elements that have
    been removed (or broken) from the row. The logic of the method remains
    the same.
    
    Use shortenIfNeeded in breakParagraph. This was the last missing block.
    
    Remove Row::breakAt and the old breakRow. Only bugs remain now :)

diff --git a/src/Row.cpp b/src/Row.cpp
index 3bc8ef0..6e975a5 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -156,12 +156,6 @@ Row::Element Row::Element::splitAt(int w, bool force)
 }
 
 
-bool Row::Element::breakAt(int w, bool force)
-{
-	return splitAt(w, force).isValid();
-}
-
-
 bool Row::isMarginSelected(bool left, DocIterator const & beg,
 		DocIterator const & end) const
 {
@@ -448,10 +442,31 @@ void Row::pop_back()
 }
 
 
-bool Row::shortenIfNeeded(int const w, int const next_width)
+namespace {
+
+// Remove stuff after it from elts, and return it.
+// if init is provided, it will be in front of the rest
+Row::Elements splitFrom(Row::Elements & elts, Row::Elements::iterator const & it,
+                        Row::Element const & init = Row::Element())
+{
+	Row::Elements ret;
+	if (init.isValid())
+		ret.push_back(init);
+	ret.insert(ret.end(), it, elts.end());
+	elts.erase(it, elts.end());
+	return ret;
+}
+
+}
+
+
+Row::Elements Row::shortenIfNeeded(int const w, int const next_width)
 {
+	// FIXME: performance: if the last element is a string, we would
+	// like to avoid computing its length.
+	finalizeLast();
 	if (empty() || width() <= w)
-		return false;
+		return Elements();
 
 	Elements::iterator const beg = elements_.begin();
 	Elements::iterator const end = elements_.end();
@@ -467,8 +482,8 @@ bool Row::shortenIfNeeded(int const w, int const next_width)
 
 	if (cit == end) {
 		// This should not happen since the row is too long.
-		LYXERR0("Something is wrong cannot shorten row: " << *this);
-		return false;
+		LYXERR0("Something is wrong, cannot shorten row: " << *this);
+		return Elements();
 	}
 
 	// Iterate backwards over breakable elements and try to break them
@@ -486,8 +501,7 @@ bool Row::shortenIfNeeded(int const w, int const next_width)
 		if (wid_brk <= w && brk.row_flags & CanBreakAfter) {
 			end_ = brk.endpos;
 			dim_.wid = wid_brk;
-			elements_.erase(cit_brk + 1, end);
-			return true;
+			return splitFrom(elements_, cit_brk + 1);
 		}
 		// assume now that the current element is not there
 		wid_brk -= brk.dim.wid;
@@ -507,7 +521,8 @@ bool Row::shortenIfNeeded(int const w, int const next_width)
 		 * - shorter than the natural width of the element, in order to enforce
 		 *   break-up.
 		 */
-		if (brk.breakAt(min(w - wid_brk, brk.dim.wid - 2), !word_wrap)) {
+		Element remainder = brk.splitAt(min(w - wid_brk, brk.dim.wid - 2), !word_wrap);
+		if (remainder.isValid()) {
 			/* if this element originally did not cause a row overflow
 			 * in itself, and the remainder of the row would still be
 			 * too large after breaking, then we will have issues in
@@ -529,14 +544,13 @@ bool Row::shortenIfNeeded(int const w, int const next_width)
 			*cit_brk = brk;
 			dim_.wid = wid_brk + brk.dim.wid;
 			// If there are other elements, they should be removed.
-			elements_.erase(cit_brk + 1, end);
-			return true;
+			return splitFrom(elements_, next(cit_brk, 1), remainder);
 		}
 	}
 
-	if (cit != beg && cit->type == VIRTUAL) {
-		// It is not possible to separate a virtual element from the
-		// previous one.
+	if (cit != beg && cit->row_flags & NoBreakBefore) {
+		// It is not possible to separate this element from the
+		// previous one. (e.g. VIRTUAL)
 		--cit;
 		wid -= cit->dim.wid;
 	}
@@ -546,25 +560,24 @@ bool Row::shortenIfNeeded(int const w, int const next_width)
 		// been added. We can cut right here.
 		end_ = cit->pos;
 		dim_.wid = wid;
-		elements_.erase(cit, end);
-		return true;
+		return splitFrom(elements_, cit);
 	}
 
 	/* If we are here, it means that we have not found a separator to
 	 * shorten the row. Let's try to break it again, but not at word
 	 * boundary this time.
 	 */
-	if (cit->breakAt(w - wid, true)) {
+	Element remainder = cit->splitAt(w - wid, true);
+	if (remainder.isValid()) {
 		end_ = cit->endpos;
 		// See comment above.
 		cit->str = rtrim(cit->str);
 		cit->endpos = cit->pos + cit->str.length();
 		dim_.wid = wid + cit->dim.wid;
 		// If there are other elements, they should be removed.
-		elements_.erase(next(cit, 1), end);
-		return true;
+		return splitFrom(elements_, next(cit, 1), remainder);
 	}
-	return false;
+	return Elements();
 }
 
 
diff --git a/src/Row.h b/src/Row.h
index 72d7a86..b0b1755 100644
--- a/src/Row.h
+++ b/src/Row.h
@@ -101,12 +101,6 @@ public:
 		 *   respects the row breaking rules of characters.
 		 */
 		Element splitAt(int w, bool force);
-		/** Break the element if possible, so that its width is less
-		 * than \param w. Returns true on success. When \param force
-		 * is true, the string is cut at any place, otherwise it
-		 * respects the row breaking rules of characters.
-		 */
-		bool breakAt(int w, bool force);
 
 		//
 		bool isRTL() const { return font.isVisibleRightToLeft(); }
@@ -296,7 +290,7 @@ public:
 	 * \param available width on next row.
 	 * \return true if the row has been shortened.
 	 */
-	bool shortenIfNeeded(int const width, int const next_width);
+	Elements shortenIfNeeded(int const width, int const next_width);
 
 	/**
 	 * If last element of the row is a string, compute its width
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 5f48d29..f611434 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -491,7 +491,7 @@ bool TextMetrics::redoParagraph(pit_type const pit, bool const align_rows)
 
 		// If there is an end of paragraph marker, its size should be
 		// substracted to the available width. The logic here is
-		// almost the same as in breakRow, remember keep them in sync.
+		// almost the same as in tokenizeParagraph, remember keep them in sync.
 		int eop = 0;
 		if (lyxrc.paragraph_markers && e.pos + 1 == par.size()
 		    && size_type(pit + 1) < text_->paragraphs().size()) {
@@ -1005,6 +1005,11 @@ public:
 
 	void put(value_type const & e) { pile_.push_back(e); }
 
+	// Put a sequence of elements on the pile (in reverse order!)
+	void put(vector<value_type> const & elts) {
+		pile_.insert(pile_.end(), elts.rbegin(), elts.rend());
+	}
+
 // This should be private, but declaring the friend functions is too much work
 //private:
 	typename T::const_iterator cit_;
@@ -1120,19 +1125,40 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 		Row::Element elt = *fcit;
 		Row::Element next_elt = elt.splitAt(width - rows.back().width(),
 		                                    !elt.font.language()->wordWrap());
-		// a new element in the row
-		rows.back().push_back(elt);
-		rows.back().finalizeLast();
-		pos = elt.endpos;
-
-		// Go to next element
-		++fcit;
-
-		// Add a new next element on the pile
-		if (next_elt.isValid()) {
-			// do as if we inserted this element in the original row
-			fcit.put(next_elt);
-			need_new_row = true;
+		if (elt.dim.wid > width - rows.back().width()) {
+			Row & rb = rows.back();
+			rb.push_back(*fcit);
+			// if the row is too large, try to cut at last separator. In case
+			// of success, reset indication that the row was broken abruptly.
+			int const next_width = max_width_ - leftMargin(rb.pit(), rb.endpos())
+				- rightMargin(rb.pit());
+
+			Row::Elements next_elts = rb.shortenIfNeeded(width, next_width);
+
+			// Go to next element
+			++fcit;
+
+			// Handle later the elements returned by shortenIfNeeded.
+			if (!next_elts.empty()) {
+				rb.flushed(false);
+				fcit.put(next_elts);
+				need_new_row = true;
+			}
+		} else {
+			// a new element in the row
+			rows.back().push_back(elt);
+			rows.back().finalizeLast();
+			pos = elt.endpos;
+
+			// Go to next element
+			++fcit;
+
+			// Add a new next element on the pile
+			if (next_elt.isValid()) {
+				// do as if we inserted this element in the original row
+				fcit.put(next_elt);
+				need_new_row = true;
+			}
 		}
 	}
 
@@ -1145,185 +1171,6 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 	return rows;
 }
 
-/** This is the function where the hard work is done. The code here is
- * very sensitive to small changes :) Note that part of the
- * intelligence is also in Row::shortenIfNeeded.
- */
-bool TextMetrics::breakRow(Row & row, int const right_margin) const
-{
-	LATTEST(row.empty());//
-	Paragraph const & par = text_->getPar(row.pit());//
-	Buffer const & buf = text_->inset().buffer();//
-	BookmarksSection::BookmarkPosList bpl =//
-		theSession().bookmarks().bookmarksInPar(buf.fileName(), par.id());//
-
-	pos_type const end = par.size();//
-	pos_type const pos = row.pos();//
-	pos_type const body_pos = par.beginOfBody();//
-	bool const is_rtl = text_->isRTL(row.pit());//
-	bool need_new_row = false;//
-
-	row.left_margin = leftMargin(row.pit(), pos);//
-	row.right_margin = right_margin;//
-	if (is_rtl)//
-		swap(row.left_margin, row.right_margin);//
-	// Remember that the row width takes into account the left_margin
-	// but not the right_margin.
-	row.dim().wid = row.left_margin;//
-	// the width available for the row.
-	int const width = max_width_ - row.right_margin;//
-
-	// check for possible inline completion
-	DocIterator const & ic_it = bv_->inlineCompletionPos();//
-	pos_type ic_pos = -1;//
-	if (ic_it.inTexted() && ic_it.text() == text_ && ic_it.pit() == row.pit())//
-		ic_pos = ic_it.pos();//
-
-	// Now we iterate through until we reach the right margin
-	// or the end of the par, then build a representation of the row.
-	pos_type i = pos;//---------------------------------------------------vvv
-	FontIterator fi = FontIterator(*this, par, row.pit(), pos);
-	// The real stopping condition is a few lines below.
-	while (true) {
-		// Firstly, check whether there is a bookmark here.
-		if (lyxrc.bookmarks_visibility == LyXRC::BMK_INLINE)
-			for (auto const & bp_p : bpl)
-				if (bp_p.second == i) {
-					Font f = *fi;
-					f.fontInfo().setColor(Color_bookmark);
-					// ❶ U+2776 DINGBAT NEGATIVE CIRCLED DIGIT ONE
-					char_type const ch = 0x2775 + bp_p.first;
-					row.addVirtual(i, docstring(1, ch), f, Change());
-				}
-
-		// The stopping condition is here so that the display of a
-		// bookmark can take place at paragraph start too.
-		if (i >= end || (i != pos && row.width() > width))//^width
-			break;
-
-		char_type c = par.getChar(i);
-		// The most special cases are handled first.
-		if (par.isInset(i)) {
-			Inset const * ins = par.getInset(i);
-			Dimension dim = bv_->coordCache().insets().dim(ins);
-			row.add(i, ins, dim, *fi, par.lookupChange(i));
-		} else if (c == ' ' && i + 1 == body_pos) {
-			// There is a space at i, but it should not be
-			// added as a separator, because it is just
-			// before body_pos. Instead, insert some spacing to
-			// align text
-			FontMetrics const & fm = theFontMetrics(text_->labelFont(par));
-			// this is needed to make sure that the row width is correct
-			row.finalizeLast();
-			int const add = max(fm.width(par.layout().labelsep),
-			                    labelEnd(row.pit()) - row.width());
-			row.addSpace(i, add, *fi, par.lookupChange(i));
-		} else if (c == '\t')
-			row.addSpace(i, theFontMetrics(*fi).width(from_ascii("    ")),
-				     *fi, par.lookupChange(i));
-		else if (c == 0x2028 || c == 0x2029) {
-			/**
-			 * U+2028 LINE SEPARATOR
-			 * U+2029 PARAGRAPH SEPARATOR
-
-			 * These are special unicode characters that break
-			 * lines/pragraphs. Not handling them lead to trouble wrt
-			 * Qt QTextLayout formatting. We add a visible character
-			 * on screen so that the user can see that something is
-			 * happening.
-			*/
-			row.finalizeLast();
-			// ⤶ U+2936 ARROW POINTING DOWNWARDS THEN CURVING LEFTWARDS
-			// ¶ U+00B6 PILCROW SIGN
-			char_type const screen_char = (c == 0x2028) ? 0x2936 : 0x00B6;
-			row.add(i, screen_char, *fi, par.lookupChange(i), i >= body_pos);
-		} else
-			row.add(i, c, *fi, par.lookupChange(i), i >= body_pos);
-
-		// add inline completion width
-		// draw logically behind the previous character
-		if (ic_pos == i + 1 && !bv_->inlineCompletion().empty()) {
-			docstring const comp = bv_->inlineCompletion();
-			size_t const uniqueTo =bv_->inlineCompletionUniqueChars();
-			Font f = *fi;
-
-			if (uniqueTo > 0) {
-				f.fontInfo().setColor(Color_inlinecompletion);
-				row.addVirtual(i + 1, comp.substr(0, uniqueTo), f, Change());
-			}
-			f.fontInfo().setColor(Color_nonunique_inlinecompletion);
-			row.addVirtual(i + 1, comp.substr(uniqueTo), f, Change());
-		}
-
-		// Handle some situations that abruptly terminate the row
-		// - Before an inset with BreakBefore
-		// - After an inset with BreakAfter
-		Inset const * prevInset = !row.empty() ? row.back().inset : 0;
-		Inset const * nextInset = (i + 1 < end) ? par.getInset(i + 1) : 0;
-		if ((nextInset && nextInset->rowFlags() & BreakBefore)
-		    || (prevInset && prevInset->rowFlags() & BreakAfter)) {
-			row.flushed(true);
-			// Force a row creation after this one if it is ended by
-			// an inset that either
-			// - has row flag RowAfter that enforces that;
-			// - or (1) did force the row breaking, (2) is at end of
-			//   paragraph and (3) the said paragraph has an end label.
-			need_new_row = prevInset &&
-				(prevInset->rowFlags() & AlwaysBreakAfter
-				 || (prevInset->rowFlags() & BreakAfter && i + 1 == end
-				     && text_->getEndLabel(row.pit()) != END_LABEL_NO_LABEL));
-			++i;
-			break;
-		}
-
-		++i;
-		++fi;
-	}
-	row.finalizeLast();
-	row.endpos(i);
-
-	// End of paragraph marker. The logic here is almost the
-	// same as in redoParagraph, remember keep them in sync.
-	ParagraphList const & pars = text_->paragraphs();
-	Change const & change = par.lookupChange(i);
-	if ((lyxrc.paragraph_markers || change.changed())
-	    && !need_new_row // not this
-	    && i == end && size_type(row.pit() + 1) < pars.size()) {
-		// add a virtual element for the end-of-paragraph
-		// marker; it is shown on screen, but does not exist
-		// in the paragraph.
-		Font f(text_->layoutFont(row.pit()));
-		f.fontInfo().setColor(Color_paragraphmarker);
-		f.setLanguage(par.getParLanguage(buf.params()));
-		// ¶ U+00B6 PILCROW SIGN
-		row.addVirtual(end, docstring(1, char_type(0x00B6)), f, change);
-	}
-
-	// Is there a end-of-paragaph change?
-	if (i == end && par.lookupChange(end).changed() && !need_new_row)
-		row.needsChangeBar(true);
-    //--------------------------------------------------------------------^^^
-	// FIXME : nothing below this
-
-	// if the row is too large, try to cut at last separator. In case
-	// of success, reset indication that the row was broken abruptly.
-	int const next_width = max_width_ - leftMargin(row.pit(), row.endpos())
-		- rightMargin(row.pit());
-
-	if (row.shortenIfNeeded(width, next_width))
-		row.flushed(false);
-	row.right_boundary(!row.empty() && row.endpos() < end//
-	                   && row.back().endpos == row.endpos());//
-	// Last row in paragraph is flushed
-	if (row.endpos() == end)//
-		row.flushed(true);//
-
-	// make sure that the RTL elements are in reverse ordering
-	row.reverseRTL(is_rtl);//
-	//LYXERR0("breakrow: row is " << row);
-
-	return need_new_row;
-}
 
 int TextMetrics::parTopSpacing(pit_type const pit) const
 {
diff --git a/src/TextMetrics.h b/src/TextMetrics.h
index 3ff1161..e38ba71 100644
--- a/src/TextMetrics.h
+++ b/src/TextMetrics.h
@@ -151,15 +151,12 @@ private:
 	/// FIXME??
 	int labelEnd(pit_type const pit) const;
 
+	// Turn paragraph oh index \c pit into a single row
 	Row tokenizeParagraph(pit_type pit) const;
 
+	// Break the row produced by tokenizeParagraph() into a list of rows.
 	RowList breakParagraph(Row const & row) const;
 
-	/// sets row.end to the pos value *after* which a row should break.
-	/// for example, the pos after which isNewLine(pos) == true
-	/// \return true when another row is required (after a newline)
-	bool breakRow(Row & row, int right_margin) const;
-
 	// Expands the alignment of row \param row in paragraph \param par
 	LyXAlignment getAlign(Paragraph const & par, Row const & row) const;
 	/// Aligns properly the row contents (computes spaces and fills)

commit 4323b15b05378fcdeb460842b84fc5002faec080
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Sun Jul 18 01:09:33 2021 +0200

    Implement handling of row_flags for row breaking
    
    To this end, add the helper function needsRowBreak which computes the
    effect of two consecutive row flags. This function implements the
    priorities described in RowFlags.h.
    
    This function is called with the relevant flags, or NoBreak* when at
    boundaries and updates need_new_row.
    
    Some common code is factored in a new cleanupRow() helper.

diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 12548e0..5f48d29 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -979,22 +979,6 @@ Row TextMetrics::tokenizeParagraph(pit_type const pit) const
 
 namespace {
 
-Row newRow(TextMetrics const & tm, pit_type pit, pos_type pos, bool is_rtl)
-{
-	Row nrow;
-	nrow.pit(pit);
-	nrow.pos(pos);
-	nrow.left_margin = tm.leftMargin(pit, pos);
-	nrow.right_margin = tm.rightMargin(pit);
-	if (is_rtl)
-		swap(nrow.left_margin, nrow.right_margin);
-	// Remember that the row width takes into account the left_margin
-	// but not the right_margin.
-	nrow.dim().wid = nrow.left_margin;
-	return nrow;
-}
-
-
 /** Helper template flexible_const_iterator<T>
  * A way to iterate over a const container, but insert fake elements in it.
  * In the case of a row, we will have to break some elements, which
@@ -1017,6 +1001,8 @@ public:
 
 	value_type operator*() const { return pile_.empty() ? *cit_ : pile_.back(); }
 
+	value_type const * operator->() const { return pile_.empty() ? &*cit_ : &pile_.back(); }
+
 	void put(value_type const & e) { pile_.push_back(e); }
 
 // This should be private, but declaring the friend functions is too much work
@@ -1050,6 +1036,44 @@ bool operator==(flexible_const_iterator<T> const & t1,
 	return t1.cit_ == t2.cit_ && t1.pile_.empty() && t2.pile_.empty();
 }
 
+Row newRow(TextMetrics const & tm, pit_type pit, pos_type pos, bool is_rtl)
+{
+	Row nrow;
+	nrow.pit(pit);
+	nrow.pos(pos);
+	nrow.left_margin = tm.leftMargin(pit, pos);
+	nrow.right_margin = tm.rightMargin(pit);
+	if (is_rtl)
+		swap(nrow.left_margin, nrow.right_margin);
+	// Remember that the row width takes into account the left_margin
+	// but not the right_margin.
+	nrow.dim().wid = nrow.left_margin;
+	return nrow;
+}
+
+
+void cleanupRow(Row & row, pos_type pos, pos_type real_endpos, bool is_rtl)
+{
+	row.endpos(pos);
+	row.right_boundary(!row.empty() && pos < real_endpos
+	                   && row.back().endpos == pos);
+	// make sure that the RTL elements are in reverse ordering
+	row.reverseRTL(is_rtl);
+}
+
+// Implement the priorities described in RowFlags.h.
+bool needsRowBreak(int f1, int f2)
+{
+	if (f1 & AlwaysBreakAfter /*|| f2 & AlwaysBreakBefore*/)
+		return true;
+	if (f1 & NoBreakAfter || f2 & NoBreakBefore)
+		return false;
+	if (f1 & BreakAfter || f2 & BreakBefore)
+		return true;
+	return false;
+}
+
+
 }
 
 
@@ -1057,6 +1081,7 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 {
 	RowList rows;
 	bool const is_rtl = text_->isRTL(bigrow.pit());
+	bool const end_label = text_->getEndLabel(bigrow.pit()) != END_LABEL_NO_LABEL;
 
 	bool need_new_row = true;
 	pos_type pos = 0;
@@ -1064,15 +1089,21 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 	flexible_const_iterator<Row> fcit = flexible_begin(bigrow);
 	flexible_const_iterator<Row> const end = flexible_end(bigrow);
 	while (true) {
+		bool const has_row = !rows.empty();
+		bool const row_empty = !has_row || rows.back().empty();
+		// The row flags of previous element, if there is one.
+		// Otherwise we use NoBreakAfter to avoid an empty row before
+		// e.g. a displayed equation.
+		int const f1 = row_empty ? NoBreakAfter : rows.back().back().row_flags;
+		// The row flags of next element, if there is one.
+		// Otherwise we use NoBreakBefore (see above), unless the
+		// paragraph has an end label (for which an empty row is OK).
+		int const f2 = (fcit == end) ? (end_label ? Inline : NoBreakBefore)
+		                             : fcit->row_flags;
+		need_new_row |= needsRowBreak(f1, f2);
 		if (need_new_row) {
-			if (!rows.empty()) {
-				Row & rb = rows.back();
-				rb.endpos(pos);
-				rb.right_boundary(!rb.empty() && rb.endpos() < bigrow.endpos()
-								   && rb.back().endpos == rb.endpos());
-				// make sure that the RTL elements are in reverse ordering
-				rb.reverseRTL(is_rtl);
-			}
+			if (!rows.empty())
+				cleanupRow(rows.back(), pos, bigrow.endpos(), is_rtl);
 			rows.push_back(newRow(*this, bigrow.pit(), pos, is_rtl));
 			// the width available for the row.
 			width = max_width_ - rows.back().right_margin;
@@ -1106,13 +1137,9 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 	}
 
 	if (!rows.empty()) {
-		Row & rb = rows.back();
+		cleanupRow(rows.back(), pos, bigrow.endpos(), is_rtl);
 		// Last row in paragraph is flushed
-		rb.flushed(true);
-		rb.endpos(bigrow.endpos());
-		rb.right_boundary(false);
-		// make sure that the RTL elements are in reverse ordering
-		rb.reverseRTL(is_rtl);
+		rows.back().flushed(true);
 	}
 
 	return rows;
@@ -1226,9 +1253,8 @@ bool TextMetrics::breakRow(Row & row, int const right_margin) const
 			}
 			f.fontInfo().setColor(Color_nonunique_inlinecompletion);
 			row.addVirtual(i + 1, comp.substr(uniqueTo), f, Change());
-		}//---------------------------------------------------------------^^^
+		}
 
-		// FIXME: Handle when breaking the rows
 		// Handle some situations that abruptly terminate the row
 		// - Before an inset with BreakBefore
 		// - After an inset with BreakAfter
@@ -1253,7 +1279,6 @@ bool TextMetrics::breakRow(Row & row, int const right_margin) const
 		++i;
 		++fi;
 	}
-	//--------------------------------------------------------------------vvv
 	row.finalizeLast();
 	row.endpos(i);
 

commit 1be84ac684ca27ce2c5b7141af9ac0a50bd531ab
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date:   Sat Jul 17 23:16:15 2021 +0200

    Change the way the element's width is updated.
    
    Remove the code that computed the width every 30 characters (yay!).
    Make sure that finalizeLast() is called after inserting a row element in
    a row in breakParagraph.

diff --git a/src/Row.cpp b/src/Row.cpp
index 705a147..3bc8ef0 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -370,8 +370,7 @@ void Row::finalizeLast()
 	if (elt.change.changed())
 		changebar_ = true;
 
-	if (elt.type == STRING) {
-		dim_.wid -= elt.dim.wid;
+	if (elt.type == STRING && elt.dim.wid == 0) {
 		elt.dim.wid = theFontMetrics(elt.font).width(elt.str);
 		dim_.wid += elt.dim.wid;
 	}
@@ -401,16 +400,8 @@ void Row::add(pos_type const pos, char_type const c,
 		e.row_flags = can_break ? CanBreakInside : Inline;
 		elements_.push_back(e);
 	}
-	if (back().str.length() % 30 == 0) {
-		dim_.wid -= back().dim.wid;
-		back().str += c;
-		back().endpos = pos + 1;
-		back().dim.wid = theFontMetrics(back().font).width(back().str);
-		dim_.wid += back().dim.wid;
-	} else {
-		back().str += c;
-		back().endpos = pos + 1;
-	}
+	back().str += c;
+	back().endpos = pos + 1;
 }
 
 
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 3354746..12548e0 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1091,6 +1091,7 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 		                                    !elt.font.language()->wordWrap());
 		// a new element in the row
 		rows.back().push_back(elt);
+		rows.back().finalizeLast();
 		pos = elt.endpos;
 
 		// Go to next element

-----------------------------------------------------------------------

Summary of changes:
 src/Row.cpp         |   79 ++++++++++++++++++++++----------------------------
 src/Row.h           |   27 +++++------------
 src/RowPainter.cpp  |    4 --
 src/TextMetrics.cpp |   52 ++++++++++++---------------------
 4 files changed, 62 insertions(+), 100 deletions(-)


hooks/post-receive
-- 
Repository for new features


More information about the lyx-cvs mailing list