From 929e1b7f1c35679424989f5ebfc78f095bb434d9 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Mon, 12 Feb 2024 13:02:27 +0800 Subject: [PATCH] perf(extmarks): avoid unnecessary invalidations for virt_text (#27435) Invalidation of most w_valid flags isn't needed when adding or removing virtual text below cursor. --- src/nvim/change.c | 124 +++++++++++++----------- src/nvim/decoration.c | 14 ++- src/nvim/extmark.c | 11 ++- src/nvim/help.c | 2 +- src/nvim/move.c | 13 --- test/functional/ui/decorations_spec.lua | 48 +++++++-- 6 files changed, 124 insertions(+), 88 deletions(-) diff --git a/src/nvim/change.c b/src/nvim/change.c index e01921a826..2db3d007d7 100644 --- a/src/nvim/change.c +++ b/src/nvim/change.c @@ -163,6 +163,71 @@ void changed_internal(buf_T *buf) need_maketitle = true; // set window title later } +/// Invalidate a window's w_valid flags and w_lines[] entries after changing lines. +static void changed_lines_invalidate_win(win_T *wp, linenr_T lnum, colnr_T col, linenr_T lnume, + linenr_T xtra) +{ + // If the changed line is in a range of previously folded lines, + // compare with the first line in that range. + if (wp->w_cursor.lnum <= lnum) { + int i = find_wl_entry(wp, lnum); + if (i >= 0 && wp->w_cursor.lnum > wp->w_lines[i].wl_lnum) { + changed_line_abv_curs_win(wp); + } + } + + if (wp->w_cursor.lnum > lnum) { + changed_line_abv_curs_win(wp); + } else if (wp->w_cursor.lnum == lnum && wp->w_cursor.col >= col) { + changed_cline_bef_curs(wp); + } + if (wp->w_botline >= lnum) { + // Assume that botline doesn't change (inserted lines make + // other lines scroll down below botline). + approximate_botline_win(wp); + } + + // Check if any w_lines[] entries have become invalid. + // For entries below the change: Correct the lnums for inserted/deleted lines. + // Makes it possible to stop displaying after the change. + for (int i = 0; i < wp->w_lines_valid; i++) { + if (wp->w_lines[i].wl_valid) { + if (wp->w_lines[i].wl_lnum >= lnum) { + // Do not change wl_lnum at index zero, it is used to compare with w_topline. + // Invalidate it instead. + // If lines haven been inserted/deleted and the buffer has virt_lines, + // invalidate the line after the changed lines as some virt_lines may + // now be drawn above a different line. + if (i == 0 || wp->w_lines[i].wl_lnum < lnume + || (xtra != 0 && wp->w_lines[i].wl_lnum == lnume + && buf_meta_total(wp->w_buffer, kMTMetaLines) > 0)) { + // line included in change + wp->w_lines[i].wl_valid = false; + } else if (xtra != 0) { + // line below change + wp->w_lines[i].wl_lnum += xtra; + wp->w_lines[i].wl_lastlnum += xtra; + } + } else if (wp->w_lines[i].wl_lastlnum >= lnum) { + // change somewhere inside this range of folded lines, + // may need to be redrawn + wp->w_lines[i].wl_valid = false; + } + } + } +} + +/// Line changed_lines_invalidate_win(), but for all windows displaying a buffer. +void changed_lines_invalidate_buf(buf_T *buf, linenr_T lnum, colnr_T col, linenr_T lnume, + linenr_T xtra) +{ + FOR_ALL_WINDOWS_IN_TAB(wp, curtab) { + if (wp->w_buffer == buf) { + changed_lines_invalidate_win(wp, lnum, col, lnume, xtra); + } + } +} + /// Common code for when a change was made. /// See changed_lines() for the arguments. /// Careful: may trigger autocommands that reload the buffer. @@ -296,55 +361,7 @@ static void changed_common(buf_T *buf, linenr_T lnum, colnr_T col, linenr_T lnum wp->w_cline_folded = folded; } - // If the changed line is in a range of previously folded lines, - // compare with the first line in that range. - if (wp->w_cursor.lnum <= lnum) { - int i = find_wl_entry(wp, lnum); - if (i >= 0 && wp->w_cursor.lnum > wp->w_lines[i].wl_lnum) { - changed_line_abv_curs_win(wp); - } - } - - if (wp->w_cursor.lnum > lnum) { - changed_line_abv_curs_win(wp); - } else if (wp->w_cursor.lnum == lnum && wp->w_cursor.col >= col) { - changed_cline_bef_curs(wp); - } - if (wp->w_botline >= lnum) { - // Assume that botline doesn't change (inserted lines make - // other lines scroll down below botline). - approximate_botline_win(wp); - } - - // Check if any w_lines[] entries have become invalid. - // For entries below the change: Correct the lnums for - // inserted/deleted lines. Makes it possible to stop displaying - // after the change. - for (int i = 0; i < wp->w_lines_valid; i++) { - if (wp->w_lines[i].wl_valid) { - if (wp->w_lines[i].wl_lnum >= lnum) { - // Do not change wl_lnum at index zero, it is used to - // compare with w_topline. Invalidate it instead. - // If the buffer has virt_lines, invalidate the line - // after the changed lines as the virt_lines for a - // changed line may become invalid. - if (i == 0 || wp->w_lines[i].wl_lnum < lnume - || (wp->w_lines[i].wl_lnum == lnume - && buf_meta_total(wp->w_buffer, kMTMetaLines) > 0)) { - // line included in change - wp->w_lines[i].wl_valid = false; - } else if (xtra != 0) { - // line below change - wp->w_lines[i].wl_lnum += xtra; - wp->w_lines[i].wl_lastlnum += xtra; - } - } else if (wp->w_lines[i].wl_lastlnum >= lnum) { - // change somewhere inside this range of folded lines, - // may need to be redrawn - wp->w_lines[i].wl_valid = false; - } - } - } + changed_lines_invalidate_win(wp, lnum, col, lnume, xtra); // Take care of side effects for setting w_topline when folds have // changed. Esp. when the buffer was changed in another window. @@ -488,13 +505,13 @@ void deleted_lines_mark(linenr_T lnum, int count) } /// Marks the area to be redrawn after a change. -/// Consider also calling changed_line_display_buf(). +/// Consider also calling changed_lines_invalidate_buf(). /// /// @param buf the buffer where lines were changed /// @param lnum first line with change /// @param lnume line below last changed line /// @param xtra number of extra lines (negative when deleting) -void buf_redraw_changed_lines_later(buf_T *buf, linenr_T lnum, linenr_T lnume, linenr_T xtra) +void changed_lines_redraw_buf(buf_T *buf, linenr_T lnum, linenr_T lnume, linenr_T xtra) { if (buf->b_mod_set) { // find the maximum area that must be redisplayed @@ -542,7 +559,7 @@ void buf_redraw_changed_lines_later(buf_T *buf, linenr_T lnum, linenr_T lnume, l void changed_lines(buf_T *buf, linenr_T lnum, colnr_T col, linenr_T lnume, linenr_T xtra, bool do_buf_event) { - buf_redraw_changed_lines_later(buf, lnum, lnume, xtra); + changed_lines_redraw_buf(buf, lnum, lnume, xtra); if (xtra == 0 && curwin->w_p_diff && curwin->w_buffer == buf && !diff_internal()) { // When the number of lines doesn't change then mark_adjust() isn't @@ -555,8 +572,7 @@ void changed_lines(buf_T *buf, linenr_T lnum, colnr_T col, linenr_T lnume, linen redraw_later(wp, UPD_VALID); wlnum = diff_lnum_win(lnum, wp); if (wlnum > 0) { - buf_redraw_changed_lines_later(wp->w_buffer, wlnum, - lnume - lnum + wlnum, 0); + changed_lines_redraw_buf(wp->w_buffer, wlnum, lnume - lnum + wlnum, 0); } } } diff --git a/src/nvim/decoration.c b/src/nvim/decoration.c index 95502e176c..e4828bcc31 100644 --- a/src/nvim/decoration.c +++ b/src/nvim/decoration.c @@ -10,6 +10,7 @@ #include "nvim/ascii_defs.h" #include "nvim/buffer.h" #include "nvim/buffer_defs.h" +#include "nvim/change.h" #include "nvim/decoration.h" #include "nvim/drawscreen.h" #include "nvim/extmark.h" @@ -91,15 +92,18 @@ void bufhl_add_hl_pos_offset(buf_T *buf, int src_id, int hl_id, lpos_T pos_start } } -void decor_redraw(buf_T *buf, int row1, int row2, DecorInline decor) +void decor_redraw(buf_T *buf, int row1, int row2, int col1, DecorInline decor) { if (decor.ext) { DecorVirtText *vt = decor.data.ext.vt; while (vt) { bool below = (vt->flags & kVTIsLines) && !(vt->flags & kVTLinesAbove); - redraw_buf_line_later(buf, row1 + 1 + below, true); + linenr_T vt_lnum = row1 + 1 + below; + redraw_buf_line_later(buf, vt_lnum, true); if (vt->flags & kVTIsLines || vt->pos == kVPosInline) { - changed_line_display_buf(buf); + // changed_lines_redraw_buf(buf, vt_lnum, vt_lnum + 1, 0); + colnr_T vt_col = vt->flags & kVTIsLines ? 0 : col1; + changed_lines_invalidate_buf(buf, vt_lnum, vt_col, vt_lnum + 1, 0); } vt = vt->next; } @@ -190,9 +194,9 @@ void buf_put_decor_sh(buf_T *buf, DecorSignHighlight *sh, int row1, int row2) } } -void buf_decor_remove(buf_T *buf, int row1, int row2, DecorInline decor, bool free) +void buf_decor_remove(buf_T *buf, int row1, int row2, int col1, DecorInline decor, bool free) { - decor_redraw(buf, row1, row2, decor); + decor_redraw(buf, row1, row2, col1, decor); if (decor.ext) { uint32_t idx = decor.data.ext.sh_idx; while (idx != DECOR_ID_INVALID) { diff --git a/src/nvim/extmark.c b/src/nvim/extmark.c index 93a2b9c16a..0f9e7749f1 100644 --- a/src/nvim/extmark.c +++ b/src/nvim/extmark.c @@ -75,7 +75,7 @@ void extmark_set(buf_T *buf, uint32_t ns_id, uint32_t *idp, int row, colnr_T col // not paired: we can revise in place if (!invalid && mt_decor_any(old_mark)) { mt_itr_rawkey(itr).flags &= (uint16_t) ~MT_FLAG_DECOR_SIGNTEXT; - buf_decor_remove(buf, row, row, mt_decor(old_mark), true); + buf_decor_remove(buf, row, row, col, mt_decor(old_mark), true); } mt_itr_rawkey(itr).flags &= (uint16_t) ~MT_FLAG_EXTERNAL_MASK; mt_itr_rawkey(itr).flags |= flags; @@ -84,7 +84,8 @@ void extmark_set(buf_T *buf, uint32_t ns_id, uint32_t *idp, int row, colnr_T col } marktree_del_itr(buf->b_marktree, itr, false); if (!invalid) { - buf_decor_remove(buf, old_mark.pos.row, old_mark.pos.row, mt_decor(old_mark), true); + buf_decor_remove(buf, old_mark.pos.row, old_mark.pos.row, old_mark.pos.col, + mt_decor(old_mark), true); } } } else { @@ -99,7 +100,7 @@ void extmark_set(buf_T *buf, uint32_t ns_id, uint32_t *idp, int row, colnr_T col revised: if (decor_flags || decor.ext) { buf_put_decor(buf, decor, row, end_row > -1 ? end_row : row); - decor_redraw(buf, row, end_row > -1 ? end_row : row, decor); + decor_redraw(buf, row, end_row > -1 ? end_row : row, col, decor); } if (idp) { @@ -170,7 +171,7 @@ void extmark_del(buf_T *buf, MarkTreeIter *itr, MTKey key, bool restore) if (mt_invalid(key)) { decor_free(mt_decor(key)); } else { - buf_decor_remove(buf, key.pos.row, key2.pos.row, mt_decor(key), true); + buf_decor_remove(buf, key.pos.row, key2.pos.row, key.pos.col, mt_decor(key), true); } } @@ -369,7 +370,7 @@ void extmark_splice_delete(buf_T *buf, int l_row, colnr_T l_col, int u_row, coln } else { invalidated = true; mt_itr_rawkey(itr).flags |= MT_FLAG_INVALID; - buf_decor_remove(buf, mark.pos.row, endpos.row, mt_decor(mark), false); + buf_decor_remove(buf, mark.pos.row, endpos.row, mark.pos.col, mt_decor(mark), false); } } } diff --git a/src/nvim/help.c b/src/nvim/help.c index 879e2801a7..779772cedf 100644 --- a/src/nvim/help.c +++ b/src/nvim/help.c @@ -803,7 +803,7 @@ void get_local_additions(void) linenr_T appended = lnum - lnum_start; if (appended) { mark_adjust(lnum_start + 1, (linenr_T)MAXLNUM, appended, 0, kExtmarkUndo); - buf_redraw_changed_lines_later(curbuf, lnum_start + 1, lnum_start + 1, appended); + changed_lines_redraw_buf(curbuf, lnum_start + 1, lnum_start + 1, appended); } break; } diff --git a/src/nvim/move.c b/src/nvim/move.c index 1cf6e52ec8..551aa1bd4d 100644 --- a/src/nvim/move.c +++ b/src/nvim/move.c @@ -595,19 +595,6 @@ void changed_line_abv_curs_win(win_T *wp) |VALID_CHEIGHT|VALID_TOPLINE); } -/// Display of line has changed for "buf", invalidate cursor position and -/// w_botline. -void changed_line_display_buf(buf_T *buf) -{ - FOR_ALL_WINDOWS_IN_TAB(wp, curtab) { - if (wp->w_buffer == buf) { - wp->w_valid &= ~(VALID_WROW|VALID_WCOL|VALID_VIRTCOL - |VALID_CROW|VALID_CHEIGHT - |VALID_TOPLINE|VALID_BOTLINE|VALID_BOTLINE_AP); - } - } -} - // Make sure the value of curwin->w_botline is valid. void validate_botline(win_T *wp) { diff --git a/test/functional/ui/decorations_spec.lua b/test/functional/ui/decorations_spec.lua index bb24b775c1..9f38c05757 100644 --- a/test/functional/ui/decorations_spec.lua +++ b/test/functional/ui/decorations_spec.lua @@ -3882,16 +3882,31 @@ if (h->n_buckets < new_n_buckets) { // expand it('works with one line', function() insert(example_text2) - feed 'gg' + feed '2gg' + screen:expect{grid=[[ + if (h->n_buckets < new_n_buckets) { // expand | + ^khkey_t *new_keys = (khkey_t *)krealloc((void *)| + h->keys, new_n_buckets * sizeof(khkey_t)); | + h->keys = new_keys; | + if (kh_is_map && val_size) { | + char *new_vals = krealloc( h->vals_buf, new_n_| + buckets * val_size); | + h->vals_buf = new_vals; | + } | + } | + {1:~ }| + | + ]]} + api.nvim_buf_set_extmark(0, ns, 1, 33, { virt_lines={ {{">> ", "NonText"}, {"krealloc", "Identifier"}, {": change the size of an allocation"}}}; virt_lines_above=true; }) screen:expect{grid=[[ - ^if (h->n_buckets < new_n_buckets) { // expand | + if (h->n_buckets < new_n_buckets) { // expand | {1:>> }{2:krealloc}: change the size of an allocation | - khkey_t *new_keys = (khkey_t *)krealloc((void *)| + ^khkey_t *new_keys = (khkey_t *)krealloc((void *)| h->keys, new_n_buckets * sizeof(khkey_t)); | h->keys = new_keys; | if (kh_is_map && val_size) { | @@ -3955,7 +3970,6 @@ if (h->n_buckets < new_n_buckets) { // expand api.nvim_buf_set_extmark(0, ns, 5, 0, { virt_lines = { {{"^^ REVIEW:", "Todo"}, {" new_vals variable seems unnecessary?", "Comment"}} }; }) - -- TODO: what about the cursor?? screen:expect{grid=[[ if (h->n_buckets < new_n_buckets) { // expand | khkey_t *new_keys = (khkey_t *) | @@ -3972,7 +3986,6 @@ if (h->n_buckets < new_n_buckets) { // expand ]]} api.nvim_buf_clear_namespace(0, ns, 0, -1) - -- Cursor should be drawn on the correct line. #22704 screen:expect{grid=[[ if (h->n_buckets < new_n_buckets) { // expand | khkey_t *new_keys = (khkey_t *) | @@ -4604,22 +4617,24 @@ if (h->n_buckets < new_n_buckets) { // expand ]]} feed('gg') - feed('dd') + feed('yyp') screen:expect{grid=[[ - ^line2 | + line1 | foo | bar | baz | + ^line1 | + line2 | line3 | line4 | line5 | - {1:~ }|*4 + {1:~ }|*2 | ]]} - feed('yyp') + feed('dd') screen:expect{grid=[[ - line2 | + line1 | foo | bar | baz | @@ -4630,6 +4645,19 @@ if (h->n_buckets < new_n_buckets) { // expand {1:~ }|*3 | ]]} + + feed('kdd') + screen:expect([[ + ^line2 | + foo | + bar | + baz | + line3 | + line4 | + line5 | + {1:~ }|*4 + | + ]]) end) end)