Cleanup TextMetrics::getPosNearX

This function mostly iterates though a row. Therefore it makes sense
to turn it into a wrapper around an new Row::x2pos() function.

Take this opportunity to use the C++17 structured bindings declaration
instead of passing a bool variable by address (which is only an output
variable).

No change intended.

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;
+	auto [p, b] = tm.getPosNearX(textRow(), x);
-	pos() = tm.getPosNearX(textRow(), x, b);
+	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;
+		auto [b, p] = tm.getPosNearX(real_next_row, xo);
-		top().pos() = tm.getPosNearX(real_next_row, xo, bound);
+		pos() = p;
-		boundary(bound);
+		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
+		/** Return character position that is the closest to pixel
-		 *  pixel position \param x. The value \param x is
+		 *  position \param x. The value \param x is adjusted to the
-		 *  adjusted to the actual pixel position.
+		 *  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,
+pair<pos_type, bool> TextMetrics::getPosNearX(Row const & row, int & x) const
-				  bool & boundary) const
 {
 	//LYXERR0("getPosNearX(" << x << ") row=" << row);
 	/// For the main Text, it is possible that this pit is not
@@ -1446,70 +1445,16 @@ 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();
+	auto [pos, boundary] = row.x2pos(x);
-	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();
-	}
 
 	x += xo - offset;
 	//LYXERR0("getPosNearX ==> pos=" << pos << ", boundary=" << boundary);
 
-	return pos;
+	return make_pair(pos, boundary);
 }
 
 
+// FIXME: only InsetTabular uses this. Remove?
 pos_type TextMetrics::x2pos(pit_type pit, int row, int x) const
 {
 	// We play safe and use parMetrics(pit) to make sure the
@@ -1519,9 +1464,8 @@ pos_type TextMetrics::x2pos(pit_type pit, int row, int x) const
 	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 getPosNearX(r, x).first;
 }
 
 
@@ -1645,8 +1589,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
+		auto [pos, bound] = getPosNearX(row, x);
-		cur.pos() = getPosNearX(row, x, bound);
+		cur.pos() = pos;
 		cur.boundary(bound);
 		cur.setCurrentFont();
 		cur.setTargetX(x);
@@ -1668,8 +1612,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
+		// No inset, set position in the text
-		cur.pos() = getPosNearX(row, x, bound);
+		auto [pos, bound] = getPosNearX(row, x);
+		cur.pos() = pos;
 		cur.boundary(bound);
 		cur.setCurrentFont();
 		cur.setTargetX(x);
@@ -1681,7 +1626,7 @@ 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);
@@ -1706,9 +1651,7 @@ void TextMetrics::setCursorFromCoordinates(Cursor & cur, int const x, int const
 
 	LYXERR(Debug::PAINTING, "row " << r << " from pos: " << row.pos());
 
-	bool bound = false;
+	auto [pos, bound] = getPosNearX(row, x);
-	int xx = x;
-	pos_type const pos = getPosNearX(row, xx, bound);
 
 	LYXERR(Debug::PAINTING, "setting cursor pit: " << pit << " pos: " << pos);
 

src/TextMetrics.h

@@ -189,7 +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;
+	std::pair<pos_type, bool> getPosNearX(Row const & row, int & x) const;
 
 	/// returns pos in given par at given x coord.
 	pos_type x2pos(pit_type pit, int row, int x) const;

src/frontends/qt/GuiWorkArea.cpp

@@ -975,6 +975,7 @@ void GuiWorkArea::generateSyntheticMouseEvent()
 		return;
 	TextMetrics const & tm = d->buffer_view_->textMetrics(text);
 
+	// FIXME: use TextMetrics::setCursorFromCoordinates.
 	// Quit gracefully if there are no metrics, since otherwise next
 	// line would crash (bug #10324).
 	// This situation seems related to a (not yet understood) timing problem.
@@ -1001,9 +1002,8 @@ void GuiWorkArea::generateSyntheticMouseEvent()
 	}
 
 	// 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);
+	auto [pos, bound] = tm.getPosNearX(*rit, x);
 
 	// Set the cursor
 	cur.pit() = pit;