perf: only redraw concealed line if cursor has moved horizontally

zeertzjq · zeertzjq · commit 595c1a724af9 · 2022-03-28T10:23:32.000+08:00
Building upon #17889, this moves conceal redrawing logic into move.c, so that concealed line is only redrawn if cursor has moved horizontally.
diff --git a/src/nvim/edit.c b/src/nvim/edit.c
@@ -1480,8 +1480,6 @@ bool edit(int cmdchar, bool startln, long count)
 /// @param ready  not busy with something
 static void ins_redraw(bool ready)
 {
-  bool conceal_cursor_moved = false;
-
   if (char_avail()) {
     return;
   }
@@ -1504,7 +1502,6 @@ static void ins_redraw(bool ready)
       update_curswant();
       ins_apply_autocmds(EVENT_CURSORMOVEDI);
     }
-    conceal_cursor_moved = true;
     curwin->w_last_cursormoved = curwin->w_cursor;
   }
 
@@ -1560,11 +1557,6 @@ static void ins_redraw(bool ready)
     curbuf->b_changed_invalid = false;
   }
 
-  if (curwin->w_p_cole > 0 && conceal_cursor_line(curwin)
-      && conceal_cursor_moved) {
-    redrawWinline(curwin, curwin->w_cursor.lnum);
-  }
-
   pum_check_clear();
   if (must_redraw) {
     update_screen(0);
diff --git a/src/nvim/move.c b/src/nvim/move.c
@@ -96,19 +96,20 @@ static void comp_botline(win_T *wp)
 }
 
 /// Redraw when w_cline_row changes and 'relativenumber' or 'cursorline' is set.
+/// Also when concealing is on and 'concealcursor' is not active.
 void redraw_for_cursorline(win_T *wp)
   FUNC_ATTR_NONNULL_ALL
 {
-  if ((wp->w_p_rnu || win_cursorline_standout(wp))
-      && (wp->w_valid & VALID_CROW) == 0
-      && !pum_visible()) {
+  if ((wp->w_valid & VALID_CROW) == 0 && !pum_visible()
+      && (wp->w_p_rnu || win_cursorline_standout(wp))) {
     // win_line() will redraw the number column and cursorline only.
     redraw_later(wp, VALID);
   }
 }
 
 /// Redraw when w_virtcol changes and 'cursorcolumn' is set or 'cursorlineopt'
 /// contains "screenline".
+/// Also when concealing is on and 'concealcursor' is active.
 static void redraw_for_cursorcolumn(win_T *wp)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -121,6 +122,12 @@ static void redraw_for_cursorcolumn(win_T *wp)
       redraw_later(wp, VALID);
     }
   }
+  // If the cursor moves horizontally when 'concealcursor' is active, then the
+  // current line needs to be redrawn in order to calculate the correct
+  // cursor position.
+  if ((wp->w_valid & VALID_VIRTCOL) == 0 && wp->w_p_cole > 0 && conceal_cursor_line(wp)) {
+    redrawWinline(wp, wp->w_cursor.lnum);
+  }
 }
 
 /*
diff --git a/src/nvim/normal.c b/src/nvim/normal.c
@@ -1288,13 +1288,6 @@ static void normal_redraw(NormalState *s)
   update_topline(curwin);
   validate_cursor();
 
-  // If the cursor moves horizontally when 'concealcursor' is active, then the
-  // current line needs to be redrawn in order to calculate the correct
-  // cursor position.
-  if (curwin->w_p_cole > 0 && conceal_cursor_line(curwin)) {
-    redrawWinline(curwin, curwin->w_cursor.lnum);
-  }
-
   if (VIsual_active) {
     update_curbuf(INVERTED);  // update inverted part
   } else if (must_redraw) {
diff --git a/test/functional/ui/syntax_conceal_spec.lua b/test/functional/ui/syntax_conceal_spec.lua
@@ -3,6 +3,7 @@ local Screen = require('test.functional.ui.screen')
 local clear, feed, command = helpers.clear, helpers.feed, helpers.command
 local eq = helpers.eq
 local insert = helpers.insert
+local poke_eventloop = helpers.poke_eventloop
 
 describe('Screen', function()
   local screen
@@ -911,7 +912,57 @@ describe('Screen', function()
       {0:~                                                    }|
                                                            |
     ]]}
-    eq(grid_lines, {{2, 0, {{'c', 0, 3}}}})
+    eq({{2, 0, {{'c', 0, 3}}}}, grid_lines)
+  end)
+
+  it('K_EVENT should not cause extra redraws with concealcursor #13196', function()
+    command('set conceallevel=1')
+    command('set concealcursor=nv')
+    command('set redrawdebug+=nodelta')
+
+    insert([[
+    aaa
+    bbb
+    ccc
+    ]])
+    screen:expect{grid=[[
+      aaa                                                  |
+      bbb                                                  |
+      ccc                                                  |
+      ^                                                     |
+      {0:~                                                    }|
+      {0:~                                                    }|
+      {0:~                                                    }|
+      {0:~                                                    }|
+      {0:~                                                    }|
+                                                           |
+    ]]}
+
+    -- XXX: hack to get notifications, and check only a single line is
+    --      updated.  Could use next_msg() also.
+    local orig_handle_grid_line = screen._handle_grid_line
+    local grid_lines = {}
+    function screen._handle_grid_line(self, grid, row, col, items)
+      table.insert(grid_lines, {row, col, items})
+      orig_handle_grid_line(self, grid, row, col, items)
+    end
+    feed('k')
+    screen:expect{grid=[[
+      aaa                                                  |
+      bbb                                                  |
+      ^ccc                                                  |
+                                                           |
+      {0:~                                                    }|
+      {0:~                                                    }|
+      {0:~                                                    }|
+      {0:~                                                    }|
+      {0:~                                                    }|
+                                                           |
+    ]]}
+    eq({{2, 0, {{'c', 0, 3}}}}, grid_lines)
+    poke_eventloop()  -- causes K_EVENT key
+    screen:expect_unchanged()
+    eq({{2, 0, {{'c', 0, 3}}}}, grid_lines)
   end)
 
   -- Copy of Test_cursor_column_in_concealed_line_after_window_scroll in

Original file line number	Diff line number	Diff line change
`@@ -1480,8 +1480,6 @@ bool edit(int cmdchar, bool startln, long count)`
`1480`	`1480`	`/// @param ready not busy with something`
`1481`	`1481`	`static void ins_redraw(bool ready)`
`1482`	`1482`	`{`
`1483`		`- bool conceal_cursor_moved = false;`
`1484`		`-`
`1485`	`1483`	`if (char_avail()) {`
`1486`	`1484`	`return;`
`1487`	`1485`	`}`
`@@ -1504,7 +1502,6 @@ static void ins_redraw(bool ready)`
`1504`	`1502`	`update_curswant();`
`1505`	`1503`	`ins_apply_autocmds(EVENT_CURSORMOVEDI);`
`1506`	`1504`	`}`
`1507`		`- conceal_cursor_moved = true;`
`1508`	`1505`	`curwin->w_last_cursormoved = curwin->w_cursor;`
`1509`	`1506`	`}`
`1510`	`1507`
`@@ -1560,11 +1557,6 @@ static void ins_redraw(bool ready)`
`1560`	`1557`	`curbuf->b_changed_invalid = false;`
`1561`	`1558`	`}`
`1562`	`1559`
`1563`		`- if (curwin->w_p_cole > 0 && conceal_cursor_line(curwin)`
`1564`		`- && conceal_cursor_moved) {`
`1565`		`- redrawWinline(curwin, curwin->w_cursor.lnum);`
`1566`		`- }`
`1567`		`-`
`1568`	`1560`	`pum_check_clear();`
`1569`	`1561`	`if (must_redraw) {`
`1570`	`1562`	`update_screen(0);`
Original file line number	Diff line number	Diff line change
`@@ -96,19 +96,20 @@ static void comp_botline(win_T *wp)`
`96`	`96`	`}`
`97`	`97`
`98`	`98`	`/// Redraw when w_cline_row changes and 'relativenumber' or 'cursorline' is set.`
	`99`	`+/// Also when concealing is on and 'concealcursor' is not active.`
`99`	`100`	`void redraw_for_cursorline(win_T *wp)`
`100`	`101`	`FUNC_ATTR_NONNULL_ALL`
`101`	`102`	`{`
`102`		`- if ((wp->w_p_rnu \|\| win_cursorline_standout(wp))`
`103`		`- && (wp->w_valid & VALID_CROW) == 0`
`104`		`- && !pum_visible()) {`
	`103`	`+ if ((wp->w_valid & VALID_CROW) == 0 && !pum_visible()`
	`104`	`+ && (wp->w_p_rnu \|\| win_cursorline_standout(wp))) {`
`105`	`105`	`// win_line() will redraw the number column and cursorline only.`
`106`	`106`	`redraw_later(wp, VALID);`
`107`	`107`	`}`
`108`	`108`	`}`
`109`	`109`
`110`	`110`	`/// Redraw when w_virtcol changes and 'cursorcolumn' is set or 'cursorlineopt'`
`111`	`111`	`/// contains "screenline".`
	`112`	`+/// Also when concealing is on and 'concealcursor' is active.`
`112`	`113`	`static void redraw_for_cursorcolumn(win_T *wp)`
`113`	`114`	`FUNC_ATTR_NONNULL_ALL`
`114`	`115`	`{`
`@@ -121,6 +122,12 @@ static void redraw_for_cursorcolumn(win_T *wp)`
`121`	`122`	`redraw_later(wp, VALID);`
`122`	`123`	`}`
`123`	`124`	`}`
	`125`	`+ // If the cursor moves horizontally when 'concealcursor' is active, then the`
	`126`	`+ // current line needs to be redrawn in order to calculate the correct`
	`127`	`+ // cursor position.`
	`128`	`+ if ((wp->w_valid & VALID_VIRTCOL) == 0 && wp->w_p_cole > 0 && conceal_cursor_line(wp)) {`
	`129`	`+ redrawWinline(wp, wp->w_cursor.lnum);`
	`130`	`+ }`
`124`	`131`	`}`
`125`	`132`
`126`	`133`	`/*`