Handle boundary in getColumnNearX (and more)

Use proper font everywhere for end-of-par marker

Fix getColumnNearX for RTL text and for centered/right-justified paragraphs.

Let computeRowMetrics update the row width.

00README_STR_METRICS_BRANCH

@@ -8,13 +8,12 @@ What is done:
   setRowHeight instead of rowBreakPoint and rowHeight.
 
 * 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.
+  metrics are computed. The list of elements is stored in the row
+  object in visual ordering, not logical.
 
 * re-implement cursorX using row elements
 
-* re-implement getColumnNearX using row elements (boundary is not
-  considered yet).
+* re-implement getColumnNearX using row elements.
 
 * Implement proper string metrics computation (with cache), when
   lyxrc.force_paint_single_char is false. In this case, remove also
@@ -23,7 +22,8 @@ What is done:
 Next steps:
 
 * get rid of old code of cursorX and getColumnNearX (which have been
-  kept for comparison purpose, guarded with KEEP_OLD_METRICS_CODE).
+  kept for comparison purpose, guarded with KEEP_OLD_METRICS_CODE in
+  order to check computations).
 
 * re-implement row painting using row elements (can it be done?)
 
@@ -31,17 +31,15 @@ Next steps:
 
 * Document the code
 
-Difference in behavior
-* 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.
+Difference in behavior (aka bug fixes)
 
-* When cursor is after a LtR separator just before a RtL chunk, the
+* end of paragraph markers metrics are computed with the font of the
+  actual text, not default font.
+
+* When cursor is after a LTR separator just before a RTL chunk, the
   cursor posiiton is computed better with the new code.
 
-Other differences that should be considered as bugs
+Other differences (aka real 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.
-
-* Boundary is not taken in account properly in getColumnNearX

src/Row.cpp

@@ -54,7 +54,7 @@ double Row::Element::pos2x(pos_type const i) const
 }
 
 
-pos_type Row::Element::x2pos(double &x) const
+	pos_type Row::Element::x2pos(double &x, bool const low) const
 {
 	//lyxerr << "x2pos: x=" << x << " w=" << width() << " " << *this;
 	// if element is rtl, flip x value
@@ -84,7 +84,7 @@ pos_type Row::Element::x2pos(double &x) const
 	}
 
 	// round to the closest side
-	if (x2 - last_w > w - x2) {
+	if (!low && (x2 - last_w > w - x2)) {
 		x2 = w;
 		++i;
 	} else
@@ -92,15 +92,13 @@ pos_type Row::Element::x2pos(double &x) const
 
 	// is element is rtl, flip values
 	if (rtl) {
-		x = last_w - x2;
-		i = endpos - i;
+		x = width() - x2;
 	} else {
 		x = x2;
-		i = pos + i;
 	}
 
 	//lyxerr << "=> p=" << i << " x=" << x << endl;
-	return i;
+	return pos + i;
 }
 
 
@@ -335,9 +333,9 @@ void Row::pop_back()
 }
 
 
-void Row::separate_back(pos_type const keep)
+void Row::shorten_if_needed(pos_type const keep, int const w)
 {
-	if (empty())
+	if (empty() || width() < w)
 		return;
 	int i = elements_.size();
 	int new_end = end_;
@@ -353,15 +351,32 @@ void Row::separate_back(pos_type const keep)
 		new_end = elements_[i].pos;
 		new_wid -= elements_[i].dim.wid;
 	}
-	if (i == 0)
+	if (i == 0) {
+		if (elements_.size() != 1) {
+			LYXERR0("Row is too large but has more than one element. " << *this);
+			return;
+		}
+#if 1
 		return;
+#else
+		// does not work yet
+		if (back().type != STRING)
+			return;
+		double xstr = w - x;
+		pos_type new_pos = back().x2pos(xstr, true);
+		back().str = back().str.substr(0, new_pos);
+		back().endpos = new_pos;
+		end_ = new_pos;
+		dim_.wid = x + xstr;
+#endif
+	}
 	end_ = new_end;
 	dim_.wid = new_wid;
 	elements_.erase(elements_.begin() + i, elements_.end());
 }
 
 
-void Row::reverseRtL()
+void Row::reverseRTL()
 {
 	pos_type i = 0;
 	pos_type const end = elements_.size();
@@ -372,7 +387,7 @@ void Row::reverseRtL()
 		if (i >= end)
 			break;
 
-		// look for a RtL sequence
+		// look for a RTL sequence
 		pos_type j = i;
 		while (j < end && elements_[j].font.isRightToLeft())
 			++j;

src/Row.h

@@ -69,10 +69,13 @@ public:
 		// \param i in the row element
 		double pos2x(pos_type const i) const;
 
-		// Return character position that is the closest to
-		// pixel position \param x. The value \param x is
-		// rounded to the actual pixel position.
-		pos_type x2pos(double &x) const;
+		/** 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.
+		*/
+		pos_type x2pos(double &x, bool low = false) const;
 
 		// The kind of row element
 		Type type;
@@ -186,11 +189,13 @@ public:
 	/// remove all row elements
 	void clear() { elements_.clear(); }
 	/**
-	 * remove all elements after last separator and update endpos
-	 * if necessary.
-	 * \param keep is the minimum amount of text to keep.
+	 * if row width is too large, remove all elements after last
+	 * separator and update endpos if necessary. If all that
+	 * rename is a large word, cut it to \param width.
+	 * \param body_pos minimum amount of text to keep.
+	 * \param width maximum width of the row
 	 */
-	void separate_back(pos_type keep);
+	void shorten_if_needed(pos_type const body_pos, int const width);
 
 	/**
 	 * If last element of the row is a string, compute its width
@@ -199,10 +204,10 @@ public:
 	void finalizeLast();
 
 	/**
-	 * Find sequences of RtL elements and reverse them.
+	 * Find sequences of right-to-left elements and reverse them.
 	 * This should be called once the row is completely built.
 	 */
-	void reverseRtL();
+	void reverseRTL();
 
 	friend std::ostream & operator<<(std::ostream & os, Row const & row);
 

src/TextMetrics.cpp

@@ -636,14 +636,17 @@ void TextMetrics::computeRowMetrics(pit_type const pit,
 				//lyxerr << "row.separator " << row.separator << endl;
 				//lyxerr << "ns " << ns << endl;
 			} else if (is_rtl) {
+				row.dimension().wid = width;
 				row.x += w;
 			}
 			break;
 		}
 		case LYX_ALIGN_RIGHT:
+			row.dimension().wid = width;
 			row.x += w;
 			break;
 		case LYX_ALIGN_CENTER:
+			row.dimension().wid += w / 2;
 			row.x += w / 2;
 			break;
 		}
@@ -868,7 +871,7 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 			// enlarge the last character to hold the end-of-par marker
 			Font f(text_->layoutFont(pit));
 			f.fontInfo().setColor(Color_paragraphmarker);
-			row.addVirtual(i, docstring(1, char_type(0x00B6)), f, Change());
+			row.addVirtual(i + 1, docstring(1, char_type(0x00B6)), f, Change());
 		}
 
 		// add inline completion width
@@ -916,8 +919,7 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 	row.finalizeLast();
 	row.endpos(i);
 	// if the row is too large, try to cut at last separator.
-	if (row.width() >= width)
-		row.separate_back(body_pos);
+	row.shorten_if_needed(body_pos, width);
 
 	// if the row ends with a separator that is not at end of
 	// paragraph, remove it
@@ -925,8 +927,8 @@ 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
-	row.reverseRtL();
+	// make sure that the RTL elements are in reverse ordering
+	row.reverseRTL();
 
 	row.dimension().wid += right_margin;
 }
@@ -1114,7 +1116,8 @@ void TextMetrics::setRowHeight(Row & row, pit_type const pit,
 pos_type TextMetrics::getColumnNearX(pit_type const pit,
 		Row const & row, int & x, bool & boundary) const
 {
-	// FIXME: handle properly boundary (not done now)
+	boundary = false;
+
 	pos_type pos = row.pos();
 	if (row.x >= x || row.empty())
 		x = row.x;
@@ -1135,9 +1138,29 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit,
 			w += cit->width();
 		}
 		if (cit == row.end())
-			lyxerr << "NOT FOUND!! x=" << x << ", wid=" << row.width() << endl;
+			lyxerr << "NOT FOUND!! x=" << x
+			       << ", wid=" << row.width() << endl;
+		/** This tests for the case where the cursor is placed
+		 * just before a font direction change. See comment on
+		 * the boundary_ member in DocIterator.h to understand
+		 * how bounddary helps here.
+		 */
+		else if (pos == cit->endpos
+			 && cit + 1 != row.end()
+			 && cit->font.isVisibleRightToLeft() != (cit + 1)->font.isVisibleRightToLeft())
+			boundary = true;
 	}
 
+	/** This tests for the case where the cursor is set at the end
+	 * of a row which has been broken due to a display inset on
+	 * next row. This can be recognized because the end of the
+	 * last element is the same as the end of the row (there is no
+	 * separator at the end of the row)
+	 */
+	if (!row.empty() && pos == row.back().endpos
+	    && row.back().endpos == row.endpos())
+		boundary = true;
+
 #if !defined(KEEP_OLD_METRICS_CODE)
 	return pos - row.pos();
 #else
@@ -1677,7 +1700,7 @@ int TextMetrics::cursorX(CursorSlice const & sl,
 	if (lyxrc.paragraph_markers && text_->isRTL(par)) {
 		ParagraphList const & pars_ = text_->paragraphs();
 		if (size_type(pit + 1) < pars_.size()) {
-			FontInfo f;
+			FontInfo f(text_->layoutFont(pit));
 			docstring const s = docstring(1, char_type(0x00B6));
 			x2 += theFontMetrics(f).width(s);
 		}

src/rowpainter.cpp

@@ -749,11 +749,10 @@ void RowPainter::paintLast()
 	case END_LABEL_NO_LABEL:
 		if (lyxrc.paragraph_markers && size_type(pit_ + 1) < pars_.size()) {
 			docstring const s = docstring(1, char_type(0x00B6));
-			FontInfo f = FontInfo();
-			FontMetrics const & fm = theFontMetrics(f);
+			FontInfo f = FontInfo(text_.layoutFont(pit_));
 			f.setColor(Color_paragraphmarker);
 			pi_.pain.text(int(x_), yo_, s, f);
-			x_ += fm.width(s);
+			x_ += theFontMetrics(f).width(s);
 		}
 		break;
 	}