Better algorithm for forcing bidi drawing

The use of RLO/LRO overrides to force text orientation was really hackish and the way it was done caused dropped letters in Mac OS X (for some unknown reasons).

This new approach is much cleaner, except that it relies on features not advertised in documentation
but present at least from Qt 4.5 to Qt 5.3:
* TextFlag enum values TextForceLeftToRight and TextForceRightToLeft, which are strong versions
  of QPainter::setLayoutDirection; they are passed as a parameter of QPainter::drawText.

* QTextLayout::setFlags method, which is required to pass the above flags to QTextLayout.

The unicode override method is still used to draw strings Mac OS X because, for some reason, the direction was not really enforced in this case.

src/frontends/Painter.h

@@ -109,10 +109,10 @@ public:
 		graphics::Image const & image) = 0;
 
 	/** draw a string at position x, y (y is the baseline). The
-	 * text direction is deduced from \c str.
+	 * text direction is given by \c rtl.
 	 * \return the width of the drawn text.
 	 */
-	virtual int text(int x, int y, docstring const & str, FontInfo const & f) = 0;
+	virtual int text(int x, int y, docstring const & str, FontInfo const & f, bool rtl = false) = 0;
 
 	/** draw a string at position x, y (y is the baseline). The
 	 * text direction is enforced by the \c Font.

src/frontends/qt4/GuiFontMetrics.cpp

@@ -22,7 +22,6 @@
 #include "insets/Inset.h"
 
 #include "support/lassert.h"
-#include "support/lstrings.h"
 
 #include <QTextLayout>
 
@@ -148,9 +147,10 @@ namespace {
 void setTextLayout(QTextLayout & tl, docstring const & s, QFont const & font,
 			     bool const rtl)
 {
-	QString const qs = toqstr(directedString(s, rtl));
+	tl.setText(toqstr(s));
-	tl.setText(qs);
 	tl.setFont(font);
+	// Note that both setFlags and the enums are undocumented
+	tl.setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight);
 	tl.beginLayout();
 	tl.createLine();
 	tl.endLayout();
@@ -162,8 +162,7 @@ int GuiFontMetrics::pos2x(docstring const & s, int const pos, bool const rtl) co
 {
 	QTextLayout tl;
 	setTextLayout(tl, s, font_, rtl);
-	// we take into account the unicode formatting characters
+	return tl.lineForTextPosition(pos).cursorToX(pos);
-	return tl.lineForTextPosition(pos + 1).cursorToX(pos + 1);
 }
 
 
@@ -172,13 +171,8 @@ 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);
+	x = tl.lineForTextPosition(0).cursorToX(pos);
 	return pos;
 }
 

src/frontends/qt4/GuiPainter.cpp

@@ -9,6 +9,10 @@
  * Full author contact details are available in file CREDITS.
  */
 
+#ifdef Q_WS_MAC
+#define USE_RTL_OVERRIDE 1
+#endif
+
 #include <config.h>
 
 #include "GuiPainter.h"
@@ -27,7 +31,6 @@
 #include "insets/Inset.h"
 
 #include "support/lassert.h"
-#include "support/lstrings.h"
 #include "support/debug.h"
 
 #include <QPixmapCache>
@@ -278,7 +281,7 @@ int GuiPainter::text(int x, int y, char_type c, FontInfo const & f)
 
 
 int GuiPainter::text(int x, int y, docstring const & s,
-		FontInfo const & f)
+		     FontInfo const & f, bool const rtl)
 {
 	//LYXERR0("text: x=" << x << ", s=" << s);
 	if (s.empty())
@@ -304,8 +307,8 @@ int GuiPainter::text(int x, int y, docstring const & s,
 		str = ' ' + str;
 #endif
 
-	QFont const & ff = getFont(f); 
+	QFont const & ff = getFont(f);
-	GuiFontMetrics const & fm = getFontMetrics(f); 
+	GuiFontMetrics const & fm = getFontMetrics(f);
 
 	// Here we use the font width cache instead of
 	//   textwidth = fontMetrics().width(str);
@@ -390,7 +393,32 @@ int GuiPainter::text(int x, int y, docstring const & s,
 	setQPainterPen(computeColor(f.realColor()));
 	if (font() != ff)
 		setFont(ff);
+
+	 /* In LyX, the character direction is forced by the language.
+	  * Therefore, we have to signal that fact to Qt.
+	  */
+#ifdef USE_RTL_OVERRIDE
+	/* Use unicode override characters to enforce drawing direction
+	 * Source: http://www.iamcal.com/understanding-bidirectional-text/
+	 */
+	if (rtl)
+		// Right-to-left override: forces to draw text right-to-left
+		str = QChar(0x202E) + str;
+	else
+		// Left-to-right override: forces to draw text left-to-right
+		str =  QChar(0x202D) + str;
 	drawText(x, y, str);
+#else
+	/* This is a cleanr solution, but it has two drawbacks
+	 * - it seems that it does not work under Mac OS X
+	 * - it is not really documented
+	 */
+	//This is much stronger than setLayoutDirection.
+	int flag = rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight;
+	drawText(x + (rtl ? textwidth : 0), y - fm.maxAscent(), 0, 0,
+		 flag | Qt::TextDontClip,
+		 str);
+#endif
 	//LYXERR(Debug::PAINTING, "draw " << string(str.toUtf8())
 	//	<< " at " << x << "," << y);
 	return textwidth;
@@ -399,8 +427,7 @@ int GuiPainter::text(int x, int y, docstring const & s,
 
 int GuiPainter::text(int x, int y, docstring const & str, Font const & f)
 {
-	docstring const dstr = directedString(str, f.isVisibleRightToLeft());
+	return text(x, y, str, f.fontInfo(), f.isVisibleRightToLeft());
-	return text(x, y, dstr, f.fontInfo());
 }
 
 
@@ -410,7 +437,6 @@ int GuiPainter::text(int x, int y, docstring const & str, Font const & f,
 	GuiFontMetrics const & fm = getFontMetrics(f.fontInfo());
 	FontInfo fi = f.fontInfo();
 	bool const rtl = f.isVisibleRightToLeft();
-	docstring const dstr = directedString(str, rtl);
 
 	// dimensions
 	int const ascent = fm.maxAscent();
@@ -424,15 +450,15 @@ int GuiPainter::text(int x, int y, docstring const & str, Font const & f,
 	Color const orig = fi.realColor();
 	fi.setPaintColor(other);
 	setClipRect(QRect(x + xmin, y - ascent, xmax - xmin, height));
-	int const textwidth = text(x, y, dstr, fi);
+	int const textwidth = text(x, y, str, fi, rtl);
 
 	// Then the part in normal color
 	// Note that in Qt5, it is not possible to use Qt::UniteClip
 	fi.setPaintColor(orig);
 	setClipRect(QRect(x, y - ascent, xmin, height));
-	text(x, y, dstr, fi);
+	text(x, y, str, fi, rtl);
 	setClipRect(QRect(x + xmax, y - ascent, textwidth - xmax, height));
-	text(x, y, dstr, fi);
+	text(x, y, str, fi, rtl);
 	setClipping(false);
 
 	return textwidth;

src/frontends/qt4/GuiPainter.h

@@ -87,10 +87,10 @@ public:
 		lyx::graphics::Image const & image);
 
 	/** draw a string at position x, y (y is the baseline). The
-	 * text direction is deduced from \c str.
+	 * text direction is given by \c rtl.
 	 * \return the width of the drawn text.
 	 */
-	virtual int text(int x, int y, docstring const & str, FontInfo const & f);
+	virtual int text(int x, int y, docstring const & str, FontInfo const & f, bool rtl = false);
 
 	/** draw a string at position x, y (y is the baseline). The
 	 * text direction is enforced by the \c Font.

src/support/lstrings.cpp

@@ -1502,23 +1502,5 @@ docstring bformat(docstring const & fmt,
 	return subst(str, from_ascii("%%"), from_ascii("%"));
 }
 
-
-docstring directedString(docstring const & s, bool const rtl)
-{
-	/* 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;
-	return (rtl ? RLO : LRO) + s + PDF;
-}
-
-
-
 } // namespace support
 } // namespace lyx

src/support/lstrings.h

@@ -327,9 +327,6 @@ template<> docstring bformat(docstring const & fmt, int arg1, int arg2);
 template<> docstring bformat(docstring const & fmt, docstring arg1, docstring arg2, docstring arg3);
 template<> docstring bformat(docstring const & fmt, docstring arg1, docstring arg2, docstring arg3, docstring arg4);
 
-/// Return a string with Unicode overrides to enforce the writing direction
-docstring directedString(docstring const & s, bool rtl);
-
 
 } // namespace support
 } // namespace lyx