From 6e0ea4269ae792225bb4e0d0f0ffcb3236c3c5c9 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Wed, 3 Jul 2024 00:22:55 +0200
Subject: [PATCH] Make sure paragraph positions are updated when scrolling

Sometimes quick selection-scrolling could cause a crash because the
position of some paragraphs is not computed. To fix that, in
BufferView::showCursor, make sure that the metrics are always kept
clean using updateMetrics(false), which is lighweight.

As a consequence, the 'update' parameter of showCursor and
scrollDocView is not needed anymore. Its removal is mechanical and
accounts for most of this commit.

The only other significant change is that, when creating synthetic
mouse events and relying on scroll() for small moves, the full metrics
recomputation is replaced by the lighter version.

More work is still to come on this code, but this should be going in
the right direction.
---
 src/BufferView.cpp               | 19 +++++++++----------
 src/BufferView.h                 |  5 ++---
 src/frontends/qt/GuiWorkArea.cpp |  6 +++---
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 70d81567c1..67aa4944f6 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -763,7 +763,7 @@ Inset const * BufferView::mathContextMenu(InsetMathNest const * inset,
 }
 
 
-void BufferView::scrollDocView(int const pixels, bool update)
+void BufferView::scrollDocView(int const pixels)
 {
 	// The scrollbar values are relative to the top of the screen, therefore the
 	// offset is equal to the target value.
@@ -786,7 +786,7 @@ void BufferView::scrollDocView(int const pixels, bool update)
 	// cut off at the top
 	if (pixels <= d->scrollbarParameters_.min) {
 		DocIterator dit = doc_iterator_begin(&buffer_);
-		showCursor(dit, SCROLL_VISIBLE, update);
+		showCursor(dit, SCROLL_VISIBLE);
 		LYXERR(Debug::SCROLLING, "scroll to top");
 		return;
 	}
@@ -795,7 +795,7 @@ void BufferView::scrollDocView(int const pixels, bool update)
 	if (pixels >= d->scrollbarParameters_.max) {
 		DocIterator dit = doc_iterator_end(&buffer_);
 		dit.backwardPos();
-		showCursor(dit, SCROLL_VISIBLE, update);
+		showCursor(dit, SCROLL_VISIBLE);
 		LYXERR(Debug::SCROLLING, "scroll to bottom");
 		return;
 	}
@@ -814,14 +814,14 @@ void BufferView::scrollDocView(int const pixels, bool update)
 		// It seems we didn't find the correct pit so stay on the safe side and
 		// scroll to bottom.
 		LYXERR0("scrolling position not found!");
-		scrollDocView(d->scrollbarParameters_.max, update);
+		scrollDocView(d->scrollbarParameters_.max);
 		return;
 	}
 
 	DocIterator dit = doc_iterator_begin(&buffer_);
 	dit.pit() = i;
 	LYXERR(Debug::SCROLLING, "pixels = " << pixels << " -> scroll to pit " << i);
-	showCursor(dit, SCROLL_VISIBLE, update);
+	showCursor(dit, SCROLL_VISIBLE);
 }
 
 
@@ -1007,20 +1007,19 @@ int BufferView::workWidth() const
 
 void BufferView::recenter()
 {
-	showCursor(d->cursor_, SCROLL_CENTER, true);
+	showCursor(d->cursor_, SCROLL_CENTER);
 }
 
 
 void BufferView::showCursor()
 {
-	showCursor(d->cursor_, SCROLL_VISIBLE, true);
+	showCursor(d->cursor_, SCROLL_VISIBLE);
 }
 
 
-void BufferView::showCursor(DocIterator const & dit, ScrollType how,
-	bool update)
+void BufferView::showCursor(DocIterator const & dit, ScrollType how)
 {
-	if (scrollToCursor(dit, how) && update)
+	if (scrollToCursor(dit, how))
 		processUpdateFlags(Update::ForceDraw);
 }
 
diff --git a/src/BufferView.h b/src/BufferView.h
index bfd523aa57..0b9ade2f3b 100644
--- a/src/BufferView.h
+++ b/src/BufferView.h
@@ -219,8 +219,7 @@ public:
 	/// This method will automatically scroll and update the BufferView
 	/// (metrics+drawing) if needed.
 	/// \param how Use this scroll strategy
-	/// \param force If true, update screen after scrolling
-	void showCursor(DocIterator const & dit, ScrollType how, bool update);
+	void showCursor(DocIterator const & dit, ScrollType how);
 	/// Scroll to the cursor.
 	/// \param how Use this scroll strategy
 	/// \return true if screen was scrolled
@@ -232,7 +231,7 @@ public:
 	/// scroll document by the given number of pixels.
 	int scroll(int pixels);
 	/// Scroll the view by a number of pixels.
-	void scrollDocView(int pixels, bool update);
+	void scrollDocView(int pixels);
 	/// Set the cursor position based on the scrollbar one.
 	void setCursorFromScrollbar();
 
diff --git a/src/frontends/qt/GuiWorkArea.cpp b/src/frontends/qt/GuiWorkArea.cpp
index 5c94f1e049..462fabda1e 100644
--- a/src/frontends/qt/GuiWorkArea.cpp
+++ b/src/frontends/qt/GuiWorkArea.cpp
@@ -586,7 +586,7 @@ void GuiWorkArea::Private::updateScrollbar()
 void GuiWorkArea::scrollTo(int value)
 {
 	stopBlinkingCaret();
-	d->buffer_view_->scrollDocView(value, true);
+	d->buffer_view_->scrollDocView(value);
 
 	if (lyxrc.cursor_follows_scrollbar) {
 		d->buffer_view_->setCursorFromScrollbar();
@@ -959,9 +959,9 @@ void GuiWorkArea::generateSyntheticMouseEvent()
 	// Scroll
 	if (step <= 2 * wh) {
 		d->buffer_view_->scroll(up ? -step : step);
-		d->buffer_view_->updateMetrics();
+		d->buffer_view_->processUpdateFlags(Update::ForceDraw);
 	} else {
-		d->buffer_view_->scrollDocView(value + (up ? -step : step), false);
+		d->buffer_view_->scrollDocView(value + (up ? -step : step));
 	}
 
 	// In which paragraph do we have to set the cursor ?
-- 
2.39.5