From 9f33db8f537e4488e229305d9a7a8707135d8e42 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Fri, 3 May 2024 14:28:30 +0200
Subject: [PATCH] Fix up 5577e877: remove logic error

Commit 5577e877 introduces forceUpdateBuffer() to delay actual
updatBuffer() calls to a central place.

In Cursor::upDownInText, the updateNeeded flag (which triggers
Update::Force update flag) is set to true when Cursor::checkDepm
returns true. This is fine, except that checkDepm also leads to a
forceBufferUpdate() call.

The logic in the method was such that, when updateNeeded is already
true when entering, forceBufferUpdate() will be called even though
checkDepm did not request it.

Fixes a slight delay with selecting in MergedManual (the manual of
manuals).
---
 src/Cursor.cpp | 24 +++++++++++++++---------
 src/Cursor.h   |  5 ++---
 src/Text.cpp   | 11 ++++++-----
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index 45ae34c4b2..c1427f731a 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -2143,7 +2143,7 @@ bool Cursor::mathBackward(bool word)
 }
 
 
-bool Cursor::upDownInText(bool up, bool & updateNeeded)
+bool Cursor::upDownInText(bool up)
 {
 	LASSERT(text(), return false);
 
@@ -2153,6 +2153,9 @@ bool Cursor::upDownInText(bool up, bool & updateNeeded)
 	getPos(xo, yo);
 	xo = beforeDispatchPosX_;
 
+	// Is a full repaint necessary?
+	bool updateNeeded = false;
+
 	// update the targetX - this is here before the "return false"
 	// to set a new target which can be used by InsetTexts above
 	// if we cannot move up/down inside this inset anymore
@@ -2221,12 +2224,13 @@ bool Cursor::upDownInText(bool up, bool & updateNeeded)
 			dummy.pos() = dummy.pos() == 0 ? dummy.lastpos() : 0;
 			dummy.pit() = dummy.pit() == 0 ? dummy.lastpit() : 0;
 
-			updateNeeded |= bv().checkDepm(dummy, *this);
-			updateTextTargetOffset();
-			if (updateNeeded)
+			if (bv().checkDepm(dummy, *this)) {
+				updateNeeded = true;
 				forceBufferUpdate();
+			}
+			updateTextTargetOffset();
 		}
-		return false;
+		return updateNeeded;
 	}
 
 	// with and without selection are handled differently
@@ -2252,6 +2256,7 @@ bool Cursor::upDownInText(bool up, bool & updateNeeded)
 			++dummy.pos();
 		if (bv().checkDepm(dummy, old)) {
 			updateNeeded = true;
+			forceBufferUpdate();
 			// Make sure that cur gets back whatever happened to dummy (Lgb)
 			operator=(dummy);
 		}
@@ -2292,13 +2297,14 @@ bool Cursor::upDownInText(bool up, bool & updateNeeded)
 		// When selection==false, this is done by TextMetrics::editXY
 		setCurrentFont();
 
-		updateNeeded |= bv().checkDepm(*this, old);
+		if (bv().checkDepm(*this, old)) {
+			updateNeeded = true;
+			forceBufferUpdate();
+		}
 	}
 
-	if (updateNeeded)
-		forceBufferUpdate();
 	updateTextTargetOffset();
-	return true;
+	return updateNeeded;
 }
 
 
diff --git a/src/Cursor.h b/src/Cursor.h
index e362073ef4..094a540f24 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -506,9 +506,8 @@ public:
 	/// return true if fullscreen update is needed
 	bool down();
 	/// move up/down in a text inset, called for LFUN_UP/DOWN,
-	/// return true if successful, updateNeeded set to true if fullscreen
-	/// update is needed, otherwise it's not touched
-	bool upDownInText(bool up, bool & updateNeeded);
+	/// return true if fullscreen update is needed
+	bool upDownInText(bool up);
 	/// move up/down in math or any non text inset, call for LFUN_UP/DOWN
 	/// return true if successful
 	bool upDownInMath(bool up);
diff --git a/src/Text.cpp b/src/Text.cpp
index 55e150d6ac..44ca43f3ae 100644
--- a/src/Text.cpp
+++ b/src/Text.cpp
@@ -4333,7 +4333,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 
 		if (!atFirstOrLastRow) {
 			needsUpdate |= cur.selHandle(select);
-			cur.upDownInText(up, needsUpdate);
+			needsUpdate |= cur.upDownInText(up);
 			needsUpdate |= cur.beforeDispatchCursor().inMathed();
 		} else {
 			pos_type newpos = up ? 0 : cur.lastpos();
@@ -4341,10 +4341,11 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 				needsUpdate |= cur.selHandle(select);
 				// we do not reset the targetx of the cursor
 				cur.pos() = newpos;
-				needsUpdate |= bv->checkDepm(cur, bv->cursor());
-				cur.updateTextTargetOffset();
-				if (needsUpdate)
+				if (bv->checkDepm(cur, bv->cursor())) {
+					needsUpdate = true;
 					cur.forceBufferUpdate();
+				}
+				cur.updateTextTargetOffset();
 				break;
 			}
 
@@ -4352,7 +4353,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 			// the selection right now, but wait for the next dispatch.
 			if (select)
 				needsUpdate |= cur.selHandle(select);
-			cur.upDownInText(up, needsUpdate);
+			needsUpdate |= cur.upDownInText(up);
 			cur.undispatched();
 		}