Improve cursor movement with boundaries

Introduce a new NoEndBoundary flag for insets like InsetNewline.

Indroduce Row::start_boundary() that is true when previous Row has
end_boundary() set.

Use this to improve cursor movement around row boundaries (both for
logical ad visible cursor movement). The new code remove some of the
newline/separator hardcoding.

src/Cursor.cpp

@@ -1288,7 +1288,9 @@ bool Cursor::posVisToNewRow(bool movingLeft)
 	// if moving left in an LTR paragraph or moving right in an
 	// RTL one, move to previous row
 	if (par_is_LTR == movingLeft) {
-		if (row.pos() == 0) { // we're at first row in paragraph
+		if (row.start_boundary())
+			boundary(true);
+		else if (row.pos() == 0) { // we're at first row in paragraph
 			if (pit() == 0) // no previous paragraph! don't move
 				return false;
 			// move to last pos in previous par

src/MetricsInfo.h

@@ -105,6 +105,7 @@ public:
 	/// The context to resolve macros
 	MacroContext const & macrocontext;
 	/// Are we at the start of a paragraph (vertical mode)?
+	/// This is not used anymore, but could be useful
 	bool vmode;
 	/// if true, do not expand insets to max width artificially
 	bool tight_insets;

src/ParagraphMetrics.cpp

@@ -95,6 +95,10 @@ size_t ParagraphMetrics::getRowIndex(pos_type pos, bool boundary) const
 {
 	LBUFERR(!rows().empty());
 
+	// This makes a difference when the first row is empty (e.g. before display math)
+	if (pos == 0 && boundary)
+		return 0;
+
 	// If boundary is set we should return the row on which
 	// the character before is inside.
 	if (pos > 0 && boundary)

src/Row.h

@@ -212,6 +212,10 @@ public:
 	///
 	pos_type endpos() const { return end_; }
 	///
+	void start_boundary(bool b) { start_boundary_ = b; }
+	///
+	bool start_boundary() const { return start_boundary_; }
+	///
 	void end_boundary(bool b) { end_boundary_ = b; }
 	///
 	bool end_boundary() const { return end_boundary_; }
@@ -373,6 +377,8 @@ private:
 	pos_type pos_ = 0;
 	/// one behind last pos covered by this row
 	pos_type end_ = 0;
+	// Is there a boundary at the start of the row (display inset...)
+	bool start_boundary_ = false;
 	// Is there a boundary at the end of the row (display inset...)
 	bool end_boundary_ = false;
 	// Shall the row be flushed when it is supposed to be justified?

src/RowFlags.h

@@ -27,30 +27,34 @@ enum RowFlags {
 	// Do not break before or after this element, except if really
 	// needed (between NoBreak* and CanBreak*).
 	Inline = 0,
+	// force (maybe empty) row before this element
+	AlwaysBreakBefore = 1 << 0,
 	// break row before this element if the row is not empty
-	BreakBefore = 1 << 0,
+	BreakBefore = 1 << 1,
 	// break row whenever needed before this element
-	CanBreakBefore = 1 << 1,
+	CanBreakBefore = 1 << 2,
 	// Avoid breaking row before this element
-	NoBreakBefore = 1 << 2,
+	NoBreakBefore = 1 << 3,
 	// flush the row before this element (useful with BreakBefore)
-	FlushBefore = 1 << 3,
+	FlushBefore = 1 << 4,
 	// force new (maybe empty) row after this element
-	AlwaysBreakAfter = 1 << 4,
+	AlwaysBreakAfter = 1 << 5,
 	// break row after this element if there are more elements
-	BreakAfter = 1 << 5,
+	BreakAfter = 1 << 6,
 	// break row whenever needed after this element
-	CanBreakAfter = 1 << 6,
+	CanBreakAfter = 1 << 7,
 	// Avoid breaking row after this element
-	NoBreakAfter = 1 << 7,
+	NoBreakAfter = 1 << 8,
 	// The contents of the row may be broken in two (e.g. string)
-	CanBreakInside = 1 << 8,
+	CanBreakInside = 1 << 9,
 	// Flush the row that ends with this element
-	Flush = 1 << 9,
+	Flush = 1 << 10,
 	// specify an alignment (left, right) for a display element
 	// (default is center)
-	AlignLeft = 1 << 10,
-	AlignRight = 1 << 11,
+	AlignLeft = 1 << 11,
+	AlignRight = 1 << 12,
+	// Forbid boundary after this element
+	NoEndBoundary = 1 << 13,
 	// A display element breaks row at both ends
 	Display = FlushBefore | BreakBefore | BreakAfter,
 	// Flags that concern breaking after element

src/Text.cpp

@@ -3054,29 +3054,16 @@ bool Text::cursorBackward(Cursor & cur)
 	// Tell BufferView to test for FitCursor in any case!
 	cur.screenUpdateFlags(Update::FitCursor);
 
+	// if on right side of a row boundary (at row start), skip it,
+	// i.e. set boundary to true, i.e. go only logically left
+	if (!cur.boundary()
+	     && cur.textRow().pos() == cur.pos()
+	     && cur.textRow().start_boundary()) {
+		return setCursor(cur, cur.pit(), cur.pos(), true, true);
+	}
+
 	// not at paragraph start?
 	if (cur.pos() > 0) {
-		// if on right side of boundary (i.e. not at paragraph end, but line end)
-		// -> skip it, i.e. set boundary to true, i.e. go only logically left
-		// there are some exceptions to ignore this: lineseps, newlines, spaces
-#if 0
-		// some effectless debug code to see the values in the debugger
-		bool bound = cur.boundary();
-		int rowpos = cur.textRow().pos();
-		int pos = cur.pos();
-		bool sep = cur.paragraph().isSeparator(cur.pos() - 1);
-		bool newline = cur.paragraph().isNewline(cur.pos() - 1);
-		bool linesep = cur.paragraph().isLineSeparator(cur.pos() - 1);
-#endif
-		if (!cur.boundary() &&
-				cur.textRow().pos() == cur.pos() &&
-				!cur.paragraph().isLineSeparator(cur.pos() - 1) &&
-				!cur.paragraph().isNewline(cur.pos() - 1) &&
-				!cur.paragraph().isEnvSeparator(cur.pos() - 1) &&
-				!cur.paragraph().isSeparator(cur.pos() - 1)) {
-			return setCursor(cur, cur.pit(), cur.pos(), true, true);
-		}
-
 		// go left and try to enter inset
 		if (checkAndActivateInset(cur, false))
 			return false;
@@ -3143,33 +3130,14 @@ bool Text::cursorForward(Cursor & cur)
 
 		// next position is left of boundary,
 		// but go to next line for special cases like space, newline, linesep
-#if 0
-		// some effectless debug code to see the values in the debugger
-		int endpos = cur.textRow().endpos();
-		int lastpos = cur.lastpos();
-		int pos = cur.pos();
-		bool linesep = cur.paragraph().isLineSeparator(cur.pos());
-		bool newline = cur.paragraph().isNewline(cur.pos());
-		bool sep = cur.paragraph().isSeparator(cur.pos());
-		if (cur.pos() != cur.lastpos()) {
-			bool linesep2 = cur.paragraph().isLineSeparator(cur.pos()+1);
-			bool newline2 = cur.paragraph().isNewline(cur.pos()+1);
-			bool sep2 = cur.paragraph().isSeparator(cur.pos()+1);
-		}
-#endif
 		if (cur.textRow().endpos() == cur.pos() + 1) {
 			if (cur.paragraph().isEnvSeparator(cur.pos()) &&
 			    cur.pos() + 1 == cur.lastpos() &&
 			    cur.pit() != cur.lastpit()) {
 				// move to next paragraph
 				return setCursor(cur, cur.pit() + 1, 0, true, false);
-			} else if (cur.textRow().endpos() != cur.lastpos() &&
-				   !cur.paragraph().isNewline(cur.pos()) &&
-				   !cur.paragraph().isEnvSeparator(cur.pos()) &&
-				   !cur.paragraph().isLineSeparator(cur.pos()) &&
-				   !cur.paragraph().isSeparator(cur.pos())) {
+			} else if (cur.textRow().end_boundary())
 				return setCursor(cur, cur.pit(), cur.pos() + 1, true, true);
-			}
 		}
 
 		// in front of RTL boundary? Stay on this side of the boundary because:

src/TextMetrics.cpp

@@ -706,7 +706,8 @@ LyXAlignment TextMetrics::getAlign(Paragraph const & par, Row const & row) const
 
 	// Display-style insets should always be on a centered row
 	if (Inset const * inset = par.getInset(row.pos())) {
-		if (inset->rowFlags() & Display) {
+		// If we are in empty row, alignment of inset does not apply (it is in next row)
+		if (!row.empty() && inset->rowFlags() & Display) {
 			if (inset->rowFlags() & AlignLeft)
 				align = LYX_ALIGN_LEFT;
 			else if (inset->rowFlags() & AlignRight)
@@ -1146,7 +1147,9 @@ void cleanupRow(Row & row, bool at_end)
 	if (!at_end && !row.flushed())
 		row.back().rtrim();
 	// boundary exists when there was no space at the end of row
-	row.end_boundary(!at_end && row.back().endpos == row.endpos());
+	row.end_boundary(!at_end
+	                 && row.back().endpos == row.endpos()
+	                 && !(row.back().row_flags & NoEndBoundary));
 	// make sure that the RTL elements are in reverse ordering
 	row.reverseRTL();
 }
@@ -1155,7 +1158,7 @@ void cleanupRow(Row & row, bool at_end)
 // Implement the priorities described in RowFlags.h.
 bool needsRowBreak(int f1, int f2)
 {
-	if (f1 & AlwaysBreakAfter /*|| f2 & AlwaysBreakBefore*/)
+	if (f1 & AlwaysBreakAfter || f2 & AlwaysBreakBefore)
 		return true;
 	if (f1 & NoBreakAfter || f2 & NoBreakBefore)
 		return false;
@@ -1183,13 +1186,21 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 		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.
+		// e.g. a displayed inset.
 		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;
+		if (rows.empty() && needsRowBreak(f1, f2)) {
+			// Create an empty row before element
+			rows.push_back(newRow(*this, bigrow.pit(), 0, is_rtl));
+			Row & newrow = rows.back();
+			cleanupRow(newrow, false);
+			newrow.end_boundary(true);
+			newrow.left_margin = leftMargin(newrow.pit(), 0, true);
+		}
 		if (rows.empty() || needsRowBreak(f1, f2)) {
 			if (!rows.empty()) {
 				// Flush row as requested by row flags
@@ -1241,6 +1252,13 @@ RowList TextMetrics::breakParagraph(Row const & bigrow) const
 			rows.back().needsChangeBar(true);
 	}
 
+	// Set start_boundary to be equal to the previous row's end boundary
+	bool sb = false;
+	for (auto & row : rows) {
+		row.start_boundary(sb);
+		sb = row.end_boundary();
+	}
+
 	return rows;
 }
 
@@ -1468,14 +1486,15 @@ pos_type TextMetrics::getPosNearX(Row const & row, int & x,
 			boundary = true;
 	}
 
+	if (row.empty())
+		boundary = row.end_boundary();
 	/** This tests for the case where the cursor is set at the end
 	 * of a row which has been broken due something else than a
 	 * separator (a display inset or a forced breaking of the
 	 * row). We know that there is a separator when the end of the
 	 * row is larger than the end of its last element.
 	 */
-	if (!row.empty() && pos == row.back().endpos
-	    && row.back().endpos == row.endpos()) {
+	else if (pos == row.back().endpos && row.back().endpos == row.endpos()) {
 		Inset const * inset = row.back().inset;
 		if (inset && (inset->lyxCode() == NEWLINE_CODE
 		              || inset->lyxCode() == SEPARATOR_CODE))
@@ -1838,7 +1857,7 @@ int TextMetrics::leftMargin(pit_type pit) const
 }
 
 
-int TextMetrics::leftMargin(pit_type const pit, pos_type const pos) const
+int TextMetrics::leftMargin(pit_type const pit, pos_type const pos, bool ignore_contents) const
 {
 	ParagraphList const & pars = text_->paragraphs();
 
@@ -1998,7 +2017,8 @@ int TextMetrics::leftMargin(pit_type const pit, pos_type const pos) const
 	    // in some insets, paragraphs are never indented
 	    && !text_->inset().neverIndent()
 	    // display style insets do not need indentation
-	    && !(!par.empty()
+	    && !(!ignore_contents
+	         && !par.empty()
 	         && par.isInset(0)
 	         && par.getInset(0)->rowFlags() & Display)
 	    && (!(tclass.isDefaultLayout(par.layout())

src/TextMetrics.h

@@ -138,8 +138,10 @@ public:
 	 * This information cannot be taken from the layout object, because
 	 * in LaTeX the beginning of the text fits in some cases
 	 * (for example sections) exactly the label-width.
+	 * When \c ignore_contents is true, alignment properties related
+	 * to insets in paragraph are not taken into account.
 	 */
-	int leftMargin(pit_type pit, pos_type pos) const;
+	int leftMargin(pit_type pit, pos_type pos, bool ignore_contents  = false) const;
 	/// Return the left beginning of a row which is not the first one.
 	/// This is the left margin when there is no indentation.
 	int leftMargin(pit_type pit) const;

src/insets/InsetNewline.cpp

@@ -44,9 +44,9 @@ InsetNewline::InsetNewline() : Inset(nullptr)
 int InsetNewline::rowFlags() const
 {
 	if (params_.kind == InsetNewlineParams::LINEBREAK)
-		return AlwaysBreakAfter;
+		return AlwaysBreakAfter | NoEndBoundary;
 	else
-	    return AlwaysBreakAfter | Flush;
+	    return AlwaysBreakAfter | NoEndBoundary | Flush;
 }
 
 

src/insets/InsetSeparator.h

@@ -65,7 +65,7 @@ public:
 		return docstring();
 	}
 	///
-	int rowFlags() const override { return BreakAfter | Flush; }
+	int rowFlags() const override { return BreakAfter | Flush | NoEndBoundary; }
 	///
 	bool nextnoindent() const { return params_.kind == InsetSeparatorParams::PLAIN; }
 private:

src/mathed/InsetMathHull.cpp

@@ -525,9 +525,6 @@ void InsetMathHull::metrics(MetricsInfo & mi, Dimension & dim) const
 	*/
 	int const bottom_display_margin = mi.base.bv->zoomedPixels(6);
 	int top_display_margin = bottom_display_margin;
-	// at start of paragraph, add an empty line
-	if (mi.vmode)
-		top_display_margin += theFontMetrics(mi.base.font).maxHeight() + 2;
 
 	int const ind = indent(*mi.base.bv);
 	mi.extrawidth = ind;
@@ -1029,9 +1026,9 @@ int InsetMathHull::rowFlags() const
 	case hullMultline:
 	case hullGather:
 		if (buffer().params().is_math_indent)
-			return Display | AlignLeft;
+			return AlwaysBreakBefore | Display | AlignLeft;
 		else
-			return Display;
+			return AlwaysBreakBefore | Display;
 	}
 	// avoid warning
 	return Display;