Rewrite Cursor::getSurroundingPos without Bidi class

New method TextMetrics::findRowElement, excerpted from CursorX.

Reimplement getSurroundingPos using Row information. This is easy when
the cursor is inside a row element. At row element edges, different
situations can occur; hopefully all these situations are taken into
account.

Rename the old getSurroundingPos to getSurroundingPosOrig and
transform getSurroundingPos into a wrapper that compares the two
methods. This will be removed when we are confident that the new
function is equivalent to the old one.

It will then be possible to remove also the Bidi class (at last!).

src/Cursor.cpp

@@ -848,7 +848,104 @@ bool Cursor::posVisLeft(bool skip_inset)
 }
 
 
-void Cursor::getSurroundingPos(pos_type & left_pos, pos_type & right_pos)
+namespace {
+
+// Return true on success
+bool findNonVirtual(Row const & row, Row::const_iterator & cit, bool onleft)
+{
+	if (onleft) {
+		while (cit != row.begin() && cit->isVirtual())
+			--cit;
+	} else {
+		while (cit != row.end() && cit->isVirtual())
+			++cit;
+	}
+	return cit != row.end() && !cit->isVirtual();
+}
+
+}
+
+void Cursor::getSurroundingPosNew(pos_type & left_pos, pos_type & right_pos) const
+{
+	// by default, we know nothing.
+	left_pos = -1;
+	right_pos = -1;
+
+	Row const & row = textRow();
+	TextMetrics const & tm = bv_->textMetrics(text());
+	double dummy = 0;
+	Row::const_iterator cit = tm.findRowElement(row, pos(), boundary(), dummy);
+	// Handle the case of empty row
+	if (cit == row.end()) {
+		if (paragraph().isRTL(buffer()->params()))
+			right_pos = row.pos();
+		else
+			left_pos = row.pos() - 1;
+		return;
+	}
+
+	// skip virtual elements and exit if no non-virtual one exists
+	if (!findNonVirtual(row, cit, !cit->isRTL()))
+		return;
+
+	// if the position is at the left side of the element, we have to
+	// look at the previous element
+	if (pos() == cit->left_pos()) {
+		LYXERR(Debug::RTL, "getSurroundingPos(" << pos() << (boundary() ? "b" : "")
+			   << "), AT LEFT of *cit=" << *cit);
+		// this one is easy (see common case below)
+		right_pos = pos() - (cit->isRTL() ? 1 : 0);
+		// at the left of the row
+		if (cit == row.begin())
+			return;
+		--cit;
+		if (!findNonVirtual(row, cit, true))
+			return;
+		// [...[ is the row element, | is cursor position (! with boundary)
+		// [ 1 2 [ is a ltr row element with pos=1 and endpos=3
+		// ] 2 1] is an rtl row element with pos=1 and endpos=3
+		//    [ 1 2 [  [|3 4 [ => (2, 3)
+		// or [ 1 2 [  ]!4 3 ] => (2, 4)
+		// or ] 2 1 ]  [|3 4 [ => (1, 3)
+		// or ] 4 3 ]  ]!2 1 ] => (3, 2)
+		left_pos = cit->right_pos() - (cit->isRTL() ? 0 : 1);
+		// happens with consecutive row of same direction
+		if (left_pos == right_pos) {
+			left_pos += cit->isRTL() ? 1 : -1;
+		}
+	}
+	// same code but with the element at the right
+	else if (pos() == cit->right_pos()) {
+		LYXERR(Debug::RTL, "getSurroundingPos(" << pos() << (boundary() ? "b" : "")
+			   << "), AT RIGHT of *cit=" << *cit);
+		// this one is easy (see common case below)
+		left_pos = pos() - (cit->isRTL() ? 0 : 1);
+		// at the right of the row
+		if (cit + 1 == row.end())
+			return;
+		++cit;
+		if (!findNonVirtual(row, cit, false))
+			return;
+		//    [ 1 2![  [ 3 4 [ => (2, 3)
+		// or [ 1 2![  ] 4 3 ] => (2, 4)
+		// or ] 2 1|]  [ 3 4 [ => (1, 3)
+		// or ] 4 3|]  ] 2 1 ] => (3, 2)
+		right_pos = cit->left_pos() - (cit->isRTL() ? 1 : 0);
+		// happens with consecutive row of same direction
+		if (right_pos == left_pos)
+			right_pos += cit->isRTL() ? -1 : 1;
+	}
+	// common case: both positions are inside the row element
+	else {
+		//    [ 1 2|3 [ => (2, 3)
+		// or ] 3|2 1 ] => (3, 2)
+		left_pos = pos() - (cit->isRTL() ? 0 : 1);
+		right_pos = pos() - (cit->isRTL() ? 1 : 0);
+	}
+}
+
+
+void Cursor::getSurroundingPosOrig(pos_type & left_pos, pos_type & right_pos) const
 {
 	// preparing bidi tables
 	Paragraph const & par = paragraph();
@@ -963,6 +1060,34 @@ void Cursor::getSurroundingPos(pos_type & left_pos, pos_type & right_pos)
 }
 
 
+void Cursor::getSurroundingPos(pos_type & left_pos, pos_type & right_pos) const
+{
+	// Check result wrt old implementation
+	// FIXME: remove after correct testing.
+	pos_type lp, rp;
+	getSurroundingPosNew(lp, rp);
+	getSurroundingPosOrig(left_pos, right_pos);
+	if (lp != left_pos || rp != right_pos) {
+		Row const & row = textRow();
+		TextMetrics const & tm = bv_->textMetrics(text());
+		double dummy = 0;
+		Row::const_iterator cit = tm.findRowElement(row, pos(), boundary(), dummy);
+		if (cit != row.end())
+			LYXERR0("Wrong surroundingpos: old=(" << left_pos << ", " << right_pos
+					<< "), new=(" << lp << ", " << rp
+					<< ") *cit= " << *cit
+					<< "\ncur = " << *this << "\nrow =" << row);
+		else
+			LYXERR0("Wrong surroundingpos: old=(" << left_pos << ", " << right_pos
+					<< "), new=(" << lp << ", " << rp
+					<< ") in empty row"
+					<< "\ncur = " << *this << "\nrow =" << row);
+	}
+	LYXERR(Debug::RTL,"getSurroundingPos(" << pos() << (boundary() ? "b" : "")
+		   << ") => (" << left_pos << ", " << right_pos <<")");
+}
+
+
 bool Cursor::posVisToNewRow(bool movingLeft)
 {
 	Paragraph const & par = paragraph();

src/Cursor.h

@@ -225,7 +225,9 @@ public:
 	 * If the cursor is at the edge of a row, the position which is "over the 
 	 * edge" will be returned as -1.
 	 */
-	void getSurroundingPos(pos_type & left_pos, pos_type & right_pos);
+	void getSurroundingPos(pos_type & left_pos, pos_type & right_pos) const;
+	void getSurroundingPosNew(pos_type & left_pos, pos_type & right_pos) const;
+	void getSurroundingPosOrig(pos_type & left_pos, pos_type & right_pos) const;
 	/// the row in the paragraph we're in
 	Row const & textRow() const;
 

src/Row.h

@@ -89,6 +89,10 @@ public:
 
 		//
 		bool isRTL() const { return font.isVisibleRightToLeft(); }
+		// This is true for virtual elements.
+		// Note that we do not use the type here. The two definitions
+		// should be equivalent
+		bool isVirtual() const { return pos == endpos; }
 
 		// The kind of row element
 		Type type;

src/TextMetrics.cpp

@@ -1410,6 +1410,56 @@ Inset * TextMetrics::checkInsetHit(int x, int y)
 }
 
 
+Row::const_iterator const
+TextMetrics::findRowElement(Row const & row, pos_type const pos,
+                            bool const boundary, double & x) const
+{
+	/**
+	 * When boundary is true, position i is in the row element (pos, endpos)
+	 * if
+	 *    pos < i <= endpos
+	 * whereas, when boundary is false, the test is
+	 *    pos <= i < endpos
+	 * The correction below allows to handle both cases.
+	*/
+	int const boundary_corr = (boundary && pos) ? -1 : 0;
+
+	x = row.left_margin;
+
+	/** Early return in trivial cases
+	 * 1) the row is empty
+	 * 2) the position is the left-most position of the row; there
+	 * is a quirk here however: if the first element is virtual
+	 * (end-of-par marker for example), then we have to look
+	 * closer
+	 */
+	if (row.empty()
+	    || (pos == row.begin()->left_pos() && !boundary
+			&& !row.begin()->isVirtual()))
+		return row.begin();
+
+	Row::const_iterator cit = row.begin();
+	for ( ; cit != row.end() ; ++cit) {
+		/** Look whether the cursor is inside the element's
+		 * span. Note that it is necessary to take the
+		 * boundary into account, and to accept virtual
+		 * elements, which have pos == endpos.
+		 */
+		if (pos + boundary_corr >= cit->pos
+		    && (pos + boundary_corr < cit->endpos || cit->isVirtual())) {
+				x += cit->pos2x(pos);
+				break;
+		}
+		x += cit->full_width();
+	}
+
+	if (cit == row.end())
+		--cit;
+
+	return cit;
+}
+
+
 int TextMetrics::cursorX(CursorSlice const & sl,
 		bool boundary) const
 {
@@ -1421,46 +1471,10 @@ int TextMetrics::cursorX(CursorSlice const & sl,
 	Row const & row = pm.getRow(sl.pos(), boundary);
 	pos_type const pos = sl.pos();
 
-	/**
-	 * When boundary is true, position i is in the row element (pos, endpos)
-	 * if
-	 *    pos < i <= endpos
-	 * whereas, when boundary is false, the test is
-	 *    pos <= i < endpos
-	 * The correction below allows to handle both cases.
-	*/
-	int const boundary_corr = (boundary && pos) ? -1 : 0;
-
-	/** Early return in trivial cases
-	 * 1) the row is empty
-	 * 2) the position is the left-most position of the row; there
-	 * is a quirck herehowever: if the first element is virtual
-	 * (end-of-par marker for example), then we have to look
-	 * closer
-	 */
-	if (row.empty()
-	    || (pos == row.begin()->left_pos()
-		&& pos != row.begin()->right_pos()))
-		return row.left_margin;
-
-	Row::const_iterator cit = row.begin();
-	double x = row.left_margin;
-	for ( ; cit != row.end() ; ++cit) {
-		/** Look whether the cursor is inside the element's
-		 * span. Note that it is necessary to take the
-		 * boundary into account, and to accept virtual
-		 * elements, which have pos == endpos.
-		 */
-		if (pos + boundary_corr >= cit->pos
-		    && (pos + boundary_corr < cit->endpos
-			|| cit->pos == cit->endpos)) {
-				x += cit->pos2x(pos);
-				break;
-		}
-		x += cit->full_width();
-	}
-
+	double x = 0;
+	findRowElement(row, pos, boundary, x);
 	return int(x);
+
 }
 
 

src/TextMetrics.h

@@ -191,12 +191,16 @@ public:
 	/// x,y are screen coordinates
 	void setCursorFromCoordinates(Cursor & cur, int x, int y);
 
+	/// Helper function: find row element that contains pos, and
+	/// compute x offset.
+	Row::const_iterator const
+	findRowElement(Row const & row, pos_type const pos,
+                   bool const boundary, double & x) const;
+
 	///
-	int cursorX(CursorSlice const & cursor,
-		bool boundary) const;
+	int cursorX(CursorSlice const & cursor, bool boundary) const;
 	///
-	int cursorY(CursorSlice const & cursor,
-		bool boundary) const;
+	int cursorY(CursorSlice const & cursor, bool boundary) const;
 
 	///
 	bool cursorHome(Cursor & cur);