[vim/vim] popup redraw regression introduced in 9.2.0112 (Issue #20092)

[Maxim Kim]

habamax created an issue (vim/vim#20092)

Steps to reproduce

Using the yegappan/lsp plugin, popups leave stray chars when scrollbar is being hidden:

yegappan/lsp#797

Introduced in

~/prj/vim $ git bisect good
4d0c57e15fd74fb9464178a06cb27acaa14081f7 is the first bad commit
commit 4d0c57e15fd74fb9464178a06cb27acaa14081f7 (tag: v9.2.0112)
Author: Yasuhiro Matsumoto <matt...@gmail.com>
Date:   Thu Mar 5 20:33:40 2026 +0000

    patch 9.2.0112: popup: windows flicker when updating text

    Problem:  popup: windows flicker when updating text
    Solution: Only refresh the popup mask if the position or size changed,
              manually set the window redraw type instead of using
              redraw_win_later() to avoid triggering a full screen redraw.
              Handle opacity 100 correctly. Keep "firstline" sticky when
              setting options (Yasuhiro Matsumoto).

    related: #19510
    closes:  #19559

    Signed-off-by: Yasuhiro Matsumoto <matt...@gmail.com>
    Signed-off-by: Christian Brabandt <c...@256bit.org>

 src/popupwin.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 src/screen.c   |  2 +-
 src/version.c  |  2 ++
 3 files changed, 57 insertions(+), 6 deletions(-)

@mattn fyi

Expected behaviour

Popup shouldn't have stray chars on the right hand side if scroll bar is hidden

Version of Vim

9.2.413

Environment

debian13, bash

Logs and stack traces

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20092@github.com>

[Maxim Kim]

habamax left a comment (vim/vim#20092)

repro steps:

1. vim --clean
2. execute following vimscript

vim9script

popup_create(repeat(["hello world", "world hello", "vim is a text editor"], 10), {
    maxheight: 10,
    minheight: 10,
    minwidth: 30,
    border: [1, 1, 1, 1],
    filter: (id, key) => {
        if key == "\<esc>"
            popup_close(id)
        else
            popup_settext(id, ["should be no stray border"])
        endif
        return true
    }
})

3. press any key

image.png (view on web)

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20092/4348297677@github.com>

[mattn]

mattn left a comment (vim/vim#20092)

Thanks for the bisect. I figured out two things, separately:

1. The bug itself — the change-detection in popup_adjust_position() doesn't check w_has_scrollbar, so when popup_settext() shrinks the buffer enough that the scrollbar disappears, the cell that held the old border / scrollbar never gets repainted.
2. Why I missed it on the original PR — this normally would have been caught by the existing "popup moved" branch (org_wincol != w_wincol), because for a centered popup losing the scrollbar shifts wincol by 1. But the centering math is integer-divided, so whether wincol actually shifts depends on the parity of Columns. My terminal happened to be even-width, where it does shift, so the bug was hidden.

(1) The detection that's missing w_has_scrollbar

popup_adjust_position() decides whether to refresh the popup mask by comparing a fixed set of fields:

if (org_winrow != wp->w_winrow
        || org_wincol != wp->w_wincol
        || org_leftcol != wp->w_leftcol
        || org_leftoff != wp->w_popup_leftoff
        || org_width != wp->w_width
        || org_height != wp->w_height)
{
    redraw_win_later(wp, UPD_NOT_VALID);
    ...
    popup_mask_refresh = TRUE;
}

wp->w_width is the popup's inner content width — it doesn't include the scrollbar column. So when the scrollbar goes away, wp->w_has_scrollbar flips true → false while none of the tracked fields change, and the mask refresh is skipped.

Pre-9.2.0112 this happened to work because f_popup_settext() called redraw_win_later(UPD_NOT_VALID), which propagated must_redraw globally and forced may_update_popup_mask() anyway. The flicker fix dropped that global propagation in favor of a popup-local redraw, exposing the gap.

(2) Why my testing missed it

For a centered popup, w_wincol is computed as:

wp->w_wincol = firstwin->w_wincol + (Columns - wp->w_width - extra_width) / 2;

Losing the scrollbar drops extra_width by 1, so wincol becomes either (Columns - 33) / 2 or (Columns - 32) / 2. With integer truncation, whether the value actually shifts depends on the parity of Columns:

Columns	wincol BEFORE	wincol AFTER	shifted?	stray?
80 (even)	23	24	yes	no (saved by org_wincol != w_wincol)
81 (odd)	24	24	no	yes
82 (even)	24	25	yes	no
83 (odd)	25	25	no	yes

My local terminal was at an even width, so the centered repro silently looked correct on my side. A pos: 'topleft' popup with explicit line/col reproduces unconditionally because w_wincol is anchored — that's how I caught it.

Fix

Add w_has_scrollbar to the change detection. PR coming.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/20092/4348632581@github.com>

[Christian Brabandt]

Closed #20092 as completed via ba85f88.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issue/20092/issue_event/25070825887@github.com>