Some fixes related to RTL text

* fix handling of boundary situations in Row::Elements::x2pos;

 * fix handling of boundary situations in TextMetrics::getColumnNearX;

 * make sure to always use Font::isVisibleRightToLeft instead of Font::isRightToLeft;

 * Improve debug messages.

00README_STR_METRICS_BRANCH

@@ -24,21 +24,26 @@ What is done:
   useless workarounds which disable kerning and ligatures.
 
 
-Next steps needed:
+Next steps:
 
 * check what happens with arabic and/or hebrew text. There may be some
   problems related to compose characters. I suspect that some code is
   needed in FontMetrics::width.
 
-Next possible steps:
-
 * Get rid of old code in cursorX and getColumnNearX; it has been
   kept for comparison purpose, guarded with KEEP_OLD_METRICS_CODE in
   order to check computations.
 
+* rename getColumnNearX to getPosNearX or x2pos (and change code
+  accordingly). It does not make sense to return a position relative
+  to the start of row, since nobody needs this.
+
 * Re-implement row painting using row elements. This is not difficult
   in principle, but the code is intricate and needs some careful
-  analysis.
+  analysis. First thing that needs to be done is to break row elements
+  with the same criterions. Currently TextMetrics::breakRow does not
+  consider on-the-fly spellchecking and selection changes, but it is
+  not clear to me that it is required.
 
 * Profile and see how performance can be improved.
 
@@ -56,4 +61,7 @@ Other differences (aka real bugs)
 
 * there are still difference in what breaks words. In particular,
   RowPainter breaks strings at: selection end, spellchecking
-  extremity.
+  extremities.
+
+* when clicking in the right margin, GetColumnNearX does not return
+  the same value as before.

src/Row.cpp

@@ -64,27 +64,26 @@ pos_type Row::Element::x2pos(double &x, bool const low) const
 	FontMetrics const & fm = theFontMetrics(font);
 	double last_w = 0;
 	double w = 0;
-	size_t i = 1;
+	size_t i = 0;
 	// non-STRING element only contain one position
 	if (type != STRING) {
-		i = 0;
 		w = width();
 	} else {
 		// FIXME: implement dichotomy search?
-		for ( ; i <= str.size() ; ++i) {
+		for ( ; i < str.size() ; ++i) {
 			last_w = w;
-			w = fm.width(str.substr(0,i));
-			if (w > x2) {
-				--i;
+			w = fm.width(str.substr(0, i + 1));
+			if (w > x2)
 				break;
-			}
 		}
 		// if (i == str.size())
 		// 	lyxerr << " NOT FOUND ";
 	}
 
+	if (i == str.size())
+		x2 = w;
 	// round to the closest side
-	if (!low && (x2 - last_w > w - x2)) {
+	else if (!low && (x2 - last_w > w - x2)) {
 		x2 = w;
 		++i;
 	} else
@@ -214,6 +213,7 @@ ostream & operator<<(ostream & os, Row const & row)
 	os << " pos: " << row.pos_ << " end: " << row.end_
 	   << " x: " << row.x
 	   << " width: " << row.dim_.wid
+	   << " right_margin: " << row.right_margin
 	   << " ascent: " << row.dim_.asc
 	   << " descent: " << row.dim_.des
 	   << " separator: " << row.separator

src/Row.h

@@ -186,6 +186,10 @@ public:
 	///
 	bool empty() const { return elements_.empty(); }
 	///
+	Element & front() { return elements_.front(); }
+	///
+	Element const & front() const { return elements_.front(); }
+	///
 	Element & back() { return elements_.back(); }
 	///
 	Element const & back() const { return elements_.back(); }

src/TextMetrics.cpp

@@ -15,7 +15,7 @@
  * Full author contact details are available in file CREDITS.
  */
 
-//#define KEEP_OLD_METRICS_CODE 1
+#define KEEP_OLD_METRICS_CODE 1
 
 #include <config.h>
 
@@ -810,8 +810,8 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 	int const width = max_width_ - right_margin;
 	pos_type const body_pos = par.beginOfBody();
 	row.clear();
-	row.dimension().wid = leftMargin(max_width_, pit, pos);
-	row.x = row.width();
+	row.x = leftMargin(max_width_, pit, pos);
+	row.dimension().wid = row.x;
 	row.right_margin = right_margin;
 
 	if (pos >= end || row.width() > width) {
@@ -1129,11 +1129,16 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit,
 
 	pos_type pos = row.pos();
 	boundary = false;
-	if (row.x >= x || row.empty())
+	if (row.empty())
 		x = row.x;
-	else if (x >= row.width() - row.right_margin) {
+	else if (x < row.x) {
+		pos = row.front().font.isVisibleRightToLeft() ?
+			row.front().endpos : row.front().pos;
+		x = row.x;
+	} else if (x > row.width() - row.right_margin) {
+		pos = row.back().font.isVisibleRightToLeft() ?
+			row.back().pos : row.back().endpos;
 		x = row.width() - row.right_margin;
-		pos = row.back().endpos;
 	} else {
 		double w = row.x;
 		Row::const_iterator cit = row.begin();
@@ -1170,9 +1175,7 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit,
 		boundary = row.right_boundary();
 
 	x += xo;
-#if !defined(KEEP_OLD_METRICS_CODE)
-	return pos - row.pos();
-#else
+#ifdef KEEP_OLD_METRICS_CODE
 	Buffer const & buffer = bv_->buffer();
 
 	int x2 = x_orig;
@@ -1284,14 +1287,15 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit,
 #endif
 
 	x2 = int(tmpx) + xo;
-	pos_type const col = c - row.pos();
+	//pos_type const col = c - row.pos();
 
 	if (abs(x2 - x) > 0.1 || boundary != boundary
 	    || c != pos) {
-		lyxerr << "getColumnNearX: new=(x=" << x - xo << ", b=" << boundary << ", p=" << pos << "), "
+		lyxerr << "getColumnNearX(" << x_orig << "): new=(x=" << x - xo << ", b=" << boundary << ", p=" << pos << "), "
 		       << "old=(x=" << x2 - xo << ", b=" << boundary2 << ", p=" << c << "), " << row;
 	}
 
+#if 0
 	if (!c || end == par.size())
 		return col;
 
@@ -1301,7 +1305,9 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit,
 	}
 
 	return min(col, end - 1 - row.pos());
-#endif
+#endif // 0
+#endif // KEEP_OLD_METRICS_CODE
+	return pos - row.pos();
 }
 
 
@@ -1617,7 +1623,7 @@ int TextMetrics::cursorX(CursorSlice const & sl,
 	int const boundary_corr = (boundary && pos) ? -1 : 0;
 
 	if (row.empty()
-	    || (row.begin()->font.isRightToLeft()
+	    || (row.begin()->font.isVisibleRightToLeft()
 		&& pos == row.begin()->endpos))
 		return int(x);
 
@@ -1632,10 +1638,9 @@ int TextMetrics::cursorX(CursorSlice const & sl,
 	}
 
 	if (cit == row.end()
-	    && (row.back().font.isRightToLeft() || pos != row.back().endpos))
-		lyxerr << "NOT FOUND!"
-		       << "pos=" << pos << "(" << boundary_corr << ")" << "\n"
-		       << row;
+	    && (row.back().font.isVisibleRightToLeft() || pos != row.back().endpos))
+		LYXERR0("cursorX(" << pos - boundary_corr
+			<< ", " << boundary_corr << "): NOT FOUND! " << row);
 
 #ifdef KEEP_OLD_METRICS_CODE
 	Paragraph const & par = text_->paragraphs()[pit];
@@ -1774,18 +1779,16 @@ int TextMetrics::cursorX(CursorSlice const & sl,
 	}
 
 	if (abs(x2 - x) > 0.01) {
-		lyxerr << "cursorX: x2=" << x2 << ", x=" << x;
+		lyxerr << "cursorX(" << pos - boundary_corr << ", " << boundary_corr
+		       << "): old=" << x2 << ", new=" << x;
 		if (cit == row.end())
-			lyxerr << "Element not found for "
-			       << pos - boundary_corr << "(" << boundary_corr << ")";
+			lyxerr << "Element not found\n";
 		else
-			lyxerr << " in [" << cit->pos << "/"
-			       << pos - boundary_corr << "(" << boundary_corr << ")"
-			       << "/" << cit->endpos << "] of " << *cit << "\n";
+			lyxerr << " found in " << *cit << "\n";
 		lyxerr << row <<endl;
 	}
-#endif
 
+#endif // KEEP_OLD_METRICS_CODE
 	return int(x);
 }