Fix positionning of cursor

The old implementation of Row::Element::pos2x and x2pos did not work
correctly with Arabic text, because characters can have shapes that
depend on context.

This new implementation leverages QTextLayout in a simplified way,
since only one word is added to the layout.

This allows to make Row::Element::x2pos more readable.

Fixes: #9115.

src/Row.cpp

@@ -36,78 +36,65 @@ using frontend::FontMetrics;
 
 double Row::Element::pos2x(pos_type const i) const
 {
-	LASSERT(i >= pos && i <= endpos, return 0);
+	// This can happen with inline completion when clicking on the
+	// row after the completion.
+	if (i < pos || i > endpos)
+		return 0;
 
 	bool const rtl = font.isVisibleRightToLeft();
 
 	int w = 0;
 	//handle first the two bounds of the element
-	if (i == pos)
-		w = 0;
+	if (i == pos || type != STRING)
+		w = rtl ? width() : 0;
 	else if (i == endpos)
-		w = width();
+		w = rtl ? 0 : width();
 	else {
-		LASSERT(type == STRING, return 0);
 		FontMetrics const & fm = theFontMetrics(font);
-		// FIXME Avoid caching of metrics there?
-		w = fm.width(str.substr(0, i - pos));
+		w = fm.pos2x(str, i - pos, font.isVisibleRightToLeft());
 	}
 
-	if (rtl)
-		return width() - w;
-	else
-		return w;
+	return w;
 }
 
 
-pos_type Row::Element::x2pos(double &x, bool const low) const
+pos_type Row::Element::x2pos(double &x) const
 {
 	//lyxerr << "x2pos: x=" << x << " w=" << width() << " " << *this;
-	// If element is rtl, flip x value
 	bool const rtl = font.isVisibleRightToLeft();
-	double x2 = rtl ? (width() - x) : x;
-
-	double last_w = 0;
-	double w = 0;
 	size_t i = 0;
+
 	switch (type) {
-	case VIRTUAL:
-		// those elements are actually empty (but they have a width)
-		break;
 	case STRING: {
 		FontMetrics const & fm = theFontMetrics(font);
-		// FIXME: implement dichotomy search?
-		for ( ; i < str.size() ; ++i) {
-			last_w = w;
-			w = fm.width(str.substr(0, i + 1));
-			if (w > x2)
-				break;
-		}
+		// FIXME: is it really necessary for x to be a double?
+		int xx = x;
+		i = fm.x2pos(str, xx, rtl);
+		x = xx;
 		break;
 	}
+	case VIRTUAL:
+		// those elements are actually empty (but they have a width)
+		i = 0;
+		x = rtl ? width() : 0;
+		break;
 	case SEPARATOR:
 	case INSET:
 	case SPACE:
-		// those elements contain only one position
-		w = width();
+		// those elements contain only one position. Round to
+		// the closest side.
+		if (x > width()) {
+			x = width();
+			i = !rtl;
+		} else {
+			x = 0;
+			i = rtl;
+		}
+
 	}
-
-	if (type == STRING && i == str.size())
-		x2 = w;
-	// round to the closest side. The !rtl is here to obtain the
-	// same rounding as with the old code (this is cosmetic and
-	// can be eventually removed).
-	else if (type != VIRTUAL && !low && (x2 - last_w + !rtl > w - x2)) {
-		x2 = w;
-		++i;
-	} else
-		x2 = last_w;
-
-	// is element is rtl, flip values back
-	x = rtl ? width() - x2 : x2;
-
 	//lyxerr << "=> p=" << pos + i << " x=" << x << endl;
 	return pos + i;
+
 }
 
 
@@ -389,7 +376,8 @@ void Row::shorten_if_needed(pos_type const keep, int const w)
 		// If there is a paragraph marker, it should be taken in account
 		if (elements_.size() == 2)
 			xstr -= back().width();
-		pos_type new_pos = front.x2pos(xstr, true);
+		//FIXME: use FontMetrics::x2pos here?? handle rtl?
+		pos_type new_pos = front.x2pos(xstr);
 		front.str = front.str.substr(0, new_pos - pos_);
 		front.dim.wid = xstr;
 		front.endpos = new_pos;

src/Row.h

@@ -71,11 +71,9 @@ public:
 
 		/** Return character position that is the closest to
 		 *  pixel position \param x. The value \param x is
-		 *  rounded to the actual pixel position. If \param
-		 *  short is true, the pixel value is rounded by
-		 *  default.
+		 *  adjusted to the actual pixel position.
 		*/
-		pos_type x2pos(double &x, bool low = false) const;
+		pos_type x2pos(double &x) const;
 
 		// Returns the position on left side of the element.
 		pos_type left_pos() const;

src/frontends/FontMetrics.h

@@ -77,6 +77,19 @@ public:
 	virtual int width(docstring const & s) const = 0;
 	/// FIXME ??
 	virtual int signedWidth(docstring const & s) const = 0;
+	/**
+	 * return the x offset of a position in the string. The
+	 * direction of the string is forced, and the returned value
+	 * is from the left edge of the word, not from the start of the string.
+	 */
+	virtual int pos2x(docstring const & s, int pos, bool rtl) const = 0;
+	/**
+	 * return the position in the string for a given x offset. The
+	 * direction of the string is forced, and the returned value
+	 * is from the left edge of the word, not from the start of the string.
+	 * the offset x is updated to match the closest position in the string.
+	 */
+	virtual int x2pos(docstring const & s, int & x, bool rtl) const = 0;
 	/// return char dimension for the font.
 	virtual Dimension const dimension(char_type c) const = 0;
 	/**

src/frontends/qt4/GuiFontMetrics.cpp

@@ -23,6 +23,8 @@
 
 #include "support/lassert.h"
 
+#include <QTextLayout>
+
 using namespace std;
 
 namespace lyx {
@@ -50,7 +52,7 @@ inline QChar const ucs4_to_qchar(char_type const ucs4)
 } // anon namespace
 
 
-GuiFontMetrics::GuiFontMetrics(QFont const & font) : metrics_(font, 0)
+GuiFontMetrics::GuiFontMetrics(QFont const & font) : font_(font), metrics_(font, 0)
 {
 }
 
@@ -140,6 +142,58 @@ int GuiFontMetrics::signedWidth(docstring const & s) const
 		return width(s);
 }
 
+namespace {
+void setTextLayout(QTextLayout & tl, docstring const & s, QFont const & font,
+			     bool const rtl)
+{
+	QString qs;
+	/* In LyX, the character direction is forced by the language.
+	 * Therefore, we have to signal that fact to Qt.
+	 * Source: http://www.iamcal.com/understanding-bidirectional-text/
+	 */
+	// Left-to-right override: forces to draw text left-to-right
+	char_type const LRO = 0x202D;
+	// Right-to-left override: forces to draw text right-to-left
+	char_type const RLO = 0x202E;
+	// Pop directional formatting: return to previous state
+	char_type const PDF = 0x202C;
+	if (rtl)
+		qs = toqstr(RLO + s + PDF);
+	else
+		qs = toqstr(LRO + s + PDF);
+
+	tl.setText(qs);
+	tl.setFont(font);
+	tl.beginLayout();
+	tl.createLine();
+	tl.endLayout();
+}
+}
+
+
+int GuiFontMetrics::pos2x(docstring const & s, int const pos, bool const rtl) const
+{
+	QTextLayout tl;
+	setTextLayout(tl, s, font_, rtl);
+	return tl.lineForTextPosition(pos + 1).cursorToX(pos + 1);
+}
+
+
+int GuiFontMetrics::x2pos(docstring const & s, int & x, bool const rtl) const
+{
+	QTextLayout tl;
+	setTextLayout(tl, s, font_, rtl);
+	int pos = tl.lineForTextPosition(0).xToCursor(x);
+	// take into account the unicode formatting characters
+	if (pos > 0)
+		--pos;
+	if (pos > int(s.length()))
+		pos = s.length();
+	// correct x value to the actual cursor position.
+	x = tl.lineForTextPosition(0).cursorToX(pos + 1);
+	return pos;
+}
+
 
 void GuiFontMetrics::rectText(docstring const & str,
 	int & w, int & ascent, int & descent) const

src/frontends/qt4/GuiFontMetrics.h

@@ -18,6 +18,7 @@
 
 #include <map>
 
+#include <QFont>
 #include <QFontMetrics>
 #include <QHash>
 
@@ -41,6 +42,8 @@ public:
 	virtual int rbearing(char_type c) const;
 	virtual int width(docstring const & s) const;
 	virtual int signedWidth(docstring const & s) const;
+	virtual int pos2x(docstring const & s, int pos, bool rtl) const;
+	virtual int x2pos(docstring const & s, int & x, bool rtl) const;
 	virtual Dimension const dimension(char_type c) const;
 
 	virtual void rectText(docstring const & str,
@@ -55,6 +58,9 @@ public:
 	int width(QString const & str) const;
 
 private:
+	/// The font
+	QFont font_;
+
 	/// Metrics on the font
 	QFontMetrics metrics_;