Get rid of TextMetrics::x2pos

Replace it with code that uses getPosNearX.

src/Cursor.cpp

@@ -1330,12 +1330,13 @@ void Cursor::posVisToRowExtremity(bool left)
 	LYXERR(Debug::RTL, "entering extremity: " << pit() << "," << pos() << ","
 		<< (boundary() ? 1 : 0));
 
+	// FIXME: no need for metrics here!
 	TextMetrics const & tm = bv_->textMetrics(text());
 	// Looking for extremities is like clicking on the left or the
 	// right of the row.
 	int x = tm.origin().x + (left ? 0 : textRow().width());
-	bool b = false;
-	pos() = tm.getPosNearX(textRow(), x, b);
+	auto [p, b] = tm.getPosNearX(textRow(), x);
+	pos() = p;
 	boundary(b);
 
 	LYXERR(Debug::RTL, "leaving extremity: " << pit() << "," << pos() << ","
@@ -2286,9 +2287,9 @@ bool Cursor::upDownInText(bool up)
 		}
 
 		Row const & real_next_row = tm.parMetrics(pit()).rows()[next_row];
-		bool bound = false;
-		top().pos() = tm.getPosNearX(real_next_row, xo, bound);
-		boundary(bound);
+		auto [b, p] = tm.getPosNearX(real_next_row, xo);
+		pos() = p;
+		boundary(b);
 		// When selection==false, this is done by TextMetrics::editXY
 		setCurrentFont();
 

src/Row.cpp

@@ -415,6 +415,70 @@ bool Row::setExtraWidth(int w)
 }
 
 
+pair<pos_type, bool> Row::x2pos(int & x) const
+{
+	pos_type retpos = pos();
+	bool boundary = false;
+	if (empty())
+		x = left_margin;
+	else if (x <= left_margin) {
+		retpos = front().left_pos();
+		x = left_margin;
+	} else if (x >= width()) {
+		retpos = back().right_pos();
+		x = width();
+	} else {
+		double w = left_margin;
+		const_iterator cit = begin();
+		const_iterator cend = end();
+		for ( ; cit != cend; ++cit) {
+			if (w <= x &&  w + cit->full_width() > x) {
+				int x_offset = int(x - w);
+				retpos = cit->x2pos(x_offset);
+				x = int(x_offset + w);
+				break;
+			}
+			w += cit->full_width();
+		}
+		if (cit == end()) {
+			retpos = back().right_pos();
+			x = width();
+		}
+		/** 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 boundary helps here.
+		 */
+		else if (retpos == cit->endpos
+		         && ((!cit->isRTL() && cit + 1 != end()
+		              && (cit + 1)->isRTL())
+		             || (cit->isRTL() && cit != begin()
+		                 && !(cit - 1)->isRTL())))
+			boundary = true;
+	}
+
+	if (empty())
+		boundary = end_boundary();
+	/** This tests for the case where the cursor is set at the end
+	 * of a row which has been broken due something else than a
+	 * separator (a display inset or a forced breaking of the
+	 * row). We know that there is a separator when the end of the
+	 * row is larger than the end of its last element.
+	 */
+	else if (retpos == back().endpos && back().endpos == endpos()) {
+		// FIXME: need a row flag here to say that cursor cannot be at the end
+		Inset const * inset = back().inset;
+		if (inset && (inset->lyxCode() == NEWLINE_CODE
+		              || inset->lyxCode() == SEPARATOR_CODE))
+			retpos = back().pos;
+		else
+			boundary = end_boundary();
+	}
+
+	return make_pair(retpos, boundary);
+}
+
+
 bool Row::sameString(Font const & f, Change const & ch) const
 {
 	if (elements_.empty())

src/Row.h

@@ -91,14 +91,14 @@ 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
-		 *  adjusted to the actual pixel position.
+		/** Return character position that is the closest to pixel
+		 *  position \param x. The value \param x is adjusted to the
+		 *  actual pixel position.
 		*/
 		pos_type x2pos(int &x) const;
 		/** Break the element in two if possible, so that its width is less
 		 * than the required values.
-		 * \return true if something has been done ; false if this is
+		 * \return true if something has been done; false if this is
 		 * not needed or not possible.
 		 * \param width: maximum width of the row.
 		 * \param next_width: available width on next rows.
@@ -252,6 +252,12 @@ public:
 	// distributed among expanders.  \return false if the justification fails.
 	bool setExtraWidth(int w);
 
+	/** Return character position and boundary value that are the
+	 *  closest to pixel position \param x. The value \param x is
+	 *  adjusted to the actual pixel position.
+	 */
+	std::pair<pos_type, bool> x2pos(int & x) const;
+
 	///
 	void add(pos_type pos, Inset const * ins, Dimension const & dim,
 		 Font const & f, Change const & ch);

src/TextMetrics.cpp

@@ -1432,8 +1432,7 @@ void TextMetrics::setRowHeight(Row & row) const
 // x is an absolute screen coord
 // returns the column near the specified x-coordinate of the row
 // x is set to the real beginning of this column
-pos_type TextMetrics::getPosNearX(Row const & row, int & x,
-				  bool & boundary) const
+pair<pos_type, bool> TextMetrics::getPosNearX(Row const & row, int & x) const
 {
 	//LYXERR0("getPosNearX(" << x << ") row=" << row);
 	/// For the main Text, it is possible that this pit is not
@@ -1446,82 +1445,12 @@ pos_type TextMetrics::getPosNearX(Row const & row, int & x,
 	int const offset = bv_->horizScrollOffset(text_, row.pit(), row.pos());
 	x += offset;
 
-	pos_type pos = row.pos();
-	boundary = false;
-	if (row.empty())
-		x = row.left_margin;
-	else if (x <= row.left_margin) {
-		pos = row.front().left_pos();
-		x = row.left_margin;
-	} else if (x >= row.width()) {
-		pos = row.back().right_pos();
-		x = row.width();
-	} else {
-		double w = row.left_margin;
-		Row::const_iterator cit = row.begin();
-		Row::const_iterator cend = row.end();
-		for ( ; cit != cend; ++cit) {
-			if (w <= x &&  w + cit->full_width() > x) {
-				int x_offset = int(x - w);
-				pos = cit->x2pos(x_offset);
-				x = int(x_offset + w);
-				break;
-			}
-			w += cit->full_width();
-		}
-		if (cit == row.end()) {
-			pos = row.back().right_pos();
-			x = row.width();
-		}
-		/** 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 boundary helps here.
-		 */
-		else if (pos == cit->endpos
-		         && ((!cit->isRTL() && cit + 1 != row.end()
-		              && (cit + 1)->isRTL())
-		             || (cit->isRTL() && cit != row.begin()
-		                 && !(cit - 1)->isRTL())))
-			boundary = true;
-	}
-
-	if (row.empty())
-		boundary = row.end_boundary();
-	/** This tests for the case where the cursor is set at the end
-	 * of a row which has been broken due something else than a
-	 * separator (a display inset or a forced breaking of the
-	 * row). We know that there is a separator when the end of the
-	 * row is larger than the end of its last element.
-	 */
-	else if (pos == row.back().endpos && row.back().endpos == row.endpos()) {
-		Inset const * inset = row.back().inset;
-		if (inset && (inset->lyxCode() == NEWLINE_CODE
-		              || inset->lyxCode() == SEPARATOR_CODE))
-			pos = row.back().pos;
-		else
-			boundary = row.end_boundary();
-	}
+	auto [pos, boundary] = row.x2pos(x);
 
 	x += xo - offset;
 	//LYXERR0("getPosNearX ==> pos=" << pos << ", boundary=" << boundary);
 
-	return pos;
-}
-
-
-pos_type TextMetrics::x2pos(pit_type pit, int row, int x) const
-{
-	// We play safe and use parMetrics(pit) to make sure the
-	// ParagraphMetrics will be redone and OK to use if needed.
-	// Otherwise we would use an empty ParagraphMetrics in
-	// upDownInText() while in selection mode.
-	ParagraphMetrics const & pm = parMetrics(pit);
-
-	LBUFERR(row < int(pm.rows().size()));
-	bool bound = false;
-	Row const & r = pm.rows()[row];
-	return getPosNearX(r, x, bound);
+	return make_pair(pos, boundary);
 }
 
 
@@ -1645,8 +1574,8 @@ Inset * TextMetrics::editXY(Cursor & cur, int x, int y,
 
 	if (!e) {
 		// No inset, set position in the text
-		bool bound = false; // is modified by getPosNearX
-		cur.pos() = getPosNearX(row, x, bound);
+		auto [pos, bound] = getPosNearX(row, x);
+		cur.pos() = pos;
 		cur.boundary(bound);
 		cur.setCurrentFont();
 		cur.setTargetX(x);
@@ -1668,8 +1597,9 @@ Inset * TextMetrics::editXY(Cursor & cur, int x, int y,
 	if (cur.text() == text_ && cur.pos() == e->pos) {
 		// non-editable inset, set cursor after the inset if x is
 		// nearer to that position (bug 9628)
-		bool bound = false; // is modified by getPosNearX
-		cur.pos() = getPosNearX(row, x, bound);
+		// No inset, set position in the text
+		auto [pos, bound] = getPosNearX(row, x);
+		cur.pos() = pos;
 		cur.boundary(bound);
 		cur.setCurrentFont();
 		cur.setTargetX(x);
@@ -1681,13 +1611,13 @@ Inset * TextMetrics::editXY(Cursor & cur, int x, int y,
 }
 
 
-void TextMetrics::setCursorFromCoordinates(Cursor & cur, int const x, int const y)
+void TextMetrics::setCursorFromCoordinates(Cursor & cur, int x, int const y)
 {
 	LASSERT(text_ == cur.text(), return);
 	pit_type const pit = getPitNearY(y);
 	LASSERT(pit != -1, return);
 
-	ParagraphMetrics const & pm = par_metrics_[pit];
+	ParagraphMetrics const & pm = parMetrics(pit);
 
 	int yy = pm.position() - pm.rows().front().ascent();
 	LYXERR(Debug::PAINTING, "x: " << x << " y: " << y <<
@@ -1706,9 +1636,7 @@ void TextMetrics::setCursorFromCoordinates(Cursor & cur, int const x, int const
 
 	LYXERR(Debug::PAINTING, "row " << r << " from pos: " << row.pos());
 
-	bool bound = false;
-	int xx = x;
-	pos_type const pos = getPosNearX(row, xx, bound);
+	auto [pos, bound] = getPosNearX(row, x);
 
 	LYXERR(Debug::PAINTING, "setting cursor pit: " << pit << " pos: " << pos);
 

src/TextMetrics.h

@@ -189,13 +189,7 @@ public:
 	/// returns the position near the specified x-coordinate of the row.
 	/// x is an absolute screen coord, it is set to the real beginning
 	/// of this column. This takes in account horizontal cursor row scrolling.
-	pos_type getPosNearX(Row const & row, int & x, bool & boundary) const;
-
-	/// returns pos in given par at given x coord.
-	pos_type x2pos(pit_type pit, int row, int x) const;
-
-	// FIXME: is there a need for this?
-	//int pos2x(pit_type pit, pos_type pos) const;
+	std::pair<pos_type, bool> getPosNearX(Row const & row, int & x) const;
 
 	/// returns the row near the specified y-coordinate in a given paragraph
 	/// (relative to the screen). If assert_in_view is true, it is made sure

src/frontends/qt/GuiWorkArea.cpp

@@ -973,7 +973,7 @@ void GuiWorkArea::generateSyntheticMouseEvent()
 	Text * text = cur.text();
 	if (!text)
 		return;
-	TextMetrics const & tm = d->buffer_view_->textMetrics(text);
+	TextMetrics & tm = d->buffer_view_->textMetrics(text);
 
 	// Quit gracefully if there are no metrics, since otherwise next
 	// line would crash (bug #10324).
@@ -981,34 +981,8 @@ void GuiWorkArea::generateSyntheticMouseEvent()
 	if (tm.empty())
 		return;
 
-	pair<pit_type, const ParagraphMetrics *> pp = up ? tm.first() : tm.last();
-	ParagraphMetrics const & pm = *pp.second;
-	pit_type const pit = pp.first;
-
-	if (pm.rows().empty())
-		return;
-
-	// Find the row at which we set the cursor.
-	RowList::const_iterator rit = pm.rows().begin();
-	RowList::const_iterator rlast = pm.rows().end();
-	int yy = pm.top();
-	for (--rlast; rit != rlast; ++rit) {
-		int h = rit->height();
-		if ((up && yy + h > 0)
-			  || (!up && yy + h > wh - defaultRowHeight()))
-			break;
-		yy += h;
-	}
-
-	// Find the position of the cursor
-	bool bound;
-	int x = d->synthetic_mouse_event_.cmd.x();
-	pos_type const pos = tm.getPosNearX(*rit, x, bound);
-
-	// Set the cursor
-	cur.pit() = pit;
-	cur.pos() = pos;
-	cur.boundary(bound);
+	const int y = up ? 0 : wh - defaultRowHeight();
+	tm.setCursorFromCoordinates(cur, d->synthetic_mouse_event_.cmd.x(), y);
 
 	d->buffer_view_->buffer().changed(false);
 }

src/insets/InsetTabular.cpp

@@ -5292,15 +5292,15 @@ void InsetTabular::doDispatch(Cursor & cur, FuncRequest & cmd)
 			// setting also the right targetX.
 			cur.selHandle(act == LFUN_DOWN_SELECT);
 			if (tabular.cellRow(cur.idx()) != tabular.nrows() - 1) {
-				int const xtarget = cur.targetX();
+				int xtarget = cur.targetX();
 				// WARNING: Once cur.idx() has been reset, the cursor is in
 				// an inconsistent state until pos() has been set. Be careful
 				// what you do with it!
 				cur.idx() = tabular.cellBelow(cur.idx());
 				cur.pit() = 0;
-				TextMetrics const & tm =
-					cur.bv().textMetrics(cell(cur.idx())->getText(0));
-				cur.pos() = tm.x2pos(cur.pit(), 0, xtarget);
+				TextMetrics const & tm = cur.bv().textMetrics(cell(cur.idx())->getText(0));
+				ParagraphMetrics const & pm = tm.parMetrics(cur.pit());
+				cur.pos() = tm.getPosNearX(pm.rows().front(), xtarget).first;
 				cur.setCurrentFont();
 			}
 		}
@@ -5331,7 +5331,7 @@ void InsetTabular::doDispatch(Cursor & cur, FuncRequest & cmd)
 			// setting also the right targetX.
 			cur.selHandle(act == LFUN_UP_SELECT);
 			if (tabular.cellRow(cur.idx()) != 0) {
-				int const xtarget = cur.targetX();
+				int xtarget = cur.targetX();
 				// WARNING: Once cur.idx() has been reset, the cursor is in
 				// an inconsistent state until pos() has been set. Be careful
 				// what you do with it!
@@ -5339,9 +5339,8 @@ void InsetTabular::doDispatch(Cursor & cur, FuncRequest & cmd)
 				cur.pit() = cur.lastpit();
 				Text const * text = cell(cur.idx())->getText(0);
 				TextMetrics const & tm = cur.bv().textMetrics(text);
-				ParagraphMetrics const & pm =
-					tm.parMetrics(cur.lastpit());
-				cur.pos() = tm.x2pos(cur.pit(), pm.rows().size()-1, xtarget);
+				ParagraphMetrics const & pm = tm.parMetrics(cur.lastpit());
+				cur.pos() = tm.getPosNearX(pm.rows().back(), xtarget).first;
 				cur.setCurrentFont();
 			}
 		}