fix(extmarks): fix signs

Don't add sign extmarks to state->active. Instead when drawing signs,
perform a full line scan for sign marks. This allows decor_redraw_line
to be moved back inside the `!number_only` block in screen.c, which
prevents decor scans when redrawing the number column when
'relativenumber' is set.

Fixes: #17638

4 files changed, 122 insertions, 27 deletions

diff --git a/src/nvim/decoration.c b/src/nvim/decoration.c
index 619e8fdadc..b533f4c46f 100644
--- a/src/nvim/decoration.c
+++ b/src/nvim/decoration.c
@@ -150,7 +150,6 @@ bool decor_redraw_reset(buf_T *buf, DecorState *state)
 {
   state->row = -1;
   state->buf = buf;
-  state->has_sign_decor = false;
   for (size_t i = 0; i < kv_size(state->active); i++) {
     DecorRange item = kv_A(state->active, i);
     if (item.virt_text_owned) {
@@ -202,11 +201,6 @@ bool decor_redraw_start(buf_T *buf, int top_row, DecorState *state)
       goto next_mark;
     }
 
-    // Don't add signs for end marks as the start mark has already been added.
-    if (mt_end(mark) && decor_has_sign(&decor)) {
-      goto next_mark;
-    }
-
     mtpos_t altpos = marktree_get_altpos(buf->b_marktree, mark, NULL);
 
     // Exclude start marks if the end mark position is above the top row
@@ -270,10 +264,6 @@ static void decor_add(DecorState *state, int start_row, int start_col, int end_r
     kv_A(state->active, index) = kv_A(state->active, index-1);
   }
   kv_A(state->active, index) = range;
-
-  if (decor_has_sign(decor)) {
-    state->has_sign_decor = true;
-  }
 }
 
 int decor_redraw_col(buf_T *buf, int col, int win_col, bool hidden, DecorState *state)
@@ -327,8 +317,7 @@ next_mark:
     bool active = false, keep = true;
     if (item.end_row < state->row
         || (item.end_row == state->row && item.end_col <= col)) {
-      if (!(item.start_row >= state->row && kv_size(item.decor.virt_text))
-          && !decor_has_sign(&item.decor)) {
+      if (!(item.start_row >= state->row && kv_size(item.decor.virt_text))) {
         keep = false;
       }
     } else {
@@ -363,19 +352,29 @@ next_mark:
   return attr;
 }
 
-void decor_redraw_signs(buf_T *buf, DecorState *state, int row,
-                        int *num_signs, sign_attrs_T sattrs[])
+void decor_redraw_signs(buf_T *buf, int row, int *num_signs, sign_attrs_T sattrs[])
 {
-  for (size_t i = 0; i < kv_size(state->active); i++) {
-    DecorRange item = kv_A(state->active, i);
-    Decoration *decor = &item.decor;
+  if (!buf->b_signs) {
+    return;
+  }
+
+  MarkTreeIter itr[1] = { 0 };
+  marktree_itr_get(buf->b_marktree, row, 0, itr);
+
+  while (true) {
+    mtkey_t mark = marktree_itr_current(itr);
+    if (mark.pos.row < 0 || mark.pos.row > row) {
+      break;
+    }
 
-    if (!decor_has_sign(decor)) {
-      continue;
+    if (mt_end(mark) || marktree_decor_level(mark) < kDecorLevelVisible) {
+      goto next_mark;
     }
 
-    if (state->row != item.start_row) {
-      continue;
+    Decoration *decor = mark.decor_full;
+
+    if (!decor || !decor_has_sign(decor)) {
+      goto next_mark;
     }
 
     int j;
@@ -403,6 +402,9 @@ void decor_redraw_signs(buf_T *buf, DecorState *state, int row,
       sattrs[j].sat_prio = decor->priority;
       (*num_signs)++;
     }
+
+next_mark:
+    marktree_itr_next(buf->b_marktree, itr);
   }
 }
 
diff --git a/src/nvim/decoration.h b/src/nvim/decoration.h
index 7e6dbdf1a7..2277a0ef1c 100644
--- a/src/nvim/decoration.h
+++ b/src/nvim/decoration.h
@@ -80,7 +80,6 @@ typedef struct {
   int col_until;
   int current;
   int eol_col;
-  bool has_sign_decor;
 } DecorState;
 
 typedef struct {
diff --git a/src/nvim/screen.c b/src/nvim/screen.c
index ee1acd8d03..1498d1c9b3 100644
--- a/src/nvim/screen.c
+++ b/src/nvim/screen.c
@@ -2208,8 +2208,6 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
   buf_T *buf = wp->w_buffer;
   bool end_fill = (lnum == buf->b_ml.ml_line_count+1);
 
-  has_decor = decor_redraw_line(buf, lnum-1, &decor_state);
-
   if (!number_only) {
     // To speed up the loop below, set extra_check when there is linebreak,
     // trailing white space and/or syntax processing to be done.
@@ -2231,6 +2229,8 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
       }
     }
 
+    has_decor = decor_redraw_line(buf, lnum-1, &decor_state);
+
     for (size_t k = 0; k < kv_size(*providers); k++) {
       DecorProvider *p = kv_A(*providers, k);
       if (p && p->redraw_line != LUA_NOREF) {
@@ -2459,9 +2459,7 @@ static int win_line(win_T *wp, linenr_T lnum, int startrow, int endrow, bool noc
 
   memset(sattrs, 0, sizeof(sattrs));
   num_signs = buf_get_signattrs(wp->w_buffer, lnum, sattrs);
-  if (decor_state.has_sign_decor) {
-    decor_redraw_signs(buf, &decor_state, lnum-1, &num_signs, sattrs);
-  }
+  decor_redraw_signs(buf, lnum-1, &num_signs, sattrs);
 
   // If this line has a sign with line highlighting set line_attr.
   // TODO(bfredl, vigoux): this should not take priority over decoration!
diff --git a/test/functional/ui/decorations_spec.lua b/test/functional/ui/decorations_spec.lua
index ae5d34fa7a..7e71ada02d 100644
--- a/test/functional/ui/decorations_spec.lua
+++ b/test/functional/ui/decorations_spec.lua
@@ -1372,6 +1372,7 @@ describe('decorations: signs', function()
     screen:set_default_attr_ids {
       [1] = {foreground = Screen.colors.Blue4, background = Screen.colors.Grey};
       [2] = {foreground = Screen.colors.Blue1, bold = true};
+      [3] = {background = Screen.colors.Yellow1, foreground = Screen.colors.Blue1};
     }
 
     ns = meths.create_namespace 'test'
@@ -1634,4 +1635,99 @@ l5
 
   end)
 
+  it('can add lots of signs', function()
+    screen:try_resize(40, 10)
+    command 'normal 10oa b c d e f g h'
+
+    for i = 1, 10 do
+      meths.buf_set_extmark(0, ns, i,  0, { end_col =  1, hl_group='Todo' })
+      meths.buf_set_extmark(0, ns, i,  2, { end_col =  3, hl_group='Todo' })
+      meths.buf_set_extmark(0, ns, i,  4, { end_col =  5, hl_group='Todo' })
+      meths.buf_set_extmark(0, ns, i,  6, { end_col =  7, hl_group='Todo' })
+      meths.buf_set_extmark(0, ns, i,  8, { end_col =  9, hl_group='Todo' })
+      meths.buf_set_extmark(0, ns, i, 10, { end_col = 11, hl_group='Todo' })
+      meths.buf_set_extmark(0, ns, i, 12, { end_col = 13, hl_group='Todo' })
+      meths.buf_set_extmark(0, ns, i, 14, { end_col = 15, hl_group='Todo' })
+      meths.buf_set_extmark(0, ns, i, -1, { sign_text='W' })
+      meths.buf_set_extmark(0, ns, i, -1, { sign_text='X' })
+      meths.buf_set_extmark(0, ns, i, -1, { sign_text='Y' })
+      meths.buf_set_extmark(0, ns, i, -1, { sign_text='Z' })
+    end
+
+    screen:expect{grid=[[
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:h}                 |
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:h}                 |
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:h}                 |
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:h}                 |
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:h}                 |
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:h}                 |
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:h}                 |
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:h}                 |
+      X Y Z W {3:a} {3:b} {3:c} {3:d} {3:e} {3:f} {3:g} {3:^h}                 |
+                                              |
+    ]]}
+  end)
+
+end)
+
+describe('decorations: virt_text', function()
+  local screen
+
+  before_each(function()
+    clear()
+    screen = Screen.new(50, 10)
+    screen:attach()
+    screen:set_default_attr_ids {
+      [1] = {foreground = Screen.colors.Brown};
+      [2] = {foreground = Screen.colors.Fuchsia};
+      [3] = {bold = true, foreground = Screen.colors.Blue1};
+    }
+  end)
+
+  it('avoids regression in #17638', function()
+    exec_lua[[
+      vim.wo.number = true
+      vim.wo.relativenumber = true
+    ]]
+
+    command 'normal 4ohello'
+    command 'normal aVIRTUAL'
+
+    local ns = meths.create_namespace('test')
+
+    meths.buf_set_extmark(0, ns, 2, 0, {
+      virt_text = {{"hello", "String"}},
+      virt_text_win_col = 20,
+    })
+
+    screen:expect{grid=[[
+      {1:  4 }                                              |
+      {1:  3 }hello                                         |
+      {1:  2 }hello               {2:hello}                     |
+      {1:  1 }hello                                         |
+      {1:5   }helloVIRTUA^L                                  |
+      {3:~                                                 }|
+      {3:~                                                 }|
+      {3:~                                                 }|
+      {3:~                                                 }|
+                                                        |
+    ]]}
+
+    -- Trigger a screen update
+    feed('k')
+
+    screen:expect{grid=[[
+      {1:  3 }                                              |
+      {1:  2 }hello                                         |
+      {1:  1 }hello               {2:hello}                     |
+      {1:4   }hell^o                                         |
+      {1:  1 }helloVIRTUAL                                  |
+      {3:~                                                 }|
+      {3:~                                                 }|
+      {3:~                                                 }|
+      {3:~                                                 }|
+                                                        |
+    ]]}
+  end)
+
 end)