]> git.lyx.org Git - features.git/commitdiff
Add a file that describe the current state of painting
authorJean-Marc Lasgouttes <lasgouttes@lyx.org>
Fri, 2 Oct 2015 13:30:16 +0000 (15:30 +0200)
committerJean-Marc Lasgouttes <lasgouttes@lyx.org>
Fri, 2 Oct 2015 13:30:16 +0000 (15:30 +0200)
This is a work in progress intended to start collective work towards improving the performance of our painting process.

The intent is to make it a living document that is updated as code evolves.

development/PAINTING_ANALYSIS [new file with mode: 0644]

diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS
new file mode 100644 (file)
index 0000000..12b9983
--- /dev/null
@@ -0,0 +1,159 @@
+# -*- org-mode -*-
+This file tries to describe the state of the metrics/painting
+mechanism, and identify the improvements that could be made. The first
+section can be read alone, although the context for them is really
+given in the following ones.
+
+Please keep this file up to date as the code evolves!!!
+
+Abbreviations:
+bv: BufferView
+pm: ParagraphMetrics
+tm::TextMetrics
+
+* Questions / Ideas
+
+These questions are consequences of the description made in the
+following section. Some actions are proposed.
+
+** Inset cache
+
+Why do we store inset dimensions both in bv::coordCache and in
+pm::insetDimension? The later one is bad when same
+buffer is shown in different views.
+
+I propose to get rid of pm::insetDimension.
+
+** 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.
+
+** Two phase drawing
+
+Why is it necessary to set the inset positions in the drawing phase?
+It seems to me that everything can be done in the metrics phase (at
+least for non-math insets).
+
+** Buffer::change issues
+
+When calling Buffer::changed outside of bv::processUpdateFlags,
+how do we now that the update strategy is set correctly? It is
+possible to reset the strategy at the end of bv::draw. What would be
+a good value? NoScreenUpdate?
+
+On a related note, what is the semantics of a call to
+Buffer::changed(false)? What does the caller mean?
+
+** Metrics outside of visible area
+
+Why is it required to compute metrics on page above and below the
+visible area? Couldn't this be done on demand? I suspect this could be
+made transparent by doing it in the proper metrics-fetching method.
+
+** What happens with FitCursor when the cursor is already OK?
+
+In this case, we invoke Buffer::change(false) with drawing disabled,
+which means that the paint machinery is invoked to update inset
+positions. Why is this necessary at all?
+
+** Merging bv::updateMetrics and tm::metrics
+
+While the full metrics computation tries hard to limit the number of
+paragraphs that are rebroken, the version that is used for inner inset
+does not try any such optimization. This can be very bad for very tall
+insets. How difficult would it be to re-use the bv::updateMetrics logic?
+
+
+* Two phase drawing
+
+There are two parts to drawing the work area:
+
+ + the metrics phase computes the size of insets and breaks the
+   paragraphs into rows. It stores the dimension of insets (both
+   normal and math) in bv::coordCache, and the size of normal
+   insets in pm::insetDimension.
+
+ + the drawing phase draws the contents and caches the inset
+   positions. Since the caching of positions is useful in itself,
+   there is a provision for drawing "without" drawing when the only
+   thing we want is to cache inset positions
+   (Painter::setDrawingEnabled).
+
+
+The machinery is controlled via bv::processUpdateFlags. This method is
+called at the end of bv::mouseEventDispatch and in
+GuiApplication::dispatch, via the updateCurrentView method. There are
+also several calls in code related to dialogs. We should evaluate
+whether this is correct.
+
+Depending on the Update::flags passed to the method, it sets an update
+strategy in (NoScreenUpdate, SingleParUpdate, FullScreenUpdate,
+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, has the same height as in
+   existing metrics. Note that the Update::SinglePar flag is *never*
+   taken into account.
+
+The screen is drawn (with appropriate update strategy), except when
+update flag is Update::None.
+
+
+** Metrics computation
+
+This is triggered by bv::updateMetrics, which calls tm::redoParagraph for
+  + all visible paragraphs
+  + paragraph above the screen (up to one page)
+  + paragraphs below the screen (up to one page again)
+
+The paragraphs outside of the screen are required to make PageUp/Down
+work.
+
+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.
+
+
+** Drawing the work area.
+
+This is done in bv::draw. This method is triggered mainly by
+Buffer::changed, which draws all the work areas that show the given buffer.
+
+Note that, When Buffer::changed is called outside of
+bv::processUpdateFlags, it is not clear whether the update strategy
+has been set to a reasonable value beforehand.
+
+The action depends on the update strategy:
+
+ + NoScreenUpdate: repaint the whole screen with drawing disabled.
+   This is only needed to update the inset positions. I am not sure
+   when this is necessary, actually. This is triggered when:
+   - Update::FitCursor is set but the cursor is already visible. It is
+     not clear why something needs to be done in this case, since this
+     should be equivalent to Update::None.
+   - this is also set when update flag is Update::None, but this value
+     is AFAICS not acted on in the code (meaning that nothing happens
+     at all in this case).
+
+ + FullScreenUpdate: repaint the whole screen. This is set when:
+   - updateMetrics has been invoked.
+   - scroll value of current row has changed (although this should not
+     be necessary).
+
+ + DecorationUpdate: actually like FullScreenUpdate, although it is
+   intended to only update inset decorations. This triggers when:
+   - Update::Decoration is set (without Update::Force) as flag.
+   - An hovered inset is detected.
+
+ + SingleParUpdate: only tries to repaint current paragraph in a way
+   that is not yet very clear to me.