Rewrite (again!) the code for caret drawing

The caret geometry is now computed in BufferView as a list of shapes
(caret, horizontal l-shape if needed, completion triangle if needed)
kept in a variable of type CaretGeometry.

The code in WorkArea.cpp only has to draw these shapes. The
CaretWidget (which never was a widget) in GuiWorkArea.cpp is gone
now.

As a consequence, the bounding box for the cursor is known precisely
and therefore rows should be repainted correctly now. This avoids
caret droppings.

Fixes bug #12024.

src/BufferView.cpp

@@ -53,6 +53,7 @@
 #include "mathed/MathData.h"
 
 #include "frontends/alert.h"
+#include "frontends/CaretGeometry.h"
 #include "frontends/Delegates.h"
 #include "frontends/FontMetrics.h"
 #include "frontends/NullPainter.h"
@@ -288,6 +289,8 @@ struct BufferView::Private
 	CursorSlice current_row_slice_;
 	/// are we hovering something that we can click
 	bool clickable_inset_;
+	/// shape of the caret
+	frontend::CaretGeometry caret_geometry_;
 };
 
 
@@ -3067,10 +3070,91 @@ void BufferView::caretPosAndDim(Point & p, Dimension & dim) const
 	p = getPos(cur);
 	// center fat carets horizontally
 	p.x_ -= dim.wid / 2;
+	// p is top-left
 	p.y_ -= dim.asc;
 }
 
 
+void BufferView::buildCaretGeometry(bool complet)
+{
+	Point p;
+	Dimension dim;
+	caretPosAndDim(p, dim);
+
+	Cursor const & cur = d->cursor_;
+	Font const & realfont = cur.real_current_font;
+	frontend::FontMetrics const & fm = theFontMetrics(realfont.fontInfo());
+	bool const isrtl = realfont.isVisibleRightToLeft();
+	int const dir = isrtl ? -1 : 1;
+
+	frontend::CaretGeometry & cg = d->caret_geometry_;
+	cg.shapes.clear();
+
+	// The caret itself, slanted for italics in text edit mode except
+	// for selections because the selection rect does not slant
+	bool const slant = fm.italic() && cur.inTexted() && !cur.selection();
+	double const slope = slant ? fm.italicSlope() : 0;
+	cg.shapes.push_back(
+		{{iround(p.x_ + dim.asc * slope), p.y_},
+		 {iround(p.x_ - dim.des * slope), p.y_ + dim.height()},
+		 {iround(p.x_ + dir * dim.wid - dim.des * slope), p.y_ + dim.height()},
+		 {iround(p.x_ + dir * dim.wid + dim.asc * slope), p.y_}}
+		);
+
+	// The language indicator _| (if needed)
+	Language const * doclang = buffer().params().language;
+	if (!((realfont.language() == doclang && isrtl == doclang->rightToLeft())
+		  || realfont.language() == latex_language)) {
+		int const lx = dim.height() / 3;
+		int const xx = iround(p.x_ - dim.des * slope);
+		int const yy = p.y_ + dim.height();
+		cg.shapes.push_back(
+			{{xx, yy - dim.wid},
+			 {xx + dir * (dim.wid + lx - 1), yy - dim.wid},
+			 {xx + dir * (dim.wid + lx - 1), yy},
+			 {xx, yy}}
+			);
+	}
+
+	// The completion triangle |> (if needed)
+	if (complet) {
+		int const m = p.y_ + dim.height() / 2;
+		int const d = dim.height() / 8;
+		// offset for slanted carret
+		int const sx = iround((dim.asc - (dim.height() / 2 - d)) * slope);
+		// starting position x
+		int const xx = p.x_ + dir * dim.wid + sx;
+		cg.shapes.push_back(
+			{{xx, m - d},
+			 {xx + dir * d, m},
+			 {xx, m + d},
+			 {xx, m + d - dim.wid},
+			 {xx + dir * d - dim.wid, m},
+			 {xx, m - d + dim.wid}}
+			);
+	}
+
+	// compute extremal x values
+	cg.left = 1000000;
+	cg.right = -1000000;
+	cg.top = 1000000;
+	cg.bottom = -1000000;
+	for (auto const & shape : cg.shapes)
+		for (Point const & p : shape) {
+			cg.left = min(cg.left, p.x_);
+			cg.right = max(cg.right, p.x_);
+			cg.top = min(cg.top, p.y_);
+			cg.bottom = max(cg.bottom, p.y_);
+		}
+}
+
+
+frontend::CaretGeometry const &  BufferView::caretGeometry() const
+{
+	return d->caret_geometry_;
+}
+
+
 bool BufferView::caretInView() const
 {
 	if (!paragraphVisible(cursor()))
@@ -3296,19 +3380,13 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret)
 	 */
 	if (paint_caret) {
 		Cursor cur(d->cursor_);
-		Point p;
-		Dimension dim;
-		caretPosAndDim(p, dim);
 		while (cur.depth() > 1) {
-			if (cur.inTexted()) {
+			if (!cur.inTexted())
-				TextMetrics const & tm = textMetrics(cur.text());
+				break;
-				if (p.x_ >= tm.origin().x_
+			TextMetrics const & tm = textMetrics(cur.text());
-					&& p.x_ + dim.width() <= tm.origin().x_ + tm.dim().width())
+			if (d->caret_geometry_.left >= tm.origin().x_
-					break;
+				&& d->caret_geometry_.right <= tm.origin().x_ + tm.dim().width())
-			} else {
-				// in mathed
 				break;
-			}
 			cur.pop();
 		}
 		cur.textRow().changed(true);

src/BufferView.h

@@ -26,6 +26,7 @@ namespace lyx {
 
 namespace support { class FileName; }
 
+namespace frontend { class CaretGeometry; }
 namespace frontend { class Painter; }
 namespace frontend { class GuiBufferViewDelegate; }
 
@@ -311,6 +312,10 @@ public:
 	bool caretInView() const;
 	/// get the position and height of the caret
 	void caretPosAndDim(Point & p, Dimension & dim) const;
+	/// compute the shape of the caret
+	void buildCaretGeometry(bool complet);
+	/// the shape of the caret
+	frontend::CaretGeometry const & caretGeometry() const;
 
 	///
 	void draw(frontend::Painter & pain, bool paint_caret);

src/frontends/Makefile.am

@@ -12,6 +12,7 @@ AM_CPPFLAGS += -I$(srcdir)/.. \
 liblyxfrontends_a_SOURCES = \
 	alert.h \
 	Application.h \
+	CaretGeometry.h \
 	FontLoader.h \
 	FontMetrics.h \
 	Delegates.h \

src/frontends/qt/GuiWorkArea.cpp

@@ -15,7 +15,6 @@
 #include "GuiWorkArea_Private.h"
 
 #include "ColorCache.h"
-#include "FontLoader.h"
 #include "GuiApplication.h"
 #include "GuiCompleter.h"
 #include "GuiKeySymbol.h"
@@ -33,7 +32,6 @@
 #include "Font.h"
 #include "FuncRequest.h"
 #include "KeySymbol.h"
-#include "Language.h"
 #include "LyX.h"
 #include "LyXRC.h"
 #include "LyXVC.h"
@@ -51,6 +49,8 @@
 #include "support/TempFile.h"
 
 #include "frontends/Application.h"
+#include "frontends/CaretGeometry.h"
+
 #include "frontends/FontMetrics.h"
 #include "frontends/WorkAreaManager.h"
 
@@ -125,100 +125,6 @@ mouse_button::state q_motion_state(Qt::MouseButtons state)
 
 namespace frontend {
 
-class CaretWidget {
-public:
-	CaretWidget() : dir(1), l_shape(false), completable(false),
-					x(0), y(0), slope(0)
-	{}
-
-	/* Draw the caret. Parameter \c horiz_offset is not 0 when there
-	 * has been horizontal scrolling in current row
-	 */
-	void draw(QPainter & painter, int horiz_offset)
-	{
-		if (dim.empty())
-			return;
-		// correction for horizontal scrolling
-		int const xx = x - horiz_offset;
-		int const lx = dim.height() / 3;
-
-		// draw caret box
-		painter.setPen(color);
-		QPainterPath path;
-		path.moveTo(xx + dim.asc * slope, y);
-		path.lineTo(xx - dim.des * slope, y + dim.height());
-		path.lineTo(xx + dir * dim.wid - dim.des * slope, y + dim.height());
-		path.lineTo(xx + dir * dim.wid + dim.asc * slope, y);
-		painter.setRenderHint(QPainter::Antialiasing, true);
-		painter.fillPath(path, color);
-		painter.setRenderHint(QPainter::Antialiasing, false);
-
-		// draw RTL/LTR indication
-		if (l_shape)
-			painter.fillRect(xx - dim.des * slope,
-			                 y + dim.height() - dim.wid + 1,
-			                 dir * (dim.wid + lx - 1), dim.wid, color);
-
-		// draw completion triangle
-		if (completable) {
-			int const m = y + dim.height() / 2;
-			int const d = dim.height() / 8;
-			// offset for slanted carret
-			int const sx = (dim.asc - (dim.height() / 2 - d)) * slope;
-			painter.setPen(QPen(color, dim.width()));
-			painter.drawLine(xx + dir * dim.wid + sx, m - d,
-			                 xx + dir * (dim.wid + d) + sx, m);
-			painter.drawLine(xx + dir * dim.wid + sx, m + d,
-			                 xx + dir * (dim.wid + d) + sx, m);
-		}
-	}
-
-	void update(BufferView const * bv, bool complet) {
-		// Cursor size and position
-		Point point;
-		bv->caretPosAndDim(point, dim);
-		x = point.x_;
-		y = point.y_;
-		completable = complet;
-
-		Cursor const & cur = bv->cursor();
-		Font const & realfont = cur.real_current_font;
-		FontMetrics const & fm = theFontMetrics(realfont.fontInfo());
-		BufferParams const & bp = bv->buffer().params();
-		bool const samelang = realfont.language() == bp.language;
-		bool const isrtl = realfont.isVisibleRightToLeft();
-		dir = isrtl ? -1 : 1;
-		// special shape
-		l_shape = (!samelang || isrtl != bp.language->rightToLeft())
-			&& realfont.language() != latex_language;
-
-		// use slanted caret for italics in text edit mode
-		// except for selections because the selection rect does not slant
-		bool const slant = fm.italic() && cur.inTexted() && !cur.selection();
-		slope = slant ? fm.italicSlope() : 0;
-
-		color = guiApp->colorCache().get(Color_cursor);
-	}
-
-	/// text direction (1 for LtR, -1 for RtL)
-	int dir;
-	/// indication for language change
-	bool l_shape;
-	/// triangle to show that a completion is available
-	bool completable;
-	///
-	QColor color;
-	/// dimension uf base caret
-	Dimension dim;
-	/// x position (were the vertical line is drawn)
-	int x;
-	/// y position (the top of the caret)
-	int y;
-	/// the slope for drawing slanted caret
-	double slope;
-};
-
-
 // This is a 'heartbeat' generating synthetic mouse move events when the
 // cursor is at the top or bottom edge of the viewport. One scroll per 0.2 s
 SyntheticMouseEvent::SyntheticMouseEvent()
@@ -227,7 +133,7 @@ SyntheticMouseEvent::SyntheticMouseEvent()
 
 
 GuiWorkArea::Private::Private(GuiWorkArea * parent)
-: p(parent), buffer_view_(nullptr), lyx_view_(nullptr), caret_(nullptr),
+: p(parent), buffer_view_(nullptr), lyx_view_(nullptr),
   caret_visible_(false), need_resize_(false), preedit_lines_(1),
   last_pixel_ratio_(1.0), completer_(new GuiCompleter(p, p)),
   dialog_mode_(false), shell_escape_(false), read_only_(false),
@@ -266,7 +172,6 @@ GuiWorkArea::Private::~Private()
 		buffer_view_->buffer().workAreaManager().remove(p);
 	} catch(...) {}
 	delete buffer_view_;
-	delete caret_;
 	// Completer has a QObject parent and is thus automatically destroyed.
 	// See #4758.
 	// delete completer_;
@@ -316,7 +221,6 @@ void GuiWorkArea::init()
 	// With Qt4.5 a mouse event will happen before the first paint event
 	// so make sure that the buffer view has an up to date metrics.
 	d->buffer_view_->resize(viewport()->width(), viewport()->height());
-	d->caret_ = new frontend::CaretWidget();
 
 	setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
 	setAcceptDrops(true);
@@ -606,7 +510,7 @@ void GuiWorkArea::Private::resetCaret()
 		&& !completer_->popupVisible()
 		&& !completer_->inlineVisible();
 
-	caret_->update(buffer_view_, completable);
+	buffer_view_->buildCaretGeometry(completable);
 
 	needs_caret_geometry_update_ = true;
 	caret_visible_ = true;
@@ -647,6 +551,33 @@ void GuiWorkArea::Private::hideCaret()
 }
 
 
+/* Draw the caret. Parameter \c horiz_offset is not 0 when there
+ * has been horizontal scrolling in current row
+ */
+void GuiWorkArea::Private::drawCaret(QPainter & painter, int horiz_offset) const
+{
+	if (buffer_view_->caretGeometry().shapes.empty())
+		return;
+
+	QColor const color = guiApp->colorCache().get(Color_cursor);
+	painter.setPen(color);
+	painter.setRenderHint(QPainter::Antialiasing, true);
+	for (auto const & shape : buffer_view_->caretGeometry().shapes) {
+		bool first = true;
+		QPainterPath path;
+		for (Point const & p : shape) {
+			if (first) {
+				path.moveTo(p.x_ - horiz_offset, p.y_);
+				first = false;
+			} else
+				path.lineTo(p.x_ - horiz_offset, p.y_);
+		}
+		painter.fillPath(path, color);
+	}
+	painter.setRenderHint(QPainter::Antialiasing, false);
+}
+
+
 void GuiWorkArea::Private::updateScrollbar()
 {
 	// Prevent setRange() and setSliderPosition from causing recursive calls via
@@ -1194,9 +1125,11 @@ void GuiWorkArea::Private::paintPreeditText(GuiPainter & pain)
 	// FIXME: shall we use real_current_font here? (see #10478)
 	FontInfo const font = buffer_view_->cursor().getFont().fontInfo();
 	FontMetrics const & fm = theFontMetrics(font);
-	int const height = fm.maxHeight();
+	Point point;
-	int cur_x = caret_->x;
+	Dimension dim;
-	int cur_y = caret_->y + height;
+	buffer_view_->caretPosAndDim(point, dim);
+	int cur_x = point.x_;
+	int cur_y = point.y_ + dim.height();
 
 	// get attributes of input method cursor.
 	// cursor_pos : cursor position in preedit string.
@@ -1249,7 +1182,7 @@ void GuiWorkArea::Private::paintPreeditText(GuiPainter & pain)
 		// if we reached the right extremity of the screen, go to next line.
 		if (cur_x + fm.width(typed_char) > p->viewport()->width() - right_margin) {
 			cur_x = right_margin;
-			cur_y += height + 1;
+			cur_y += dim.height() + 1;
 			++preedit_lines_;
 		}
 		// preedit strings are displayed with dashed underline
@@ -1347,7 +1280,7 @@ void GuiWorkArea::paintEvent(QPaintEvent * ev)
 	if (d->caret_visible_) {
 		if (d->needs_caret_geometry_update_)
 			d->updateCaretGeometry();
-		d->caret_->draw(pain, d->buffer_view_->horizScrollOffset());
+		d->drawCaret(pain, d->buffer_view_->horizScrollOffset());
 	}
 
 	d->updateScreen(ev->rect());
@@ -1390,10 +1323,11 @@ void GuiWorkArea::inputMethodEvent(QInputMethodEvent * e)
 
 
 	// redraw area of preedit string.
-	int height = d->caret_->dim.height();
+	// int height = d->caret_->dim.height();
-	int cur_y = d->caret_->y;
+	// int cur_y = d->caret_->y;
-	viewport()->update(0, cur_y, viewport()->width(),
+	// viewport()->update(0, cur_y, viewport()->width(),
-		(height + 1) * d->preedit_lines_);
+	// 	(height + 1) * d->preedit_lines_);
+	viewport()->update();
 
 	if (d->preedit_string_.empty()) {
 		d->preedit_lines_ = 1;
@@ -1411,12 +1345,14 @@ QVariant GuiWorkArea::inputMethodQuery(Qt::InputMethodQuery query) const
 	switch (query) {
 		// this is the CJK-specific composition window position and
 		// the context menu position when the menu key is pressed.
-		case Qt::ImMicroFocus:
+	case Qt::ImMicroFocus: {
-			return QRect(d->caret_->x - 10 * (d->preedit_lines_ != 1),
+		CaretGeometry const & cg = bufferView().caretGeometry();
-						 d->caret_->y + d->caret_->dim.height() * d->preedit_lines_,
+		return QRect(cg.left - 10 * (d->preedit_lines_ != 1),
-						 d->caret_->dim.width(), d->caret_->dim.height());
+		             cg.top + cg.height() * d->preedit_lines_,
-		default:
+		             cg.width(), cg.height());
-			return QWidget::inputMethodQuery(query);
+	}
+	default:
+		return QWidget::inputMethodQuery(query);
 	}
 }
 

src/frontends/qt/GuiWorkArea_Private.h

@@ -68,7 +68,6 @@ public:
 /**
  * Implementation of the work area (buffer view GUI)
 */
-class CaretWidget;
 
 struct GuiWorkArea::Private
 {
@@ -91,6 +90,10 @@ struct GuiWorkArea::Private
 	void showCaret();
 	/// hide the caret if it is visible
 	void hideCaret();
+	/* Draw the caret. Parameter \c horiz_offset is not 0 when there
+	 * has been horizontal scrolling in current row
+	 */
+	void drawCaret(QPainter & painter, int horiz_offset) const;
 	/// Set the range and value of the scrollbar and connect to its valueChanged
 	/// signal.
 	void updateScrollbar();
@@ -118,8 +121,6 @@ struct GuiWorkArea::Private
 	///
 	QImage screen_;
 
-	///
-	CaretWidget * caret_;
 	/// is the caret currently displayed
 	bool caret_visible_;
 	///