`set guifont` overrides `set lines` height when ran at the same time

[glacambre]

I'm also having the resizing problem. It can change the lines and colmns variables, but suddelnly reset the size to the textbox size.

When I execute :set lines manually while typing in nvim, it can keep its modified size.

Any idea to fix it?

I disabled all the firefox addons except firenvim, but nothing has changed.

function! s:IsFirenvimActive(event) abort
  if !exists('*nvim_get_chan_info')
    return 0
  endif
  let l:ui = nvim_get_chan_info(a:event.chan)
  return has_key(l:ui, 'client') && has_key(l:ui.client, 'name') &&
      \ l:ui.client.name =~? 'Firenvim'
endfunction

function! SetLinesForFirefox()
    set lines=28 columns=110 laststatus=0
endfunction

function! OnUIEnter(event) abort
  if s:IsFirenvimActive(a:event)
    call SetLinesForFirefox()
    nnoremap <silent> <Leader>f :call SetLinesForFirefox()<CR>
    set guifont=Cica:h14
  endif
endfunction

autocmd UIEnter * call OnUIEnter(deepcopy(v:event))

Environment

• Firefox 83
• Ubuntu 18.03

Originally posted by @fx-kirin in https://github.com/glacambre/firenvim/issues/650#issuecomment-733569521

[fx-kirin]

Yes, you are right. When I put timer_start to my script, the resizing works fine.

function! SetLinesForFirefox(timer)
    set lines=28 columns=110 laststatus=0
endfunction

function! OnUIEnter(event) abort
  if s:IsFirenvimActive(a:event)
    set guifont=Cica:h14
    call timer_start(1000, function("SetLinesForFirefox"))
  endif
endfunction

[glacambre]

I think this is impossible to solve without large amounts of hack because of the way resizing currently works in neovim.

The reason this bug happens is that set guifont changes the number of characters that can be displayed in the window. For example, if you double the height of your characters, you halve the number of lines that can be displayed in the window.

Changing the number of lines that can be displayed in the window means that Firenvim needs to warn Neovim that the dimensions of the window changed. This is done by calling nvim_ui_try_resize_grid, passing the desired grid, width and height as argument.

Whenever Neovim decides the window dimensions should change (e.g. when :set lines=... is ran by the user or when the UI calls nvim_ui_try_resize_grid), Neovim sends an event named grid_lines to the UI.

So we have the following sequence of events:

• Neovim sends an option_set event, telling Firenvim to change the font/font size.
• Firenvim deals with it, discovers that the number of lines that can be displayed has changed. It warns Neovim with nvim_ui_try_resize_grid.
• Neovim sends a grid_resize event for the :set lines=... option set by the user.
• Firenvim deals with it and resizes to the :set lines=... dimensions.
• Neovim sends a grid_resize event for the nvim_ui_try_resize_grid call made by Firenvim.
• Firenvim deals with it and the changes made by :set lines=... are dropped.

After thinking about this for a while, I came to the following insight: if I can identify the grid_resize events that are caused by nvim_ui_try_resize_grid, I could record the grid_resize events that are NOT caused by a nvim_ui_try_resize_grid and replay them after all pending grid_resize events caused by nvim_ui_try_resize_grid.

However, there is no way to tell if a grid_resize event was caused by a nvim_ui_try_resize_grid call or something else. But since we know the grid, width and height we passed to nvim_ui_try_resize_grid and this information is all available in grid_resize events, we could just consider any grid_resize event whose attributes matches the arguments of our call to nvim_ui_try_resize_grid to be caused by our call to nvim_ui_try_resize_grid.

So this takes care of a :set guifont=... followed by a :set lines=.... But there's also the reverse, what should happen for :set lines=... followed by :set guifont=...? Something similar: if a grid_resize event that is not caused by a call to nvim_ui_try_resize_grid happens, just replay that event every time a grid_resize event triggered by a nvim_ui_try_resize_grid is received during the second that follows the receiving of the non-nvim_ui_try_resize_grid event.

This is all embodied by the following patch:

commit 1d4bfb517f7fa039e75ffc8386c4b561683bd01a (HEAD -> guifont_setlines_resize_conflicts)
Author: glacambre <[email protected]>
Date:   Sun Jan 10 16:34:04 2021 +0100

    Fix `guifont` overriding `:set lines`

diff --git a/src/render/RedrawCanvas.ts b/src/render/RedrawCanvas.ts
index 3340335..86dc173 100644
--- a/src/render/RedrawCanvas.ts
+++ b/src/render/RedrawCanvas.ts
@@ -366,6 +366,43 @@ function damageMessagesSpace (state: State) {
     msgPos.y = state.canvas.height;
 }
 
+let lastNonOptionSetResize : [number, number, number] = undefined;
+let lastNonOptionSetResizeTime = performance.now();
+let pendingGridResize : [number, number, number] = undefined;
+const optionSetResizes = new Map();
+function resizeId(grid: number, width: number, height: number) {
+    return `${grid}-${width}-${height}`;
+}
+function optionSetResize(width: number, height: number) {
+    const gid = getGridId();
+    const rid = resizeId(gid, width, height);
+
+    let count = optionSetResizes.get(rid);
+    if (count === undefined) {
+        count = 0;
+    }
+    optionSetResizes.set(rid, count + 1);
+
+    functions.ui_try_resize_grid(gid, width, height);
+}
+function registerGridResize(grid: number, width: number, height: number) {
+    const rid = resizeId(grid, width, height);
+    let count = optionSetResizes.get(rid);
+    let isReply;
+    if (count > 0) {
+        count = count - 1;
+        isReply = true;
+    } else {
+        count = 0;
+        isReply = false;
+    }
+    optionSetResizes.set(rid, count);
+    return isReply;
+}
+function hasPendingOptionSetResizes() {
+    return Array.from(optionSetResizes.values()).find(v => v > 0) > 0;
+}
+
 const handlers : { [key:string] : (...args: any[])=>void } = {
     busy_start: () => { globalState.canvas.style.cursor = "wait"; },
     busy_stop: () => { globalState.canvas.style.cursor = "auto"; },
@@ -464,6 +501,31 @@ const handlers : { [key:string] : (...args: any[])=>void } = {
         }
     },
     grid_resize: (id: number, width: number, height: number) => {
+        const isOptionSetReply = registerGridResize(id, width, height);
+        if (isOptionSetReply) {
+            // If this grid_resize is a response to a nvim.ui_try_resize_grid
+            // call triggered by an option_set message, we need to re-send the
+            // previous non-option_set grid_resize in two cases:
+            // - if it happened between the nvim.ui_try_resize_grid call and
+            //   its answer
+            // - if it happened less than 1 second ago
+            if (pendingGridResize !== undefined) {
+                functions.ui_try_resize_grid(...pendingGridResize);
+                pendingGridResize = undefined;
+            } else {
+                if (lastNonOptionSetResize !== undefined
+                    && (performance.now() - lastNonOptionSetResizeTime) < 1000) {
+                    functions.ui_try_resize_grid(...lastNonOptionSetResize);
+                }
+            }
+        } else {
+            lastNonOptionSetResize = [id, width, height];
+            lastNonOptionSetResizeTime = performance.now();
+            if (hasPendingOptionSetResizes()) {
+                pendingGridResize = lastNonOptionSetResize;
+            }
+        }
+        // Actual handling of grid_resize
         const state = globalState;
         const createGrid = state.gridCharacters[id] === undefined;
         if (createGrid) {
@@ -606,29 +668,61 @@ const handlers : { [key:string] : (...args: any[])=>void } = {
         const state = globalState;
         switch (option) {
             case "guifont": {
+                const [oldCharWidth, oldCharHeight] = getGlyphInfo(state);
+
                 const guifont = parseGuifont(((typeof value) === "string" ? value : ""), {
                     "font-family": "monospace",
                     "font-size": "9pt"
                 });
                 setFontString(state, `${guifont["font-size"]} ${guifont["font-family"]}`);
-                const [charWidth, charHeight] = getGlyphInfo(state);
-                functions.ui_try_resize_grid(getGridId(),
-                                             Math.floor(state.canvas.width / charWidth),
-                                             Math.floor(state.canvas.height / charHeight));
+
+                const gid = getGridId();
+                const curGridSize = globalState.gridSizes[gid];
+                const [newCharWidth, newCharHeight] = getGlyphInfo(state);
+
+                if (curGridSize === undefined) {
+                    functions.ui_try_resize_grid(gid,
+                                                 Math.round(state.canvas.width / newCharWidth),
+                                                 Math.round(state.canvas.height / newCharHeight));
+                } else {
+                    const columnCount = Math.round(curGridSize.width * oldCharWidth / newCharWidth);
+                    const lineCount = Math.round(curGridSize.height * oldCharHeight / newCharHeight);
+                    if (curGridSize.width !== columnCount || curGridSize.height !== lineCount) {
+                        optionSetResize(columnCount, lineCount);
+                    }
+                }
             }
             break;
             case "linespace": {
+                const [oldCharWidth, oldCharHeight] = getGlyphInfo(state);
+
                 state.linespace = value;
                 invalidateMetrics();
-                const [charWidth, charHeight] = getGlyphInfo(state);
+
                 const gid = getGridId();
-                const curGridSize = state.gridSizes[gid];
+                const curGridSize = globalState.gridSizes[gid];
+                const [newCharWidth, newCharHeight] = getGlyphInfo(state);
+
                 if (curGridSize !== undefined) {
-                    pushDamage(getGridId(), DamageKind.Cell, curGridSize.height, curGridSize.width, 0, 0);
+                    pushDamage(gid,
+                               DamageKind.Cell,
+                               curGridSize.height,
+                               curGridSize.width,
+                               0,
+                               0);
+                }
+
+                if (curGridSize === undefined) {
+                    functions.ui_try_resize_grid(gid,
+                                                 Math.round(state.canvas.width / newCharWidth),
+                                                 Math.round(state.canvas.height / newCharHeight));
+                } else {
+                    const columnCount = Math.round(curGridSize.width * oldCharWidth / newCharWidth);
+                    const lineCount = Math.round(curGridSize.height * oldCharHeight / newCharHeight);
+                    if (curGridSize.width !== columnCount || curGridSize.height !== lineCount) {
+                        optionSetResize(columnCount, lineCount);
+                    }
                 }
-                functions.ui_try_resize_grid(gid,
-                                             Math.floor(state.canvas.width / charWidth),
-                                             Math.floor(state.canvas.height / charHeight));
             }
             break;
         }

However, due to the complexity and hackiness of the whole thing, I'm not sure I want to merge that. I'll need to think more about this.

[larsks]

@glacambre thanks for pointing me at this issue! The timer workaround is working great.