From 1de82e16c1216e1dbe22cf7a8ec9ea9e9e69b631 Mon Sep 17 00:00:00 2001 From: fredizzimo Date: Wed, 5 Jul 2023 16:31:34 +0300 Subject: [PATCH] fix(ui): delay win_viewport until screen update #24182 Problem: Sometimes, when nvim sends the `win_viewport` event, for example when scrolling with visible folds on the screen, it reports the `scroll_delta` value one batch into "future". So when the client application is trying to show the new viewport it's not yet updated, resulting in temporary corruption / screen flickering. For more details see #23609, and starting from [this comment]( https://github.com/neovide/neovide/pull/1790#issuecomment-1518697747) in https://github.com/neovide/neovide/pull/1790,, where the issue was first detected. Note that some of the conclusions in those are not fully accurate, but the general observations are. Solution: When there are pending updates to a Window, delay the `win_viewport` UI event until the updates are sent. This ensures that there's no flush between sending the viewport and updating of the lines corresponding to the new viewport. Document the existing viewport behaviour (for cases where there are no extra flushes), give a hint about how applications can deal with the slightly surprising behaviour of the viewport event being sent after the updates. Fixes https://github.com/neovim/neovim/issues/23609 --- runtime/doc/ui.txt | 7 +++++++ src/nvim/window.c | 4 +++- test/functional/ui/multigrid_spec.lua | 1 - 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/runtime/doc/ui.txt b/runtime/doc/ui.txt index ead5e31e32..ec1d43d878 100644 --- a/runtime/doc/ui.txt +++ b/runtime/doc/ui.txt @@ -643,6 +643,13 @@ tabs. only count as one line. When scrolling more than a full screen it is an approximate value. + All updates, such as `grid_line`, in a batch affects the new viewport, + despite the fact that `win_viewport` is received after the updates. + Applications implementing, for example, smooth scrolling should take + this into account and keep the grid separated from what's displayed on + the screen and copy it to the viewport destination once `win_viewport` + is received. + ["win_extmark", grid, win, ns_id, mark_id, row, col] ~ Updates the position of an extmark which is currently visible in a window. Only emitted if the mark has the `ui_watched` attribute. diff --git a/src/nvim/window.c b/src/nvim/window.c index 187115a4d6..7ac6652cc2 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -1029,7 +1029,9 @@ void ui_ext_win_position(win_T *wp, bool validate) void ui_ext_win_viewport(win_T *wp) { - if ((wp == curwin || ui_has(kUIMultigrid)) && wp->w_viewport_invalid) { + // NOTE: The win_viewport command is delayed until the next flush when there are pending updates. + // This ensures that the updates and the viewport are sent together. + if ((wp == curwin || ui_has(kUIMultigrid)) && wp->w_viewport_invalid && wp->w_redr_type == 0) { const linenr_T line_count = wp->w_buffer->b_ml.ml_line_count; // Avoid ml_get errors when producing "scroll_delta". const linenr_T cur_topline = MIN(wp->w_topline, line_count); diff --git a/test/functional/ui/multigrid_spec.lua b/test/functional/ui/multigrid_spec.lua index 2e66068037..1778c8218b 100644 --- a/test/functional/ui/multigrid_spec.lua +++ b/test/functional/ui/multigrid_spec.lua @@ -3808,7 +3808,6 @@ describe('ext_multigrid', function() ]], win_viewport={ [2] = {win = {id = 1000}, topline = 30586, botline = 30592, curline = 30591, curcol = 0, linecount = 30592, sum_scroll_delta = 30588}; }} - command('autocmd CursorMoved * ++once call line("w$")') -- FIXME: this should not be needed feed('gg') screen:expect{grid=[[ ## grid 1