Merge pull request #8771 from bfredl/glitches

Fix redraw glitches with newgrid refactor

Fixes #8768 #8775 

6 files changed, 134 insertions, 20 deletions

diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
index 63c2c4a1b9..d0db43c588 100644
--- a/src/nvim/api/ui.c
+++ b/src/nvim/api/ui.c
@@ -302,6 +302,15 @@ static void remote_ui_grid_scroll(UI *ui, Integer grid, Integer top,
     args = (Array)ARRAY_DICT_INIT;
     ADD(args, INTEGER_OBJ(rows));
     push_call(ui, "scroll", args);
+
+    // some clients have "clear" being affected by scroll region,
+    // so reset it.
+    args = (Array)ARRAY_DICT_INIT;
+    ADD(args, INTEGER_OBJ(0));
+    ADD(args, INTEGER_OBJ(ui->height-1));
+    ADD(args, INTEGER_OBJ(0));
+    ADD(args, INTEGER_OBJ(ui->width-1));
+    push_call(ui, "set_scroll_region", args);
   }
 }
 
diff --git a/src/nvim/screen.c b/src/nvim/screen.c
index 65a3c17286..e8dbc11710 100644
--- a/src/nvim/screen.c
+++ b/src/nvim/screen.c
@@ -283,8 +283,11 @@ void update_screen(int type)
   if (msg_scrolled) {
     clear_cmdline = true;
     if (dy_flags & DY_MSGSEP) {
+      int valid = MAX(Rows - msg_scrollsize(), 0);
+      if (valid == 0) {
+        redraw_tabline = true;
+      }
       FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
-        int valid = Rows - msg_scrollsize();
         if (wp->w_winrow + wp->w_height > valid) {
           wp->w_redr_type = NOT_VALID;
           wp->w_lines_valid = 0;
@@ -292,9 +295,6 @@ void update_screen(int type)
         if (wp->w_winrow + wp->w_height + wp->w_status_height > valid) {
           wp->w_redr_status = true;
         }
-        if (valid == 0) {
-          redraw_tabline = true;
-        }
       }
     } else if (msg_scrolled > Rows - 5) {  // clearing is faster
       type = CLEAR;
diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c
index 56c47ed6cc..cfcfaa83a0 100644
--- a/src/nvim/tui/tui.c
+++ b/src/nvim/tui/tui.c
@@ -823,7 +823,7 @@ static void tui_grid_clear(UI *ui, Integer g)
   UGrid *grid = &data->grid;
   ugrid_clear(grid);
   kv_size(data->invalid_regions) = 0;
-  clear_region(ui, grid->top, grid->bot, grid->left, grid->right,
+  clear_region(ui, 0, grid->height-1, 0, grid->width-1,
                data->clear_attrs);
 }
 
diff --git a/src/nvim/ugrid.c b/src/nvim/ugrid.c
index 48f3cff2d7..36936970f8 100644
--- a/src/nvim/ugrid.c
+++ b/src/nvim/ugrid.c
@@ -44,7 +44,7 @@ void ugrid_resize(UGrid *grid, int width, int height)
 
 void ugrid_clear(UGrid *grid)
 {
-  clear_region(grid, grid->top, grid->bot, grid->left, grid->right,
+  clear_region(grid, 0, grid->height-1, 0, grid->width-1,
                HLATTRS_INIT);
 }
 
diff --git a/test/functional/ui/screen.lua b/test/functional/ui/screen.lua
index d71d8cf3a8..c40b2210ff 100644
--- a/test/functional/ui/screen.lua
+++ b/test/functional/ui/screen.lua
@@ -73,6 +73,7 @@
 
 local helpers = require('test.functional.helpers')(nil)
 local request, run, uimeths = helpers.request, helpers.run, helpers.uimeths
+local eq = helpers.eq
 local dedent = helpers.dedent
 
 local Screen = {}
@@ -389,12 +390,21 @@ function Screen:_handle_mode_info_set(cursor_style_enabled, mode_info)
 end
 
 function Screen:_handle_clear()
+  -- the first implemented UI protocol clients (python-gui and builitin TUI)
+  -- allowed the cleared region to be restricted by setting the scroll region.
+  -- this was never used by nvim tough, and not documented and implemented by
+  -- newer clients, to check we remain compatible with both kind of clients,
+  -- ensure the scroll region is in a reset state.
+  local expected_region = {
+    top = 1, bot = self._height, left = 1, right = self._width
+  }
+  eq(expected_region, self._scroll_region)
   self:_clear_block(1, self._height, 1, self._width)
 end
 
 function Screen:_handle_grid_clear(grid)
   assert(grid == 1)
-  self:_handle_clear()
+  self:_clear_block(1, self._height, 1, self._width)
 end
 
 function Screen:_handle_eol_clear()
@@ -446,22 +456,30 @@ function Screen:_handle_scroll(count)
   local bot = self._scroll_region.bot
   local left = self._scroll_region.left
   local right = self._scroll_region.right
+  self:_handle_grid_scroll(1, top-1, bot, left-1, right, count, 0)
+end
+
+function Screen:_handle_grid_scroll(grid, top, bot, left, right, rows, cols)
+  top = top+1
+  left = left+1
+  assert(grid == 1)
+  assert(cols == 0)
   local start, stop, step
 
-  if count > 0 then
+  if rows > 0 then
     start = top
-    stop = bot - count
+    stop = bot - rows
     step = 1
   else
     start = bot
-    stop = top - count
+    stop = top - rows
     step = -1
   end
 
   -- shift scroll region
   for i = start, stop, step do
     local target = self._rows[i]
-    local source = self._rows[i + count]
+    local source = self._rows[i + rows]
     for j = left, right do
       target[j].text = source[j].text
       target[j].attrs = source[j].attrs
@@ -470,19 +488,11 @@ function Screen:_handle_scroll(count)
   end
 
   -- clear invalid rows
-  for i = stop + step, stop + count, step do
+  for i = stop + step, stop + rows, step do
     self:_clear_row_section(i, left, right)
   end
 end
 
-function Screen:_handle_grid_scroll(grid, top, bot, left, right, rows, cols)
-  assert(grid == 1)
-  assert(cols == 0)
-  -- TODO: if we truly believe we should translate the other way
-  self:_handle_set_scroll_region(top,bot-1,left,right-1)
-  self:_handle_scroll(rows)
-end
-
 function Screen:_handle_hl_attr_define(id, rgb_attrs, cterm_attrs, info)
   self._attr_table[id] = {rgb_attrs, cterm_attrs}
   self._hl_info[id] = info
diff --git a/test/functional/ui/screen_basic_spec.lua b/test/functional/ui/screen_basic_spec.lua
index 75a2d4978d..957d8c0915 100644
--- a/test/functional/ui/screen_basic_spec.lua
+++ b/test/functional/ui/screen_basic_spec.lua
@@ -354,6 +354,101 @@ local function screen_tests(newgrid)
         {0:~                                                    }|
                                                              |
       ]])
+
+      feed(':echo "'..string.rep('x\\n', 12)..'"<cr>')
+      screen:expect([[
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+        x                                                    |
+                                                             |
+        {7:Press ENTER or type command to continue}^              |
+      ]])
+
+      feed('<cr>')
+      screen:expect([[
+        {4: [No Name] }{2: [No Name] }{3:                              }{4:X}|
+        ^                                                     |
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+        {0:~                                                    }|
+                                                             |
+      ]])
+
+    end)
+
+    it('redraws properly with :tab split right after scroll', function()
+      feed('30Ofoo<esc>gg')
+
+      command('vsplit')
+      screen:expect([[
+        ^foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        {1:[No Name] [+]              }{3:[No Name] [+]             }|
+                                                             |
+      ]])
+
+      feed('<PageDown>')
+      screen:expect([[
+        ^foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        foo                       {3:│}foo                       |
+        {1:[No Name] [+]              }{3:[No Name] [+]             }|
+                                                             |
+      ]])
+
+      command('tab split')
+      screen:expect([[
+        {4: }{5:2}{4:+ [No Name] }{2: + [No Name] }{3:                         }{4:X}|
+        ^foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+        foo                                                  |
+                                                             |
+      ]])
     end)
   end)