From 9c443d965114318dacbb67db68f5a3fd9b1b8fc2 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Wed, 4 Sep 2024 15:48:38 +0200 Subject: [PATCH] Cleanup Page Up/Down code Since BufferView::scroll() does not correct scrolling amount at top/bottom anymore, remove its return value and add a new one to BufferView::updateMetrics(bool) that returns this correction. Rely on updateMetrics(false) to set the metrics straight after moving the anchor y position. Use assert_in_view = false when calling TextMetrics::editXY() because this parameter does not work as advertised: if an inset not totally visible, the code will not try to go inside it to look for a smaller row that is totally visible. Two small (hopfully useful) modifications: - take into account the cursor X target instead of its screen position - adapt the cursor position depending on lyxrc.mac_like_cursor_movement when hitting start or end of document. --- src/BufferView.cpp | 37 +++++++++++++++++++++---------------- src/BufferView.h | 18 +++++++++--------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/BufferView.cpp b/src/BufferView.cpp index 75d145fa75..e58612ef49 100644 --- a/src/BufferView.cpp +++ b/src/BufferView.cpp @@ -2128,6 +2128,8 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr) case LFUN_SCREEN_UP: case LFUN_SCREEN_DOWN: { Point p = getPos(cur); + // replace x by the cursor target + p.x = cur.targetX(); // This code has been commented out to enable to scroll down a // document, even if there are large insets in it (see bug #5465). /*if (p.y < 0 || p.y > height_) { @@ -2135,29 +2137,28 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr) showCursor(); p = getPos(cur); }*/ - int const scrolled = scroll(act == LFUN_SCREEN_UP - ? -height_ : height_); - if (act == LFUN_SCREEN_UP && scrolled > -height_) - p = Point(0, 0); - if (act == LFUN_SCREEN_DOWN && scrolled < height_) - p = Point(width_, height_); + scroll(act == LFUN_SCREEN_UP ? -height_ : height_); + // update metrics + int const correction = updateMetrics(false); + if (act == LFUN_SCREEN_UP && correction < 0) + p = Point(lyxrc.mac_like_cursor_movement ? 0 : p.x, 0); + if (act == LFUN_SCREEN_DOWN && correction > 0) + p = Point(lyxrc.mac_like_cursor_movement ? width_ : p.x, height_); bool const in_texted = cur.inTexted(); cur.setCursor(doc_iterator_begin(cur.buffer())); cur.selHandle(false); - // Force an immediate computation of metrics because we need it below - if (scrolled) - processUpdateFlags(Update::Force); d->text_metrics_[&buffer_.text()].editXY(cur, p.x, p.y, - true, act == LFUN_SCREEN_UP); + false, act == LFUN_SCREEN_UP); //FIXME: what to do with cur.x_target()? bool update = in_texted && cur.bv().checkDepm(cur, old); cur.finishUndo(); - if (update || cur.mark()) + if (update) { dr.screenUpdate(Update::Force | Update::FitCursor); - if (update) dr.forceBufferUpdate(); + } else + dr.screenUpdate(Update::ForceDraw | Update::FitCursor); break; } @@ -2858,10 +2859,9 @@ int BufferView::minVisiblePart() } -int BufferView::scroll(int pixels) +void BufferView::scroll(int pixels) { d->anchor_ypos_ -= pixels; - return -pixels; } @@ -3220,10 +3220,10 @@ void BufferView::updateMetrics() } -void BufferView::updateMetrics(bool force) +int BufferView::updateMetrics(bool force) { if (!ready()) - return; + return 0; //LYXERR0("updateMetrics " << _v_(force)); @@ -3256,6 +3256,7 @@ void BufferView::updateMetrics(bool force) tm.updateMetrics(d->anchor_pit_, d->anchor_ypos_, height_); // Check that the end of the document is not too high + int const old_ypos = d->anchor_ypos_; int const min_visible = lyxrc.scroll_below_document ? minVisiblePart() : height_; if (tm.last().first == lastpit && tm.last().second->hasPosition() && tm.last().second->bottom() < min_visible) { @@ -3272,6 +3273,9 @@ void BufferView::updateMetrics(bool force) tm.updateMetrics(d->anchor_pit_, d->anchor_ypos_, height_); } + // The global adjustment that have been made above + int const correction = d->anchor_ypos_ - old_ypos; + /* 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 computation. I do not @@ -3307,6 +3311,7 @@ void BufferView::updateMetrics(bool force) LYXERR(Debug::WORKAREA, "BufferView::updateMetrics"); d->coord_cache_.dump(); } + return correction; } diff --git a/src/BufferView.h b/src/BufferView.h index 57d602de7f..e67895fa50 100644 --- a/src/BufferView.h +++ b/src/BufferView.h @@ -229,7 +229,7 @@ public: /// scroll the view by the given number of pixels. This only /// updates the anchor vertical position, but does not recompute /// metrics nor trigger a screen refresh. - int scroll(int pixels); + void scroll(int pixels); /// Scroll the view by a number of pixels. void scrollDocView(int pixels); /// Set the cursor position based on the scrollbar one. @@ -418,15 +418,15 @@ private: /// Update current paragraph metrics. /// \return true if no further update is needed. bool singleParUpdate(); - /** Helper for the public updateMetrics() and for processUpdateFlags() - * * When \c force is true, get rid of all paragraph metrics and - rebuild them anew. - * * When it is false, keep the paragraphs that are still visible in - * WorkArea and rebuild the missing ones. - * - * This does also set the anchor paragraph and its position correctly + /** Helper for the public updateMetrics() and for processUpdateFlags(). + * This does also set the anchor paragraph and its position correctly. + * \param force when true, get rid of all paragraph metrics and + * rebuild them anew. Otherwise keep the paragraphs that are + * still visible in work area and rebuild the missing ones. + * \return the correction needed (same sign as anchor vertical + * position change) when hitting top or bottom constraints. */ - void updateMetrics(bool force); + int updateMetrics(bool force); // Set the row on which the cursor lives. void setCurrentRowSlice(CursorSlice const & rowSlice);