From: Jean-Marc Lasgouttes <lasgouttes@lyx.org>
Date: Wed, 20 Feb 2019 11:01:44 +0000 (+0100)
Subject: Honor Update::SinglePar properly
X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=edd203a3603618883c61c022a8e40de1979bbce9;p=features.git

Honor Update::SinglePar properly

The SinglePar update flags has been a no-op for a long time without
naybody noticing. This means that the current paragraph was
always rebroken and redrawn, even when only moving the cursor around.

Now we only do that when Update::SinglePar has been specified. This
means that there may be cases where update will not be correct
anymore, because this flag has not been specified. These places will
have to be found and fixed.

Update PAINTING_ANALYSIS.
---

diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS
index 32bc93a5ff..a52a036aaa 100644
--- a/development/PAINTING_ANALYSIS
+++ b/development/PAINTING_ANALYSIS
@@ -20,15 +20,9 @@ following section. Some actions are proposed.
 
 ** SinglePar update
 
-The flag Update::SinglePar is set in many places but never acted on.
-Instead, metrics update is skipped whenever the recorded height of
-current paragraph did not change and Update::Force was not specified.
-Is it better to keep that (which incurs extra work) or to condition it
-on Update::SinglePar? If the first solution is kept, the flag
-SingleParUpdate shall be removed.
-
-Moreover, I fail to see (yet) where the 'single' part of the program
-is acted on.
+This flag only has an effect in the current BufferView, but I think it
+is useful in other views too. Doing this will require some work on the
+update pipeline, though.
 
 ** Buffer::change issues
 
@@ -40,6 +34,7 @@ a good value? NoScreenUpdate?
 On a related note, what is the semantics of a call to
 Buffer::changed(false)? What does the caller mean?
 
+
 ** How to avoid redraw with FitCursor when the cursor is already OK?
 
 In this case, we invoke Buffer::change(false) with drawing disabled
@@ -60,12 +55,6 @@ cursor.
 
 * Clean-up of drawing code
 
-** Make SinglePar update flag useful again. 
-
-The current code can be very expensive when moving cursor inside a
-huge table, for example. We should test the flag again, although this
-will probably lead to some glitches here and there.
-
 ** Set Row::changed() in a finer way
 
 *** singleParUpdate
@@ -139,15 +128,14 @@ DecorationUpdate). It triggers a recomputation of the metrics when either:
  + Update::Force has been specified
  + Update::FitCursor has been specified and there is a need to scroll
    the display.
- + the current paragraph, after rebreak, does not have the same height as in
-   existing metrics. Note that the Update::SinglePar flag is *never*
-   taken into account.
+ + Update::SinglePar has been specified and the current paragraph has
+   not changed height.
 
 If a computation of metrics has taken place, Force is removed from the
 flags and ForceDraw is added instead.
 
-It is OK to call processUptateFlags several times before an update. In
-this case, the effects are cumulative.processUpdateFlags execute the
+It is OK to call processUpateFlags several times before an update. In
+this case, the effects are cumulative. processUpdateFlags executes the
 metrics-related actions, but defers the actual drawing to the next
 paint event.
 
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 35a126b18a..a46420e339 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -500,11 +500,6 @@ void BufferView::processUpdateFlags(Update::flags flags)
 	 */
 	buffer_.updateMacros();
 
-	// SinglePar is ignored for now (this should probably change). We
-	// set it ourselves below, at the price of always rebreaking the
-	// paragraph at cursor. This can be expensive for large tables.
-	flags = flags & ~Update::SinglePar;
-
 	// First check whether the metrics and inset positions should be updated
 	if (flags & Update::Force) {
 		// This will update the CoordCache items and replace Force
@@ -512,15 +507,14 @@ void BufferView::processUpdateFlags(Update::flags flags)
 		updateMetrics(flags);
 	}
 
-	// Detect whether we can only repaint a single paragraph.
+	// Detect whether we can only repaint a single paragraph (if we
+	// are not already redrawing all).
 	// We handle this before FitCursor because the later will require
 	// correct metrics at cursor position.
-	if (!(flags & Update::ForceDraw)) {
-		if (singleParUpdate())
-			flags = flags | Update::SinglePar;
-		else
+	if (!(flags & Update::ForceDraw)
+	    && (flags & Update::SinglePar)
+		&& !singleParUpdate())
 			updateMetrics(flags);
-	}
 
 	// Then make sure that the screen contains the cursor if needed
 	if (flags & Update::FitCursor) {
@@ -538,7 +532,7 @@ void BufferView::processUpdateFlags(Update::flags flags)
 	LYXERR(Debug::PAINTING, "Cumulative flags: " << flagsAsString(flags));
 
 	// Now compute the update strategy
-	// Possibly values in flag are None, Decoration, ForceDraw
+	// Possibly values in flag are None, SinglePar, Decoration, ForceDraw
 	LATTEST((d->update_flags_ & ~(Update::None | Update::SinglePar
 	                              | Update::Decoration | Update::ForceDraw)) == 0);