Do not split words at selection boundary

The display of partially-selected word is now done in a new Painter::text method
which displays the string twice with different clip settings. This allows to
catter for the case where Color_selectiontext is not black.

Morover, the code that uses unicode override characters to force the
direction of a string is moved to lstrings.h.

Fixes: #9116

00README_STR_METRICS_BRANCH

@@ -47,6 +47,10 @@ What is done:
   justified. This speeds-up painting by reducing the number of strings
   to draw.
 
+* Do not cut strings at selection boundary in RowPainter. This avoids
+  ligature/kerning breaking in latin text, and bad rendering problems
+  in Arabic.
+
 
 Next steps:
 
@@ -72,5 +76,3 @@ Steps for later (aka out of the scope of this branch):
   happens. This depends on the update machinery.
 
   This would allow to get rid of the Bidi.cpp code.
-
-

src/frontends/Painter.h

@@ -14,9 +14,11 @@
 #define PAINTER_H
 
 #include "support/strfwd.h"
+#include "support/types.h"
 
 namespace lyx {
 
+class Font;
 class FontInfo;
 
 namespace graphics { class Image; }
@@ -106,12 +108,26 @@ public:
 	virtual void image(int x, int y, int w, int h,
 		graphics::Image const & image) = 0;
 
-	/// draw a string at position x, y (y is the baseline)
+	/** draw a string at position x, y (y is the baseline). The
-	/**
+	 * text direction is deduced from \c str.
-	* \return the width of the drawn text.
+	 * \return the width of the drawn text.
-	*/
+	 */
 	virtual int text(int x, int y, docstring const & str, FontInfo const & f) = 0;
 
+	/** draw a string at position x, y (y is the baseline). The
+	 * text direction is enforced by the \c Font.
+	 * \return the width of the drawn text.
+	 */
+	virtual int text(int x, int y, docstring const & str, Font const & f) = 0;
+
+	/** draw a string at position x, y (y is the baseline), but
+	 * make sure that the part between \c from and \c to is in
+	 * \c other color. The text direction is enforced by the \c Font.
+	 * \return the width of the drawn text.
+	 */
+	virtual int text(int x, int y, docstring const & str, Font const & f,
+			 Color other, size_type from, size_type to) = 0;
+
 	void setDrawingEnabled(bool drawing_enabled)
 	{ drawing_enabled_ = drawing_enabled; }
 

src/frontends/qt4/GuiFontMetrics.cpp

@@ -22,10 +22,12 @@
 #include "insets/Inset.h"
 
 #include "support/lassert.h"
+#include "support/lstrings.h"
 
 #include <QTextLayout>
 
 using namespace std;
+using namespace lyx::support;
 
 namespace lyx {
 namespace frontend {
@@ -42,7 +44,7 @@ namespace {
  * why this works well for symbol fonts used in mathed too, even though
  * these are not real ucs4 characters. These are codepoints in the
  * computer modern fonts used, nothing unicode related.
- * See comment in QLPainter::text() for more explanation.
+ * See comment in GuiPainter::text() for more explanation.
  **/
 inline QChar const ucs4_to_qchar(char_type const ucs4)
 {
@@ -146,22 +148,7 @@ namespace {
 void setTextLayout(QTextLayout & tl, docstring const & s, QFont const & font,
 			     bool const rtl)
 {
-	QString qs;
+	QString const qs = toqstr(directedString(s, 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;
-	if (rtl)
-		qs = toqstr(RLO + s + PDF);
-	else
-		qs = toqstr(LRO + s + PDF);
-
 	tl.setText(qs);
 	tl.setFont(font);
 	tl.beginLayout();
@@ -175,6 +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 + 1).cursorToX(pos + 1);
 }
 

src/frontends/qt4/GuiPainter.cpp

@@ -20,13 +20,14 @@
 #include "GuiImage.h"
 #include "qt_helpers.h"
 
-#include "FontInfo.h"
+#include "Font.h"
 #include "Language.h"
 #include "LyXRC.h"
 
 #include "insets/Inset.h"
 
 #include "support/lassert.h"
+#include "support/lstrings.h"
 #include "support/debug.h"
 
 #include <QPixmapCache>
@@ -41,6 +42,7 @@
 #endif
 
 using namespace std;
+using namespace lyx::support;
 
 namespace lyx {
 namespace frontend {
@@ -305,17 +307,16 @@ int GuiPainter::text(int x, int y, docstring const & s,
 	QFont const & ff = getFont(f); 
 	GuiFontMetrics const & fm = getFontMetrics(f); 
 
-	int textwidth;
-
 	// Here we use the font width cache instead of
 	//   textwidth = fontMetrics().width(str);
 	// because the above is awfully expensive on MacOSX
-	textwidth = fm.width(s);
+	int const textwidth = fm.width(s);
-	textDecoration(f, x, y, textwidth);
 
 	if (!isDrawingEnabled())
 		return textwidth;
 
+	textDecoration(f, x, y, textwidth);
+
 	// Qt4 does not display a glyph whose codepoint is the
 	// same as that of a soft-hyphen (0x00ad), unless it
 	// occurs at a line-break. As a kludge, we force Qt to
@@ -389,8 +390,6 @@ int GuiPainter::text(int x, int y, docstring const & s,
 	setQPainterPen(computeColor(f.realColor()));
 	if (font() != ff)
 		setFont(ff);
-	// We need to draw the text as LTR as we use our own bidi code.
-	QPainter::setLayoutDirection(Qt::LeftToRight);
 	drawText(x, y, str);
 	//LYXERR(Debug::PAINTING, "draw " << string(str.toUtf8())
 	//	<< " at " << x << "," << y);
@@ -398,6 +397,48 @@ 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, dstr, f.fontInfo());
+}
+
+
+int GuiPainter::text(int x, int y, docstring const & str, Font const & f,
+		     Color other, size_type from, size_type to)
+{
+	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();
+	int const height = fm.maxAscent() + fm.maxDescent();
+	int xmin = fm.pos2x(str, from, rtl);
+	int xmax = fm.pos2x(str, to, rtl);
+	if (xmin > xmax)
+		swap(xmin, xmax);
+
+	// First the part in other color
+	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);
+
+	// 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);
+	setClipRect(QRect(x + xmax, y - ascent, textwidth - xmax, height));
+	text(x, y, dstr, fi);
+	setClipping(false);
+
+	return textwidth;
+}
+
+
 void GuiPainter::textDecoration(FontInfo const & f, int x, int y, int width)
 {
 	if (f.underbar() == FONT_ON)

src/frontends/qt4/GuiPainter.h

@@ -86,9 +86,25 @@ public:
 	virtual void image(int x, int y, int w, int h,
 		lyx::graphics::Image const & image);
 
-	/// draw a string at position x, y (y is the baseline)
+	/** draw a string at position x, y (y is the baseline). The
-	virtual int text(int x, int y,
+	 * text direction is deduced from \c str.
-		docstring const & str, FontInfo const & f);
+	 * \return the width of the drawn text.
+	 */
+	virtual int text(int x, int y, docstring const & str, FontInfo const & f);
+
+	/** draw a string at position x, y (y is the baseline). The
+	 * text direction is enforced by the \c Font.
+	 * \return the width of the drawn text.
+	 */
+	virtual int text(int x, int y, docstring const & str, Font const & f);
+
+	/** draw a string at position x, y (y is the baseline), but
+	 * make sure that the part between \c from and \c to is in
+	 * \c other color. The text direction is enforced by the \c Font.
+	 * \return the width of the drawn text.
+	 */
+	virtual int text(int x, int y, docstring const & str, Font const & f,
+			 Color other, size_type from, size_type to);
 
 	/// draw a char at position x, y (y is the baseline)
 	virtual int text(int x, int y, char_type c, FontInfo const & f);

src/rowpainter.cpp

@@ -164,6 +164,7 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font)
 {
 	// This method takes up 70% of time when typing
 	pos_type pos = bidi_.vis2log(vpos);
+	pos_type start_pos = pos;
 	// first character
 	char_type c = par_.getChar(pos);
 	docstring str;
@@ -190,10 +191,6 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font)
 	// Track-change status.
 	Change const & change_running = par_.lookupChange(pos);
 
-	// selected text?
-	bool const selection = (pos >= row_.sel_beg && pos < row_.sel_end)
-		|| pi_.selected;
-
 	// spelling correct?
 	bool const spell_state =
 		lyxrc.spellcheck_continuously && par_.isMisspelled(pos);
@@ -201,12 +198,8 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font)
 	// collect as much similar chars as we can
 	for (++vpos ; vpos < end ; ++vpos) {
 		pos = bidi_.vis2log(vpos);
-		if (pos < font_span.first || pos > font_span.last)
-			break;
 
-		bool const new_selection = pos >= row_.sel_beg && pos < row_.sel_end;
+		if (!font_span.inside(pos))
-		if (new_selection != selection)
-			// Selection ends or starts here.
 			break;
 
 		bool const new_spell_state =
@@ -225,6 +218,9 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font)
 		if (c == '\t')
 			break;
 
+		// When row_.separator == 0, it is possible to print a
+		// string longer than a word in one fell swoop.
+		// Therefore there is no need to break at spaces.
 		if (!isPrintableNonspace(c)
 		    && (c != ' ' || row_.separator > 0))
 			break;
@@ -241,6 +237,19 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font)
 		str.push_back(c);
 	}
 
+	// Make pos point to the last character in the string.
+	// Using "pos = bidi_.vis2log(vpos)" does not work for some reason.
+	if (vpos < end)
+		pos = bidi_.vis2log(vpos - 1);
+
+	// Now make pos point to the position _after_ the string.
+	// Using vis2log for that is not a good idea in general, we
+	// want logical ordering.
+	if (font.isVisibleRightToLeft())
+		--pos;
+	else
+		++pos;
+
 	if (str[0] == '\t')
 		str.replace(0,1,from_ascii("    "));
 
@@ -253,30 +262,36 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font)
 	 * See also http://thread.gmane.org/gmane.editors.lyx.devel/79740
 	 * for an earlier thread on the subject
 	 */
-	// 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 (font.isVisibleRightToLeft()) {
 		reverse(str.begin(), str.end());
-		str = RLO + str + PDF;
+		// If the string is reversed, the positions need to be adjusted
-	} else
+		++pos;
-		str = LRO + str + PDF;
+		++start_pos;
-
+		swap(start_pos, pos);
-	if (!selection && !change_running.changed()) {
-		x_ += pi_.pain.text(int(x_), yo_, str, font.fontInfo());
-		return;
 	} 
 
-	FontInfo copy = font.fontInfo();
+	// at least part of text selected?
-	if (change_running.changed())
+	bool const some_sel = (pos >= row_.sel_beg && start_pos < row_.sel_end)
-		copy.setPaintColor(change_running.color());
+		|| pi_.selected;
-	else if (selection)
+	// all the text selected?
-		copy.setPaintColor(Color_selectiontext);
+	bool const all_sel = (start_pos >= row_.sel_beg && pos < row_.sel_end)
+		|| pi_.selected;
 
-	x_ += pi_.pain.text(int(x_), yo_, str, copy);
+	if (all_sel) {
+		Font copy = font;
+		copy.fontInfo().setPaintColor(Color_selectiontext);
+		x_ += pi_.pain.text(int(x_), yo_, str, copy);
+	} else if (change_running.changed()) {
+		Font copy = font;
+		copy.fontInfo().setPaintColor(change_running.color());
+		x_ += pi_.pain.text(int(x_), yo_, str, copy);
+	} else if (!some_sel) {
+		x_ += pi_.pain.text(int(x_), yo_, str, font);
+	} else {
+		x_ += pi_.pain.text(int(x_), yo_, str, font, Color_selectiontext,
+				    max(row_.sel_beg, start_pos) - start_pos,
+				    min(row_.sel_end, pos) - start_pos);
+	}
 }
 
 

src/support/lstrings.cpp

@@ -1502,5 +1502,23 @@ 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,6 +327,9 @@ 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