Implement proper handling of RtL in Rows

Now the row elements are sorted according to RtL/LtR.

Some additional cleanup.

00README_STR_METRICS_BRANCH

@@ -9,6 +9,7 @@ What is done:
 
 * change breakRow operation to operate on text strings on which
   metrics are computed. The list of elements is stored in the row object
+  in visual ordering, not logical.
 
 * Implement proper string metrics computation (with cache), when
   lyxrc.force_paint_single_char is false.
@@ -26,13 +27,15 @@ Next steps:
 * profile and see how performance can be improved.
 
 Difference in behavior
-* words longer than the screen are no monger broken at an arbitrary
-point. This will not be useful anymore with horizontal scrolling.
 * end of paragraph markers metrics are computed with the font of the
-actual text, not default font. This will be extended to the other
-methods.
+  actual text, not default font. This will be extended to the other
+  methods.
 
 Other differences that should be considered as bugs
+* words longer than the screen are no monger broken at an arbitrary
+  point. This is a problem for languages like chinese that do not use
+  separators.
+
 * there are still some difference in width computation wrt
   TextMetrics::rowWidth. This happens in particular with Description
   environment when the row is broken at bodypos. The method rowWidth

src/Row.cpp

@@ -130,16 +130,16 @@ ostream & operator<<(ostream & os, Row const & row)
 	Row::Elements::const_iterator it = row.elements_.begin();
 	for ( ; it != row.elements_.end() ; ++it) {
 		switch (it->type) {
-		case Row::Element::STRING_ELT:
+		case Row::Element::STRING:
 			os << "**STRING: " << to_utf8(it->str) << endl;
 			break;
-		case Row::Element::INSET_ELT:
+		case Row::Element::INSET:
 			os << "**INSET: " << to_utf8(it->inset->layoutName()) << endl;
 			break;
-		case Row::Element::SEPARATOR_ELT:
+		case Row::Element::SEPARATOR:
 			os << "**SEPARATOR: " << endl;
 			break;
-		case Row::Element::SPACE_ELT:
+		case Row::Element::SPACE:
 			os << "**SPACE: " << it->dim.wid << endl;
 			break;
 		}
@@ -153,7 +153,7 @@ bool Row::sameString(Font const & f, Change const & ch) const
 	if (elements_.empty())
 		return false;
 	Element const & elt = elements_.back();
-	return elt.type == Element::STRING_ELT && !elt.final
+	return elt.type == Element::STRING && !elt.final
 		   && elt.font == f && elt.change == ch;
 }
 
@@ -167,18 +167,18 @@ void Row::finalizeLast()
 		return;
 	elt.final = true;
 
-	if (elt.type == Element::STRING_ELT) {
+	if (elt.type == Element::STRING) {
 		elt.dim.wid = theFontMetrics(elt.font).width(elt.str);
 		dim_.wid += elt.dim.wid;
 	}
 }
 
 
-void Row::add(pos_type const pos, Inset const * ins, Dimension const & dim)
+void Row::add(pos_type const pos, Inset const * ins, Dimension const & dim,
+	      Font const & f, Change const & ch)
 {
 	finalizeLast();
-	Element e(Element::INSET_ELT);
-	e.pos = pos;
+	Element e(Element::INSET, pos, f, ch);
 	e.inset = ins;
 	e.dim = dim;
 	elements_.push_back(e);
@@ -186,27 +186,30 @@ void Row::add(pos_type const pos, Inset const * ins, Dimension const & dim)
 }
 
 
-void Row::add(pos_type const pos, docstring const & s,
-	      Font const & f, Change const & ch)
-{
-	if (sameString(f, ch))
-		elements_.back().str += s;
-	else {
-		finalizeLast();
-		Element e(Element::STRING_ELT);
-		e.pos = pos;
-		e.str = s;
-		e.font = f;
-		e.change = ch;
-		elements_.push_back(e);
-	}
-}
-
-
 void Row::add(pos_type const pos, char_type const c,
 	      Font const & f, Change const & ch)
 {
-	add(pos, docstring(1,c), f, ch);
+	if (!sameString(f, ch)) {
+		finalizeLast();
+		Element e(Element::STRING, pos, f, ch);
+		elements_.push_back(e);
+	}
+	//lyxerr << "FONT " <<back().font.language() << endl;
+	back().str += c;
+	back().endpos = pos + 1;
+}
+
+
+void Row::add(pos_type const pos, docstring const & s,
+	      Font const & f, Change const & ch)
+{
+	if (!sameString(f, ch)) {
+		finalizeLast();
+		Element e(Element::STRING, pos, f, ch);
+		elements_.push_back(e);
+	}
+	back().str += s;
+	back().endpos = pos + 1;
 }
 
 
@@ -214,22 +217,19 @@ void Row::addSeparator(pos_type const pos, char_type const c,
 		       Font const & f, Change const & ch)
 {
 	finalizeLast();
-	Element e(Element::SEPARATOR_ELT);
-	e.pos = pos;
+	Element e(Element::SEPARATOR, pos, f, ch);
 	e.str += c;
-	e.font = f;
-	e.change = ch;
 	e.dim.wid = theFontMetrics(f).width(c);
 	elements_.push_back(e);
 	dim_.wid += e.dim.wid;
 }
 
 
-void Row::addSpace(pos_type pos, int width)
+void Row::addSpace(pos_type const pos, int const width,
+		   Font const & f, Change const & ch)
 {
 	finalizeLast();
-	Element e(Element::SEPARATOR_ELT);
-	e.pos = pos;
+	Element e(Element::SEPARATOR, pos, f, ch);
 	e.dim.wid = width;
 	elements_.push_back(e);
 	dim_.wid += e.dim.wid;
@@ -268,4 +268,25 @@ void Row::separate_back(pos_type const keep)
 	elements_.erase(elements_.begin() + i, elements_.end());
 }
 
+
+void Row::reverseRtL()
+{
+	pos_type i = 0;
+	pos_type const end = elements_.size();
+	while (i < end) {
+		// skip LtR elements
+		while (!elements_[i].font.isRightToLeft() && i < end)
+			++i;
+		if (i >= end)
+			break;
+
+		// look for a RtL sequence
+		pos_type j = i;
+		while (elements_[j].font.isRightToLeft() && j < end)
+			++j;
+		reverse(elements_.begin() + i, elements_.begin() + j);
+		i = j;
+	}
+}
+
 } // namespace lyx

src/Row.h

@@ -42,22 +42,25 @@ public:
  */
 	struct Element {
 		enum Type {
-			STRING_ELT,
-			SEPARATOR_ELT,
-			INSET_ELT,
-			SPACE_ELT
+			STRING,
+			SEPARATOR,
+			INSET,
+			SPACE
 		};
 
-		Element(Type const t) : type(t), pos(0), inset(0), 
-					final(false) {}
+		Element(Type const t, pos_type p, Font const & f, Change const & ch) 
+			: type(t), pos(p), endpos(p + 1), inset(0), final(false),
+			  font(f), change(ch) {}
 
 		//
-		bool isLineSeparator() const { return type == SEPARATOR_ELT; }
+		bool isLineSeparator() const { return type == SEPARATOR; }
 
 		// The kind of row element
 		Type type;
 		// position of the element in the paragraph
 		pos_type pos;
+		// first position after the element in the paragraph
+		pos_type endpos;
 		// The dimension of the chunk (only width for strings)
 		Dimension dim;
 
@@ -118,18 +121,19 @@ public:
 	int descent() const { return dim_.des; }
 
 	///
-	void add(pos_type pos, Inset const * ins, Dimension const & dim);
-	///
-	void add(pos_type pos, docstring const & s,
+	void add(pos_type pos, Inset const * ins, Dimension const & dim,
 		 Font const & f, Change const & ch);
 	///
 	void add(pos_type pos, char_type const c,
 		 Font const & f, Change const & ch);
 	///
+	void add(pos_type pos, docstring const & s,
+		 Font const & f, Change const & ch);
+	///
 	void addSeparator(pos_type pos, char_type const c,
 			  Font const & f, Change const & ch);
 	///
-	void addSpace(pos_type pos, int width);
+	void addSpace(pos_type pos, int width, Font const & f, Change const & ch);
 	///
 	bool empty() const { return elements_.empty(); }
 	///
@@ -153,10 +157,13 @@ public:
 	 */
 	void finalizeLast();
 
-	friend std::ostream & operator<<(std::ostream & os, Row const & row);
+	/**
+	 * Find sequences of RtL elements and reverse them.
+	 * This should be called once the row is completely built.
+	 */
+	void reverseRtL();
 
-	/// current debugging only
-	void dump(char const * = "") const;
+	friend std::ostream & operator<<(std::ostream & os, Row const & row);
 
 	/// width of a separator (i.e. space)
 	double separator;

src/TextMetrics.cpp

@@ -843,12 +843,11 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 	FontIterator fi = FontIterator(*this, par, pit, pos);
 	while (i < end && row.width() < width) {
 		char_type c = par.getChar(i);
-		Language const * language = fi->language();
 		// The most special cases are handled first.
 		if (par.isInset(i)) {
 			Inset const * ins = par.getInset(i);
 			Dimension dim = pm.insetDimension(ins);
-			row.add(i, ins, dim);
+			row.add(i, ins, dim, *fi, par.lookupChange(i));
 		} else if (par.isLineSeparator(i)) {
 			// In theory, no inset has this property. If
 			// this is done, a new addSeparator which
@@ -857,19 +856,8 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 			LATTEST(!par.isInset(i));
 			row.addSeparator(i, c, *fi, par.lookupChange(i));
 		} else if (c == '\t')
-			row.add(i, from_ascii("    "), *fi, par.lookupChange(i));
-		else if (language->rightToLeft()) {
-			if (language->lang() == "arabic_arabtex" ||
-			    language->lang() == "arabic_arabi" ||
-			    language->lang() == "farsi") {
-				if (!Encodings::isArabicComposeChar(c))
-					row.add(i, par.transformChar(c, i),
-						*fi, par.lookupChange(i));
-			} else if (language->lang() == "hebrew" &&
-				   !Encodings::isHebrewComposeChar(c)) {
-				row.add(i, c, *fi, par.lookupChange(i));
-			}
-		} else
+			row.addSpace(i, theFontMetrics(*fi).width(from_ascii("    ")), *fi, par.lookupChange(i));
+		else
 			row.add(i, c, *fi, par.lookupChange(i));
 
 		// end of paragraph marker
@@ -913,7 +901,7 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 				row.pop_back();
 			int const add = max(fm.width(par.layout().labelsep),
 					    labelEnd(pit) - row.width());
-			row.addSpace(i, add);
+			row.addSpace(i, add, *fi, par.lookupChange(i));
 		}
 
 	}
@@ -930,6 +918,11 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 	    && row.endpos() < par.size())
 		row.pop_back();
 
+	// make sure that the RtL elements are in reverse ordering
+	lyxerr << ">>>>>>>>>>BEFORE REVERSE" << row;
+	row.reverseRtL();
+	lyxerr << "<<<<<<<<<<AFTER REVERSE" << row;
+
 	row.dimension().wid += right_margin;
 
 	// manual labels cannot be broken in LaTeX. But we