The branch, biginset, has been updated. - Log -----------------------------------------------------------------
commit c56c1dd5007603ba3f68add0d405de54ed3a418a Author: Jean-Marc Lasgouttes <lasgout...@lyx.org> Date: Mon Nov 20 17:24:09 2023 +0100 Update PAINTING_ANALYSIS diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS index a506371..438d7ca 100644 --- a/development/PAINTING_ANALYSIS +++ b/development/PAINTING_ANALYSIS @@ -3,7 +3,7 @@ Understanding the painting process 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 +sections 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!!! @@ -20,9 +20,10 @@ following section. Some actions are proposed. ** SinglePar update -This flag only has an effect in the current BufferView and at -top-level, but I think it is useful in other views too. Doing this -will require some work on the update pipeline, though. +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 @@ -53,17 +54,16 @@ The global idea would be to extend FitCursor to cover also horizontal cursor. -* Clean-up of drawing code +* TODO Clean-up of drawing code ** Set Row::changed() in a finer way *** singleParUpdate When the height of the current paragraph changes, there is no need for -a full screen update. Only the rows after the current one need to have -their position recomputed. +a full screen update (at top level, at least). Only the rows after the +current one need to have their position recomputed. -This is also true when scrolling (how to do that?) *** redoParagraph @@ -71,13 +71,16 @@ It should be possible to check whether the new row is the same as the old one and keep its changed() status in this case. This would reduce a lot the amount of stuff to redraw. + ** Put labels and friends in the Row as elements It should not be necessary to access the Paragraph object to draw. Adding the static elements to Row is a lot of work, but worth it IMO. + ** When a paragraph ends with a newline, compute correctly the height of the extra row. + ** Merging bv::updateMetrics and tm::metrics While the full metrics computation tries hard to limit the number of @@ -89,6 +92,12 @@ insets. We should re-use the bv::updateMetrics logic: 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. +Also, the anchor is relative to the outer paragraph, which means that +for a very long inset it is necessary to rebreak until the contents +that needs to be shown (to compute the heights). + +All in all, this is difficult to get right. This is less important now +that SinglePar updates work in nested insets. * Description of current drawing mechanism @@ -99,10 +108,12 @@ There are three 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. + normal and math) in bv::coordCache and the vertical position of the + top-level paragraphs. + the nodraw drawing phase paints the screen (see below) with a null - painter. The only useful effect is to store the inset positions. + painter. The only useful effect is to store the positions of + visible insets. + an update() signal is sent. This in turn will trigger a paint event, and the actual screen painting will happen then. @@ -115,18 +126,18 @@ 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: +DecorationUpdate). It triggers a call to updateMetrics when either: + Update::Force has been specified + Update::FitCursor has been specified and there is a need to scroll the display. + Update::SinglePar has been specified and the current paragraph has - not changed height. + 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 processUpateFlags several times before an update. In +It is OK to call processUpdateFlags 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. @@ -137,21 +148,32 @@ update flag is Update::None. ** 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 -for page up/down) and computed as needed. +This is done bv::updateMetrics. When the parameter 'force' of this +method is true, that first thing that is done is to clear the metrics +caches to start from a clean slate. + +Then, starting upwards then downwards from the anchor paragraph +(anchor_pit_) and its vertical position (anchor_ypos_), +tm::updateMetrics every visible paragraph whose metrics is not know +(all of them when force==true) is recomputed using tm::redoParagraph. 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. +all the paragraphs of the inset. Then, a single big row is created in +tm::tokenizeParagraph, which is later broken in multiple rows by +tm::breakParagraph. -Then, a single big row is created in tm::tokenizeParagraph, which is -later broken in multiple rows by tm::breakParagraph. +If it turns out that the top or bottom margin are incorrect (paragraphs +are too high/low), tm::updateMetrics will be called again with fixed +values of anchor_ypos_ (this does not incur much extra work). -At the end of the function, bv::updatePosCache is called. It triggers +At the end of bv::updateMetrics, 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. +This way of working means that scrolling can be achieved by just +updating anchor_ypos_ and calling bv::processUpdateFlags(Update::ForceDraw). + ** Drawing the work area. This is done in bv::draw. This method is triggered by a paint event, commit 1b664fd930c3cdd88eb573f5b287e58f1ec6ceb4 Author: Jean-Marc Lasgouttes <lasgout...@lyx.org> Date: Fri Nov 17 18:30:37 2023 +0100 Improve the code that limits scrolling at top/bottom The most visible part of this commit is the move of part of BufferView::updateMetrics to a new TextMetrics::updateMetrics. This new method makes sure that metrics are known for all visible paragraphs (starting from anchor), and that the positions of the paragraphs have been recorded. This method is called up to 3 times in BufferView::updateMetrics: * unconditionally, to update all visible metrics, * then, if the bottom of the document is visible and too high, after updating the anchor ypos, * and similarly if the top of the document is visible and too low. This fixes for example the case where one jumps to Section 5.3 at the end of Tutorial and 'scroll_below_document' is false. Some now redundant code is removed from BufferView::scrollToCursor. The anchor-setting code in BufferView::draw is not clearly useful, but left here just in case. It generates a debug warning, though. Part of bug #12297. diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 5aab06c..2693f4e 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -765,6 +765,7 @@ void BufferView::scrollDocView(int const pixels, bool update) TextMetrics const & tm = textMetrics(&buffer_.text()); if (tm.first().second->top() - pixels <= height_ && tm.last().second->bottom() - pixels >= 0) { + LYXERR(Debug::SCROLLING, "small skip"); d->anchor_ypos_ -= pixels; processUpdateFlags(Update::ForceDraw); return; @@ -787,6 +788,7 @@ void BufferView::scrollDocView(int const pixels, bool update) return; } + LYXERR(Debug::SCROLLING, "search paragraph"); // find paragraph at target position int par_pos = d->scrollbarParameters_.min; pit_type i = 0; @@ -1100,8 +1102,6 @@ bool BufferView::scrollToCursor(DocIterator const & dit, ScrollType how) d->anchor_ypos_ = - offset + row_dim.ascent(); if (how == SCROLL_CENTER) d->anchor_ypos_ += height_/2 - row_dim.height() / 2; - else if (!lyxrc.scroll_below_document && d->anchor_pit_ == max_pit) - d->anchor_ypos_ = height_ - offset - row_dim.descent(); else if (offset > height_) d->anchor_ypos_ = height_ - offset - defaultRowHeight(); else @@ -3114,7 +3114,7 @@ void BufferView::updateMetrics(bool force) return; Text & buftext = buffer_.text(); - pit_type const npit = int(buftext.paragraphs().size()); + pit_type const lastpit = int(buftext.paragraphs().size()) - 1; if (force) { // Clear out the position cache in case of full screen redraw, @@ -3133,62 +3133,35 @@ void BufferView::updateMetrics(bool force) if (d->inlineCompletionPos_.fixIfBroken()) d->inlineCompletionPos_ = DocIterator(); - if (d->anchor_pit_ >= npit) + if (d->anchor_pit_ > lastpit) // The anchor pit must have been deleted... - d->anchor_pit_ = npit - 1; + d->anchor_pit_ = lastpit; - if (!tm.contains(d->anchor_pit_)) - // Rebreak anchor paragraph. - tm.redoParagraph(d->anchor_pit_); - ParagraphMetrics & anchor_pm = tm.parMetrics(d->anchor_pit_); + // Update metrics around the anchor + tm.updateMetrics(d->anchor_pit_, d->anchor_ypos_, height_); - // make sure than first paragraph of document is not too low - if (d->anchor_pit_ == 0) { - int scrollRange = d->scrollbarParameters_.max - d->scrollbarParameters_.min; + // Check that the end of the document is not too high + int const min_visible = lyxrc.scroll_below_document ? minVisiblePart() : height_; + if (tm.last().first == lastpit && tm.last().second->bottom() < min_visible) { + d->anchor_ypos_ += min_visible - tm.last().second->bottom(); + LYXERR(Debug::SCROLLING, "Too high, adjusting anchor ypos to " << d->anchor_ypos_); + tm.updateMetrics(d->anchor_pit_, d->anchor_ypos_, height_); + } - // Complete buffer visible? Then it's easy. - if (scrollRange == 0) - d->anchor_ypos_ = anchor_pm.ascent(); - else { - // avoid empty space above the first row - d->anchor_ypos_ = min(d->anchor_ypos_, anchor_pm.ascent()); - } - } - anchor_pm.setPosition(d->anchor_ypos_); - - LYXERR(Debug::PAINTING, "metrics: " << " anchor pit = " << d->anchor_pit_ - << " anchor ypos = " << d->anchor_ypos_); - - // Redo paragraphs above anchor if necessary. - int y1 = d->anchor_ypos_ - anchor_pm.ascent(); - // We are now just above the anchor paragraph. - pit_type pit1 = d->anchor_pit_ - 1; - for (; pit1 >= 0 && y1 > 0; --pit1) { - if (!tm.contains(pit1)) - tm.redoParagraph(pit1); - ParagraphMetrics & pm = tm.parMetrics(pit1); - y1 -= pm.descent(); - // Save the paragraph position in the cache. - pm.setPosition(y1); - y1 -= pm.ascent(); - } - - // Redo paragraphs below the anchor if necessary. - int y2 = d->anchor_ypos_ + anchor_pm.descent(); - // We are now just below the anchor paragraph. - pit_type pit2 = d->anchor_pit_ + 1; - for (; pit2 < npit && y2 < height_; ++pit2) { - if (!tm.contains(pit2)) - tm.redoParagraph(pit2); - ParagraphMetrics & pm = tm.parMetrics(pit2); - y2 += pm.ascent(); - // Save the paragraph position in the cache. - pm.setPosition(y2); - y2 += pm.descent(); - } - - //FIXME: do we want that? - // if updating, remove paragraphs that are outside of screen + // Check that the start of the document is not too low + if (tm.first().first == 0 && tm.first().second->top() > 0) { + d->anchor_ypos_ -= tm.first().second->top(); + LYXERR(Debug::SCROLLING, "Too low, adjusting anchor ypos to " << d->anchor_ypos_); + tm.updateMetrics(d->anchor_pit_, d->anchor_ypos_, height_); + } + + /* FIXME: do we want that? It avoids potential issues with old + * paragraphs that should have been recomputed but have not, at + * the price of potential extra metrics computaiton. I do not + * think that the performance gain is high, so that for now the + * extra paragraphs are removed + */ + // Remove paragraphs that are outside of screen while(tm.first().second->bottom() <= 0) { //LYXERR0("Forget pit: " << tm.first().first); tm.forget(tm.first().first); @@ -3198,6 +3171,8 @@ void BufferView::updateMetrics(bool force) tm.forget(tm.last().first); } + /* FIXME: if paragraphs outside of the screen are not removed + * above, one has to search for the first visible one here */ // Normalize anchor for next time if (d->anchor_pit_ != tm.first().first || d->anchor_ypos_ != tm.first().second->position()) { @@ -3208,14 +3183,6 @@ void BufferView::updateMetrics(bool force) d->anchor_ypos_ = tm.first().second->position(); } - LYXERR(Debug::PAINTING, "Metrics: " - << " anchor pit = " << d->anchor_pit_ - << " anchor ypos = " << d->anchor_ypos_ - << " y1 = " << y1 - << " y2 = " << y2 - << " pit1 = " << pit1 - << " pit2 = " << pit2); - // Now update the positions of insets in the cache. updatePosCache(); @@ -3683,6 +3650,7 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) updateScrollbarParameters(); // Normalize anchor for next time (in case updateMetrics did not do it yet) + // FIXME: is this useful? pair<pit_type, ParagraphMetrics const *> firstpm = tm.first(); pair<pit_type, ParagraphMetrics const *> lastpm = tm.last(); for (pit_type pit = firstpm.first; pit <= lastpm.first; ++pit) { @@ -3690,9 +3658,10 @@ void BufferView::draw(frontend::Painter & pain, bool paint_caret) if (pm.bottom() > 0) { if (d->anchor_pit_ != pit || d->anchor_ypos_ != pm.position()) - LYXERR(Debug::PAINTING, __func__ << ": Found new anchor pit = " << pit + LYXERR0(__func__ << ": Found new anchor pit = " << pit << " anchor ypos = " << pm.position() - << " (was " << d->anchor_pit_ << ", " << d->anchor_ypos_ << ")"); + << " (was " << d->anchor_pit_ << ", " << d->anchor_ypos_ << ")" + "\nIf you see this message, please report."); d->anchor_pit_ = pit; d->anchor_ypos_ = pm.position(); break; diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index ddff853..62b877f 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -216,6 +216,56 @@ void TextMetrics::newParMetricsUp() } +void TextMetrics::updateMetrics(pit_type const anchor_pit, int const anchor_ypos, + int const bv_height) +{ + LASSERT(text_->isMainText(), return); + pit_type const npit = pit_type(text_->paragraphs().size()); + + if (!contains(anchor_pit)) + // Rebreak anchor paragraph. + redoParagraph(anchor_pit); + ParagraphMetrics & anchor_pm = parMetrics(anchor_pit); + anchor_pm.setPosition(anchor_ypos); + + // Redo paragraphs above anchor if necessary. + int y1 = anchor_ypos - anchor_pm.ascent(); + // We are now just above the anchor paragraph. + pit_type pit1 = anchor_pit - 1; + for (; pit1 >= 0 && y1 > 0; --pit1) { + if (!contains(pit1)) + redoParagraph(pit1); + ParagraphMetrics & pm = parMetrics(pit1); + y1 -= pm.descent(); + // Save the paragraph position in the cache. + pm.setPosition(y1); + y1 -= pm.ascent(); + } + + // Redo paragraphs below the anchor if necessary. + int y2 = anchor_ypos + anchor_pm.descent(); + // We are now just below the anchor paragraph. + pit_type pit2 = anchor_pit + 1; + for (; pit2 < npit && y2 < bv_height; ++pit2) { + if (!contains(pit2)) + redoParagraph(pit2); + ParagraphMetrics & pm = parMetrics(pit2); + y2 += pm.ascent(); + // Save the paragraph position in the cache. + pm.setPosition(y2); + y2 += pm.descent(); + } + + LYXERR(Debug::PAINTING, "TextMetrics::updateMetrics " + << " anchor pit = " << anchor_pit + << " anchor ypos = " << anchor_ypos + << " y1 = " << y1 + << " y2 = " << y2 + << " pit1 = " << pit1 + << " pit2 = " << pit2); +} + + bool TextMetrics::metrics(MetricsInfo const & mi, Dimension & dim, int min_width) { LBUFERR(mi.base.textwidth > 0); diff --git a/src/TextMetrics.h b/src/TextMetrics.h index f53701b..d5606b6 100644 --- a/src/TextMetrics.h +++ b/src/TextMetrics.h @@ -73,6 +73,10 @@ public: /// void newParMetricsUp(); + /// Update metrics up to \c bv_height. Only usable for main text (for now). + void updateMetrics(pit_type const anchor_pit, int const anchor_ypos, + int const bv_height); + /// compute text metrics. bool metrics(MetricsInfo const & mi, Dimension & dim, int min_width = 0); commit dfe7aec516b2e3d1201e94dbf9841d0006788bf6 Author: Jean-Marc Lasgouttes <lasgout...@lyx.org> Date: Mon Nov 20 12:09:53 2023 +0100 fixup quickscroll diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 666b736..5aab06c 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -3189,7 +3189,7 @@ void BufferView::updateMetrics(bool force) //FIXME: do we want that? // if updating, remove paragraphs that are outside of screen - while(tm.first().second->bottom() < 0) { + while(tm.first().second->bottom() <= 0) { //LYXERR0("Forget pit: " << tm.first().first); tm.forget(tm.first().first); } ----------------------------------------------------------------------- Summary of changes: development/PAINTING_ANALYSIS | 62 +++++++++++++++++-------- src/BufferView.cpp | 101 ++++++++++++++--------------------------- src/TextMetrics.cpp | 50 ++++++++++++++++++++ src/TextMetrics.h | 4 ++ 4 files changed, 131 insertions(+), 86 deletions(-) hooks/post-receive -- Repository for new features -- lyx-cvs mailing list lyx-cvs@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-cvs