screen: add some documentation of internals of msg_grid implementation

6 files changed, 54 insertions, 16 deletions

diff --git a/src/nvim/grid_defs.h b/src/nvim/grid_defs.h
index d9be53468f..9e588d0387 100644
--- a/src/nvim/grid_defs.h
+++ b/src/nvim/grid_defs.h
@@ -58,18 +58,30 @@ typedef struct {
   // external UI.
   bool throttled;
 
-  // offsets for the grid relative to the global screen
+  // offsets for the grid relative to the global screen. Used by screen.c
+  // for windows that don't have w_grid->chars etc allocated
   int row_offset;
   int col_offset;
 
   // whether the compositor should blend the grid with the background grid
   bool blending;
+
+  // whether the grid can be focused with mouse clicks.
   bool focusable;
 
-  // state owned by the compositor.
+  // Below is state owned by the compositor. Should generally not be set/read
+  // outside this module, except for specific compatibilty hacks
+
+  // position of the grid on the composed screen.
   int comp_row;
   int comp_col;
+
+  // z-index of the grid. Grids with higher index is draw on top.
+  // default_grid.comp_index is always zero.
   size_t comp_index;
+
+  // compositor should momentarily ignore the grid. Used internally when
+  // moving around grids etc.
   bool comp_disabled;
 } ScreenGrid;
 
diff --git a/src/nvim/memline.c b/src/nvim/memline.c
index bb125df968..b69812a389 100644
--- a/src/nvim/memline.c
+++ b/src/nvim/memline.c
@@ -3522,7 +3522,6 @@ static char *findswapname(buf_T *buf, char **dirp, char *old_fname,
             }
             xfree(name);
 
-
             // pretend screen didn't scroll, need redraw anyway
             msg_reset_scroll();
           }
diff --git a/src/nvim/message.c b/src/nvim/message.c
index 9e5c35d58b..2354cce2b7 100644
--- a/src/nvim/message.c
+++ b/src/nvim/message.c
@@ -170,7 +170,7 @@ void msg_grid_validate(void)
     ui_call_grid_resize(msg_grid.handle, msg_grid.Columns, msg_grid.Rows);
 
     msg_grid.throttled = false;  // don't throttle in 'cmdheight' area
-    msg_scroll_at_flush = msg_scrolled;
+    msg_scrolled_at_flush = msg_scrolled;
     msg_grid.focusable = false;
     if (!msg_scrolled) {
       msg_grid_set_pos(Rows - p_ch, false);
@@ -2197,6 +2197,21 @@ void msg_scroll_up(bool may_throttle)
             HL_ATTR(HLF_MSG));
 }
 
+/// Send throttled message output to UI clients
+///
+/// The way message.c uses the grid_xx family of functions is quite inefficient
+/// relative to the "gridline" UI protocol used by TUI and modern clients.
+/// For instance scrolling is done one line at a time. By throttling drawing
+/// on the message grid, we can coalesce scrolling to a single grid_scroll
+/// per screen update.
+///
+/// NB: The bookkeeping is quite messy, and rests on a bunch of poorly
+/// documented assumtions. For instance that the message area always grows while
+/// being throttled, messages are only being output on the last line etc.
+///
+/// Probably message scrollback storage should reimplented as a file_buffer, and
+/// message scrolling in TUI be reimplemented as a modal floating window. Then
+/// we get throttling "for free" using standard redraw_win_later code paths.
 void msg_scroll_flush(void)
 {
   if (!msg_grid.throttled) {
@@ -2205,7 +2220,7 @@ void msg_scroll_flush(void)
   msg_grid.throttled = false;
   int pos_delta = msg_grid_pos_at_flush - msg_grid_pos;
   assert(pos_delta >= 0);
-  int delta = MIN(msg_scrolled - msg_scroll_at_flush, msg_grid.Rows);
+  int delta = MIN(msg_scrolled - msg_scrolled_at_flush, msg_grid.Rows);
 
   if (pos_delta > 0) {
     ui_ext_msg_set_pos(msg_grid_pos, true);
@@ -2228,7 +2243,7 @@ void msg_scroll_flush(void)
             HL_ATTR(HLF_MSG), false);
     msg_grid.dirty_col[row] = 0;
   }
-  msg_scroll_at_flush = msg_scrolled;
+  msg_scrolled_at_flush = msg_scrolled;
   msg_grid_scroll_discount = 0;
 }
 
@@ -2257,7 +2272,7 @@ void msg_reset_scroll(void)
     redraw_all_later(NOT_VALID);
   }
   msg_scrolled = 0;
-  msg_scroll_at_flush = 0;
+  msg_scrolled_at_flush = 0;
 }
 
 /*
@@ -2701,7 +2716,7 @@ static int do_more_prompt(int typed_char)
           if (msg_dothrottle() && !msg_grid.throttled) {
             // Tricky: we redraw at one line higher than usual. Therefore
             // the non-flushed area is one line larger.
-            msg_scroll_at_flush--;
+            msg_scrolled_at_flush--;
             msg_grid_scroll_discount++;
           }
           // scroll up, display line at bottom
@@ -2908,10 +2923,10 @@ int msg_end(void)
     return FALSE;
   }
 
-  // @TODO(bfredl): calling flush here inhibits substantial performance
-  // improvements. Caller should call ui_flush before waiting on user input or
-  // CPU busywork.
-  // ui_flush();  // calls msg_ext_ui_flush
+  // NOTE: ui_flush() used to be called here. This had to be removed, as it
+  // inhibited substantial performance improvements. It is assumed that relevant
+  // callers invoke ui_flush() before going into CPU busywork, or restricted
+  // event processing after displaying a message to the user.
   msg_ext_ui_flush();
   return true;
 }
diff --git a/src/nvim/message.h b/src/nvim/message.h
index 1703384bb5..fdb9bc96ca 100644
--- a/src/nvim/message.h
+++ b/src/nvim/message.h
@@ -91,12 +91,22 @@ extern MessageHistoryEntry *last_msg_hist;
 
 EXTERN bool msg_ext_need_clear INIT(= false);
 
+// allocated grid for messages. Used when display+=msgsep is set, or
+// ext_multigrid is active. See also the description at msg_scroll_flush()
 EXTERN ScreenGrid msg_grid INIT(= SCREEN_GRID_INIT);
+EXTERN int msg_grid_pos INIT(= 0);
+
+// "adjusted" message grid. This grid accepts positions relative to
+// default_grid. Internally it will be translated to a position on msg_grid
+// relative to the start of the message area, or directly mapped to default_grid
+// for legacy (display-=msgsep) message scroll behavior.
+// // TODO(bfredl): refactor "internal" message logic, msg_row etc
+// to use the correct positions already.
 EXTERN ScreenGrid msg_grid_adj INIT(= SCREEN_GRID_INIT);
 
-EXTERN int msg_scroll_at_flush INIT(= 0);
+// value of msg_scrolled at latest msg_scroll_flush.
+EXTERN int msg_scrolled_at_flush INIT(= 0);
 
-EXTERN int msg_grid_pos INIT(= 0);
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "message.h.generated.h"
diff --git a/src/nvim/screen.c b/src/nvim/screen.c
index e182c62c71..8f415a8ed5 100644
--- a/src/nvim/screen.c
+++ b/src/nvim/screen.c
@@ -320,7 +320,7 @@ int update_screen(int type)
   // separate bookkeeping for now.
   if (msg_did_scroll) {
     msg_did_scroll = false;
-    msg_scroll_at_flush = 0;
+    msg_scrolled_at_flush = 0;
   }
 
   if (type >= CLEAR || !default_grid.valid) {
@@ -379,7 +379,7 @@ int update_screen(int type)
       redraw_tabline = TRUE;
     }
     msg_scrolled = 0;
-    msg_scroll_at_flush = 0;
+    msg_scrolled_at_flush = 0;
     need_wait_return = false;
   }
 
diff --git a/src/nvim/ui_compositor.c b/src/nvim/ui_compositor.c
index 163eadbc95..f91442642d 100644
--- a/src/nvim/ui_compositor.c
+++ b/src/nvim/ui_compositor.c
@@ -353,6 +353,8 @@ static void compose_line(Integer row, Integer startcol, Integer endcol,
     size_t n = (size_t)(until-col);
 
     if (row == msg_sep_row && grid->comp_index <= msg_grid.comp_index) {
+      // TODO(bfredl): when we implement borders around floating windows, then
+      // msgsep can just be a border "around" the message grid.
       grid = &msg_grid;
       sattr_T msg_sep_attr = (sattr_T)HL_ATTR(HLF_MSGSEP);
       for (int i = col; i < until; i++) {