Store in the Row object the list of elements it contains

* Row now contains a vector of Elements
* replace Row::dump by a proper << operator
* the width is updated as elements are added
* breakRow is reimplmented to use this infrastructure

00README_STR_METRICS_BRANCH

@@ -8,18 +8,13 @@ What is done:
   setRowHeight instead of rowBreakPoint and rowHeight.
 
 * change breakRow operation to operate on text strings on which
-  metrics are computed. Note that for now
-  FontMetrics::width(docstring) still computes the sum of character
-  widths, so that behavior is unchanged.
+  metrics are computed. The list of elements is stored in the row object
 
 * Implement proper string metrics computation (with cache), when
   lyxrc.force_paint_single_char is false.
 
 Next steps:
 
-* Make breakRow build a list of elements (string, inset,
-  separator,...) in the row. This will be reused by other methods
-
 * get rid of rowWidth (breakRow does compute this)
 
 * re-implement getColumnNearX using row elements
@@ -37,4 +32,9 @@ point. This will not be useful anymore with horizontal scrolling.
 actual text, not default font. This will be extended to the other
 methods.
 
-The other differences should be considered as bugs.
+Other differences that should be considered as bugs
+* 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
+  is kept for now in order to be able to detect row parsing errors,
+  but it could be removed right now.

src/ParagraphMetrics.cpp

@@ -190,8 +190,7 @@ void ParagraphMetrics::dump() const
 {
 	lyxerr << "Paragraph::dump: rows.size(): " << rows_.size() << endl;
 	for (size_t i = 0; i != rows_.size(); ++i) {
-		lyxerr << "  row " << i << ":   ";
-		rows_[i].dump();
+		lyxerr << "  row " << i << ":   " << rows_[i];
 	}
 }
 

src/Row.cpp

@@ -20,8 +20,13 @@
 
 #include "DocIterator.h"
 
+#include "frontends/FontMetrics.h"
+
 #include "support/debug.h"
 
+#include <ostream>
+
+using namespace std;
 
 namespace lyx {
 
@@ -29,7 +34,7 @@ namespace lyx {
 Row::Row()
 	: separator(0), label_hfill(0), x(0),
 	sel_beg(-1), sel_end(-1),
-	begin_margin_sel(false), end_margin_sel(false), 
+	begin_margin_sel(false), end_margin_sel(false),
 	changed_(false), crc_(0), pos_(0), end_(0)
 {}
 
@@ -62,7 +67,7 @@ bool Row::isMarginSelected(bool left_margin, DocIterator const & beg,
 	// Is the chosen margin selected ?
 	if (sel_pos == margin_pos) {
 		if (beg.pos() == end.pos())
-			// This is a special case in which the space between after 
+			// This is a special case in which the space between after
 			// pos i-1 and before pos i is selected, i.e. the margins
 			// (see DocIterator::boundary_).
 			return beg.boundary() && !end.boundary();
@@ -71,21 +76,21 @@ bool Row::isMarginSelected(bool left_margin, DocIterator const & beg,
 			// drawn if the cursor is after the margin.
 			return !end.boundary();
 		else if (beg.pos() == margin_pos)
-			// If the selection begins around the margin, it is 
+			// If the selection begins around the margin, it is
 			// only drawn if the cursor is before the margin.
 			return beg.boundary();
-		else 
+		else
 			return true;
 	}
 	return false;
 }
 
 
-void Row::setSelectionAndMargins(DocIterator const & beg, 
+void Row::setSelectionAndMargins(DocIterator const & beg,
 		DocIterator const & end) const
 {
 	setSelection(beg.pos(), end.pos());
-	
+
 	if (selection()) {
 		end_margin_sel = isMarginSelected(false, beg, end);
 		begin_margin_sel = isMarginSelected(true, beg, end);
@@ -116,14 +121,151 @@ bool Row::selection() const
 	return sel_beg != -1 && sel_end != -1;
 }
 
-
-void Row::dump(char const * s) const
+ostream & operator<<(ostream & os, Row const & row)
 {
-	LYXERR0(s << " pos: " << pos_ << " end: " << end_
-		<< " width: " << dim_.wid
-		<< " ascent: " << dim_.asc
-		<< " descent: " << dim_.des);
+	os << " pos: " << row.pos_ << " end: " << row.end_
+	   << " width: " << row.dim_.wid
+	   << " ascent: " << row.dim_.asc
+	   << " descent: " << row.dim_.des << "\n";
+	Row::Elements::const_iterator it = row.elements_.begin();
+	for ( ; it != row.elements_.end() ; ++it) {
+		switch (it->type) {
+		case Row::Element::STRING_ELT:
+			os << "**STRING: " << to_utf8(it->str) << endl;
+			break;
+		case Row::Element::INSET_ELT:
+			os << "**INSET: " << to_utf8(it->inset->layoutName()) << endl;
+			break;
+		case Row::Element::SEPARATOR_ELT:
+			os << "**SEPARATOR: " << endl;
+			break;
+		case Row::Element::SPACE_ELT:
+			os << "**SPACE: " << it->dim.wid << endl;
+			break;
+		}
+	}
+	return os;
 }
 
 
+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
+		   && elt.font == f && elt.change == ch;
+}
+
+
+void Row::finalizeLast()
+{
+	if (elements_.empty())
+		return;
+	Element & elt = elements_.back();
+	if (elt.final)
+		return;
+	elt.final = true;
+
+	if (elt.type == Element::STRING_ELT) {
+		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)
+{
+	finalizeLast();
+	Element e(Element::INSET_ELT);
+	e.pos = pos;
+	e.inset = ins;
+	e.dim = dim;
+	elements_.push_back(e);
+	dim_.wid += dim.wid;
+}
+
+
+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);
+}
+
+
+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;
+	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)
+{
+	finalizeLast();
+	Element e(Element::SEPARATOR_ELT);
+	e.pos = pos;
+	e.dim.wid = width;
+	elements_.push_back(e);
+	dim_.wid += e.dim.wid;
+}
+
+
+void Row::pop_back()
+{
+	dim_.wid -= elements_.back().dim.wid;
+	elements_.pop_back();
+}
+
+
+void Row::separate_back(pos_type const keep)
+{
+	if (empty())
+		return;
+	int i = elements_.size();
+	int new_end = end_;
+	int new_wid = dim_.wid;
+	if (i > 0 && elements_[i - 1].isLineSeparator() && new_end > keep) {
+		--i;
+		new_end = elements_[i].pos;
+		new_wid -= elements_[i].dim.wid;
+	}
+
+	while (i > 0 && !elements_[i - 1].isLineSeparator() && new_end > keep) {
+		--i;
+		new_end = elements_[i].pos;
+		new_wid -= elements_[i].dim.wid;
+	}
+	if (i == 0)
+		return;
+	end_ = new_end;
+	dim_.wid = new_wid;
+	elements_.erase(elements_.begin() + i, elements_.end());
+}
+
 } // namespace lyx

src/Row.h

@@ -15,14 +15,19 @@
 #ifndef ROW_H
 #define ROW_H
 
+#include "Changes.h"
+#include "Dimension.h"
+#include "Font.h"
+
+#include "support/docstring.h"
 #include "support/types.h"
 
-#include "Dimension.h"
-
+#include <vector>
 
 namespace lyx {
 
 class DocIterator;
+class Inset;
 
 /**
  * An on-screen row of text. A paragraph is broken into a
@@ -31,6 +36,45 @@ class DocIterator;
  */
 class Row {
 public:
+/**
+ * One element of a Row. It has a set of attributes that can be used
+ * by other methods that need to parse the Row contents.
+ */
+	struct Element {
+		enum Type {
+			STRING_ELT,
+			SEPARATOR_ELT,
+			INSET_ELT,
+			SPACE_ELT
+		};
+
+		Element(Type const t) : type(t), pos(0), inset(0), 
+					final(false) {}
+
+		//
+		bool isLineSeparator() const { return type == SEPARATOR_ELT; }
+
+		// The kind of row element
+		Type type;
+		// position of the element in the paragraph
+		pos_type pos;
+		// The dimension of the chunk (only width for strings)
+		Dimension dim;
+
+		// Non-zero if element is an inset
+		Inset const * inset;
+
+		// Non-empty if element is a string or separator
+		docstring str;
+		// is it possible to add contents to this element?
+		bool final;
+		//
+		Font font;
+		//
+		Change change;
+	};
+
+
 	///
 	Row();
 	///
@@ -49,9 +93,9 @@ public:
 	bool selection() const;
 	/// Set the selection begin and end and whether the left and/or right
 	/// margins are selected.
-	void setSelectionAndMargins(DocIterator const & beg, 
+	void setSelectionAndMargins(DocIterator const & beg,
 		DocIterator const & end) const;
-	
+
 	///
 	void pos(pos_type p);
 	///
@@ -73,6 +117,44 @@ 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,
+		 Font const & f, Change const & ch);
+	///
+	void add(pos_type pos, char_type const c,
+		 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);
+	///
+	bool empty() const { return elements_.empty(); }
+	///
+	Element & back() { return elements_.back(); }
+	///
+	Element const & back() const { return elements_.back(); }
+	/// remove last element
+	void pop_back();
+	/// 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.
+	 */
+	void separate_back(pos_type keep);
+
+	/**
+	 * If last element of the row is a string, compute its width
+	 * and mark it final.
+	 */
+	void finalizeLast();
+
+	friend std::ostream & operator<<(std::ostream & os, Row const & row);
+
 	/// current debugging only
 	void dump(char const * = "") const;
 
@@ -101,6 +183,18 @@ private:
 	bool isMarginSelected(bool left_margin, DocIterator const & beg,
 		DocIterator const & end) const;
 
+	/**
+	 * Returns true if a char or string with font \c f and change
+	 * type \c ch can be added to the current last element of the
+	 * row.
+	 */
+	bool sameString(Font const & f, Change const & ch) const;
+
+	///
+	typedef std::vector<Element> Elements;
+	///
+	Elements elements_;
+
 	/// has the Row appearance changed since last drawing?
 	mutable bool changed_;
 	/// CRC of row contents.

src/TextMetrics.cpp

@@ -467,7 +467,12 @@ bool TextMetrics::redoParagraph(pit_type const pit)
 		row.pos(first);
 		breakRow(row, right_margin, pit);
 		setRowHeight(row, pit);
+		int w = row.width();
 		row.dimension().wid = rowWidth(right_margin, pit, first, row.endpos());
+		if (row.width() != w) {
+			lyxerr << w << " => " << row.width() << ", body=" << par.beginOfBody() << ", size=" << par.size()<< ", inset=" << par.inInset().layoutName()<< endl;
+			lyxerr << row;
+		}
 		row.setChanged(false);
 		if (row_index || row.endpos() < par.size())
 			// If there is more than one row, expand the text to
@@ -800,9 +805,13 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 	Paragraph const & par = text_->getPar(pit);
 	pos_type const end = par.size();
 	pos_type const pos = row.pos();
-	int const left = leftMargin(max_width_, pit, pos);
 	int const width = max_width_ - right_margin;
-	if (pos == end || width < 0) {
+	pos_type const body_pos = par.beginOfBody();
+	row.clear();
+	row.dimension().wid = leftMargin(max_width_, pit, pos);
+
+	if (pos >= end || row.width() > width) {
+		row.dimension().wid += right_margin;
 		row.endpos(end);
 		return;
 	}
@@ -818,7 +827,6 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 		return addressBreakPoint(pos, par);
 #endif
 
-
 	// check for possible inline completion
 	DocIterator const & inlineCompletionPos = bv_->inlineCompletionPos();
 	pos_type inlineCompletionLPos = -1;
@@ -829,115 +837,106 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit
 		inlineCompletionLPos = inlineCompletionPos.pos() - 1;
 	}
 
-	pos_type const body_pos = par.beginOfBody();
-	int x = left;
-	pos_type point = end;
-
-	FontIterator fi = FontIterator(*this, par, pit, pos);
-	// Accumulator for character strings
-	docstring chunkstr;
-	Font chunkfont = *fi;
-
 	// Now we iterate through until we reach the right margin
-	// or the end of the par, then choose the possible break
-	// nearest that.
-
+	// or the end of the par, then build a representation of the row.
 	pos_type i = pos;
-	for ( ; i < end; ++i, ++fi) {
-		// Add the chunk width when it is finished
-		if (par.isInset(i) || *fi != chunkfont
-		    || (body_pos && i == body_pos)) {
-			x += theFontMetrics(chunkfont).width(chunkstr);
-			chunkstr.clear();
-			chunkfont = *fi;
-		}
-
+	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)) {
-			x += pm.insetDimension(par.getInset(i)).wid;
+			Inset const * ins = par.getInset(i);
+			Dimension dim = pm.insetDimension(ins);
+			row.add(i, ins, dim);
+		} else if (par.isLineSeparator(i)) {
+			// In theory, no inset has this property. If
+			// this is done, a new addSeparator which
+			// takes an inset as parameter should be
+			// added.
+			LATTEST(!par.isInset(i));
+			row.addSeparator(i, c, *fi, par.lookupChange(i));
 		} else if (c == '\t')
-			chunkstr += "    ";
+			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))
-					chunkstr += par.transformChar(c, i);
+					row.add(i, par.transformChar(c, i),
+						*fi, par.lookupChange(i));
 			} else if (language->lang() == "hebrew" &&
 				   !Encodings::isHebrewComposeChar(c)) {
-				chunkstr+= c;
+				row.add(i, c, *fi, par.lookupChange(i));
 			}
 		} else
-			chunkstr += c;
+			row.add(i, c, *fi, par.lookupChange(i));
 
+		// end of paragraph marker
 		if (lyxrc.paragraph_markers
-		    && i == end - 1 && size_type(pit + 1) < pars.size())
+		    && i == end - 1 && size_type(pit + 1) < pars.size()) {
 			// enlarge the last character to hold the end-of-par marker
-			chunkstr += char_type(0x00B6);
+			Font f(text_->layoutFont(pit));
+			f.fontInfo().setColor(Color_paragraphmarker);
+			row.add(i, char_type(0x00B6), f, Change());
+		}
 
 		// add inline completion width
-		if (inlineCompletionLPos == i)
-			chunkstr += bv_->inlineCompletion();
+		if (inlineCompletionLPos == i) {
+			Font f = *fi;
+			f.fontInfo().setColor(Color_inlinecompletion);
+			row.add(i, bv_->inlineCompletion(), f, Change());
+		}
+
+		// Handle some situations that abruptly terminate the row
+		// - A newline inset
+		// - Before a display inset
+		// - After a display inset
+		Inset const * inset = 0;
+		if (par.isNewline(i)
+		    || (i + 1 < end && (inset = par.getInset(i + 1))
+			&& inset->display())
+		    || (!row.empty() && row.back().inset
+			&& row.back().inset->display())) {
+			++i;
+			break;
+		}
+
+		++i;
+		++fi;
 
 		// add the auto-hfill from label end to the body
 		if (body_pos && i == body_pos) {
 			FontMetrics const & fm = theFontMetrics(
 				text_->labelFont(par));
-			int add = fm.width(par.layout().labelsep);
-			//if (par.isLineSeparator(i - 1))
-			//	add -= singleWidth(pit, i - 1);
-
-			add = max(add, labelEnd(pit) - x);
-			x += add;
+			if (!row.empty() && row.back().isLineSeparator())
+				row.pop_back();
+			int const add = max(fm.width(par.layout().labelsep),
+					    labelEnd(pit) - row.width());
+			row.addSpace(i, add);
 		}
 
-		if (par.isNewline(i)) {
-			point = i + 1;
-			break;
-		}
-		Inset const * inset = 0;
-		// Break before...
-		if (i + 1 < end) {
-			if ((inset = par.getInset(i + 1)) && inset->display()) {
-				point = i + 1;
-				break;
-			}
-			// ...and after.
-			if ((inset = par.getInset(i)) && inset->display()) {
-				point = i + 1;
-				break;
-			}
-		}
-
-		if (par.isLineSeparator(i)) {
-			x += theFontMetrics(chunkfont).width(chunkstr);
-			chunkstr.clear();
-			chunkfont = *fi;
-			if (x >= width) {
-				// exit on last registered breakpoint:
-				break;
-			}
-			// register breakpoint:
-			point = i + 1;
-		}
 	}
 
-	if (i == end) {
-		x += theFontMetrics(chunkfont).width(chunkstr);
-		// maybe found one, but the par is short enough.
-		if (x < width)
-			point = end;
-	}
+	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);
+
+	// if the row ends with a separator that is not at end of
+	// paragraph, remove it
+	if (!row.empty() && row.back().isLineSeparator()
+	    && row.endpos() < par.size())
+		row.pop_back();
+
+	row.dimension().wid += right_margin;
 
 	// manual labels cannot be broken in LaTeX. But we
 	// want to make our on-screen rendering of footnotes
 	// etc. still break
-	if (body_pos && point < body_pos)
-		point = body_pos;
-
-	row.endpos(point);
+	// if (body_pos && point < body_pos)
+	// 	point = body_pos;
 }