[LyX/master] Allow multiple calls to processUpdateFlags before redraw

2017-10-25 Thread Jean-Marc Lasgouttes
commit 8d8988de475bf2055f253823e53fd5627be5de78
Author: Jean-Marc Lasgouttes 
Date:   Wed Oct 11 18:00:48 2017 +0200

Allow multiple calls to processUpdateFlags before redraw

The goal of this commit is to ensure that a processUpdateFlags call
that requires no redraw will not override a previous one that did
require a redraw.

To this end, the semantics of the flag argument is now different: its
value is now OR'ed with a private update_flags_ variable. This
variable is only reset after the buffer view has actually been
redrawn.

A new Update::ForceRedraw flag has been added. It requires a full
redraw but no metrics computation. It is not used in the main code
(yet), but avoids to compute metrics repeatedly in consecutive
processUpdateFlags calls.

The process is now as follows:
- if flags is just None, return immediately, there is nothing to do.
- the Force flag is honored (full metrics computation) and replaced
  with ForceDraw.
- the FitCursor flag is honored and removed from the flags.
- the SinglePar update is added if ForceDraw is not in flags and only
  the current par has been modified.

The remaining flags are only then added to the BufferView update
flags, and the update strategy is computed for the next paint event.

Finally the dubious call to updateMacros in updateMetrics has been
removed for performance reasons.
---
 development/PAINTING_ANALYSIS |   21 +++--
 src/BufferView.cpp|  176 ++---
 src/BufferView.h  |9 ++-
 src/TextMetrics.cpp   |7 +-
 src/update_flags.h|   14 +++-
 5 files changed, 127 insertions(+), 100 deletions(-)

diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS
index f734edb..ec3566e 100644
--- a/development/PAINTING_ANALYSIS
+++ b/development/PAINTING_ANALYSIS
@@ -60,12 +60,6 @@ cursor.
 
 * Clean-up of drawing code
 
-The goal is to make painting with drawing disable fast enough that it
-can be used after every metrics computation. Then we can separate real
-drawing from metrics.
-
-Other changes are only clean-ups.
-
 ** When a paragraph ends with a newline, compute correctly the height of the 
extra row.
 ** Merging bv::updateMetrics and tm::metrics
 
@@ -76,7 +70,7 @@ insets. We should re-use the bv::updateMetrics logic:
  + transfer all the logic of bv::updateMetrics to tm.
  + Main InsetText should not be special.
 
-The difficuly for a tall table cell for example, is that it may be
+The difficulty for a tall table cell for example, is that it may be
 necessary to break the whole contents to know the width of the cell.
 
 
@@ -113,11 +107,19 @@ DecorationUpdate). It triggers a recomputation of the 
metrics when either:
existing metrics. Note that the Update::SinglePar flag is *never*
taken into account.
 
+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
+metrics-related actions, but defers the actual drawing to the next
+paint event.
+
 The screen is drawn (with appropriate update strategy), except when
 update flag is Update::None.
 
 
-** Metrics computation
+** Metrics computation (and nodraw drawing phase)
 
 This is triggered by bv::updateMetrics, which calls tm::redoParagraph for
 all visible paragraphs. Some Paragraphs above or below the screen (needed
@@ -127,6 +129,9 @@ tm::redoParagraph will call Inset::metrics for each inset. 
In the case
 of text insets, this will invoke recursively tm::metrics, which redoes
 all the paragraphs of the inset.
 
+At the end of the function, bv::updatePosCache is called. It triggers
+a repaint of the document with a NullPainter (a painter that does
+nothing). This has the effect of caching all insets positions.
 
 ** Drawing the work area.
 
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 53d374f..4e2428d 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -228,7 +228,8 @@ enum ScreenUpdateStrategy {
 
 struct BufferView::Private
 {
-   Private(BufferView & bv) : update_strategy_(NoScreenUpdate),
+   Private(BufferView & bv) : update_strategy_(FullScreenUpdate),
+   update_flags_(Update::Force),
wh_(0), cursor_(bv),
anchor_pit_(0), anchor_ypos_(0),
inlineCompletionUniqueChars_(0),
@@ -245,6 +246,8 @@ struct BufferView::Private
///
ScreenUpdateStrategy update_strategy_;
///
+   Update::flags update_flags_;
+   ///
CoordCache coord_cache_;
 
/// Estimated average par height for scrollbar.
@@ -445,79 +448,85 @@ bool BufferView::needsFitCursor() const
 }
 
 
-void BufferView::processUpdateFlags(Update::flags flags)
+namespace {
+
+// this is 

[LyX/master] Allow multiple calls to processUpdateFlags before redraw

2017-10-16 Thread Jean-Marc Lasgouttes
commit c19c54dd5b85726df1b5187616d17d5430028c16
Author: Jean-Marc Lasgouttes 
Date:   Wed Oct 11 18:00:48 2017 +0200

Allow multiple calls to processUpdateFlags before redraw

The goal of this commit is to ensure that a processUpdateFlags call
that requires no redraw will not override a previous one that did
require a redraw.

To this end, the semantics of the flag argument is now different: its
value is now OR'ed with a private update_flags_ variable. This
variable is only reset after the buffer view has actually been
redrawn.

A new Update::ForceRedraw flag has been added. It requires a full
redraw but no metrics computation. It is not used in the main code
(yet), but avoids to compute metrics repeatedly in consecutive
processUpdateFlags calls.

Finally the dubious call to updateMacros in updateMetrics has been
removed for performance reasons.
---
 development/PAINTING_ANALYSIS |   21 +--
 src/BufferView.cpp|   55 
 src/BufferView.h  |7 +++--
 src/update_flags.h|   14 --
 4 files changed, 61 insertions(+), 36 deletions(-)

diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS
index f734edb..ec3566e 100644
--- a/development/PAINTING_ANALYSIS
+++ b/development/PAINTING_ANALYSIS
@@ -60,12 +60,6 @@ cursor.
 
 * Clean-up of drawing code
 
-The goal is to make painting with drawing disable fast enough that it
-can be used after every metrics computation. Then we can separate real
-drawing from metrics.
-
-Other changes are only clean-ups.
-
 ** When a paragraph ends with a newline, compute correctly the height of the 
extra row.
 ** Merging bv::updateMetrics and tm::metrics
 
@@ -76,7 +70,7 @@ insets. We should re-use the bv::updateMetrics logic:
  + transfer all the logic of bv::updateMetrics to tm.
  + Main InsetText should not be special.
 
-The difficuly for a tall table cell for example, is that it may be
+The difficulty for a tall table cell for example, is that it may be
 necessary to break the whole contents to know the width of the cell.
 
 
@@ -113,11 +107,19 @@ DecorationUpdate). It triggers a recomputation of the 
metrics when either:
existing metrics. Note that the Update::SinglePar flag is *never*
taken into account.
 
+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
+metrics-related actions, but defers the actual drawing to the next
+paint event.
+
 The screen is drawn (with appropriate update strategy), except when
 update flag is Update::None.
 
 
-** Metrics computation
+** Metrics computation (and nodraw drawing phase)
 
 This is triggered by bv::updateMetrics, which calls tm::redoParagraph for
 all visible paragraphs. Some Paragraphs above or below the screen (needed
@@ -127,6 +129,9 @@ tm::redoParagraph will call Inset::metrics for each inset. 
In the case
 of text insets, this will invoke recursively tm::metrics, which redoes
 all the paragraphs of the inset.
 
+At the end of the function, bv::updatePosCache is called. It triggers
+a repaint of the document with a NullPainter (a painter that does
+nothing). This has the effect of caching all insets positions.
 
 ** Drawing the work area.
 
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 53d374f..2c79c34 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -228,7 +228,8 @@ enum ScreenUpdateStrategy {
 
 struct BufferView::Private
 {
-   Private(BufferView & bv) : update_strategy_(NoScreenUpdate),
+   Private(BufferView & bv) : update_strategy_(FullScreenUpdate),
+   update_flags_(Update::Force),
wh_(0), cursor_(bv),
anchor_pit_(0), anchor_ypos_(0),
inlineCompletionUniqueChars_(0),
@@ -245,6 +246,8 @@ struct BufferView::Private
///
ScreenUpdateStrategy update_strategy_;
///
+   Update::flags update_flags_;
+   ///
CoordCache coord_cache_;
 
/// Estimated average par height for scrollbar.
@@ -447,16 +450,16 @@ bool BufferView::needsFitCursor() const
 
 void BufferView::processUpdateFlags(Update::flags flags)
 {
-   // This is close to a hot-path.
-   LYXERR(Debug::PAINTING, "BufferView::processUpdateFlags()"
-   << "[fitcursor = " << (flags & Update::FitCursor)
-   << ", forceupdate = " << (flags & Update::Force)
-   << ", singlepar = " << (flags & Update::SinglePar)
-   << "]  buffer: " << _);
+   // The update flags are reset to None after the redraw is actually done
+   d->update_flags_ = d->update_flags_ | flags;
 
-   // FIXME Does this really need doing here? It's done in updateBuffer, 
and
-   // if the Buffer