redraw a specific floating window ignoring other pending screen updates

[theofabilous]

Problem

There is no way to redraw and update a specific window without updating other windows that "need redrawing". Individual windows can be marked for redraw, but updates are only ever flushed to the UI via update_screen() which will go through all windows with pending updates. In most cases this is either necessary (e.g. for splits, resizing, etc) or it's not a problem.

However, there is much work being done in PR #27855 (tracking issue #27811) to externalize messages and the command line for the TUI via floating windows (also noice.nvim). The current high-level redrawing logic for the command line/message grid is orthogonal to that of windows, which has the following advantages:

1. Visual selection is not cleared when redrawing the command line. For :'<,'>[command]... this is great because you can see what the ex-cmd is (presumably) operating on. Any call to :redraw, opening a new float, etc. will clear the visual selection highlight.
2. no redrawing clash with inccommand. command line can be redrawn independently, without being blocked (#9777, #9783, #20463) or clearing the preview.

Neither can be achieved via regular window redrawing, so if the cmdline/msg grid is externalized with a floating window we lose both of these benefits (I think #27950 somewhat addresses point (2) but still wouldn't allow properly re-updating the cmdline afterwards). Afaik this also applies to the native popup wildmenu which has its own seperate redrawing codepath.

In brief, externalized ui component providers in the TUI (specifically ext_cmdline and ext_messages, also likely ext_popumenu for wildmenu) have to use hacks and workarounds to maintain ephemeral screen state (like visual selection in cmdline, or cmdpreview) because there is no publicly exposed mechanism to redraw a window without screwing up the dirty screen state (which should be kept dirty). Some PRs have addressed this problem but in an ad-hoc/hacky manner.

Expected behavior

Either:

1. a function in the api/stdlib that allows redrawing a specific float without touching any other pending screen updates. I think(hope?) its reasonable to expect that the (re)drawing of floating windows can be done in isolation of other windows/the "main" window grid. Somewhat related: feat(api): add nvim__redraw for more granular redrawing #28101 but in its current state/scope it does not fix the issue
2. internal logic for keeping/restoring desirable ephemeral screen states: specifically the visual selection and command preview. I suppose the redrawing logic could check and handle each of theses cases individually, e.g. not removing the visual highlight, re-triggering the cmdpreview callback if needed, etc. There may be other similar "dirty screen states" that we want to keep though, I just can't think of any others.

[luukvbaal]

This sounds nice but there is no controlling when update_screen() will be called (often). Other windows marked for redraw will be redrawn at that point anyways, so updating a single window does not really make sense (unless redrawing() == false). I think a better solution would be to get rid of unwanted ephemeral screen state.

I think #27950 somewhat addresses point (2) but still wouldn't allow properly re-updating the cmdline afterwards

What makes you say this? Have you tried it out? The "before doing 'cmdpreview'" happens on each key press while in cmdline mode. So a cmdline window can be updated, then the ephemeral 'cmdpreview' state is pushed to the screen and stays there until the next key press. I don't think there is a need for "re-updating the cmdline afterwards".

[theofabilous]

Anecdotally I was toying with a command line highlighting plugin a while back and ran into all of these issues. Trying to open a floating window in the command line area caused a bunch of problems similar to those tracked in folke/noice.nvim#6.

I ended up doing a symbol name lookup with nm in the neovim binary for the win_update() function address (it's static, not exported so not directly available via ffi) and calling that function directly, which worked. But everything about that approach is cursed so I just ended up using Nvim_color_cmdline lmao.

I mention this because it's kind of the implementation that I had mind: a function similar to update_screen() but that doesn't call win_update() for every window, just a specific one.

[bfredl]

I mention this because it's kind of the implementation that I had mind: a function similar to update_screen() but that doesn't call win_update() for every window, just a specific one.

We had a lot of such ad-hoc mechanims in the past, which had be painfully kept in sync and maintained along with any improvement to update_screen(). These will not be added back. no exceptions.

We could add a update_screen(bool priority) arg or something to only redraw windows with some new high-priority marker, but as a general direction, I don't like that.

The direction we have already chosen and should continue on is to make any fragile state which not must be overdrawn by accident into real stateful decorations which in fact can be safely redrawn, like all other stuff on the screen. thus just calling update_screen() when stuff actually needs to be sent out to the external UI will be a non-problem.

[luukvbaal]

What I'm suggesting is a method to basically immediately invoke something similar to update_screen()'s functionality, but only for a specific floating window.

It could be that this works for the external cmdline case and may be solved by adding a higher priority redraw type, to be updated in isolation by update_screen() when present. But my point was that usually update_screen() will be called anyways, likely in the same event loop cycle or soon after. So adding a general API to update a specific window feels like creating a disingenuous expectation.

But the way I see it, an unholy amount of work would be needed to get rid of this in the cmdpreview logic.

I think that may be overestimated; could even be done in Lua. And as bfredl mentions, is what we should aim to do.

[justinmk]

The direction we have already chosen and should continue on is to make any fragile state which not must be overdrawn by accident into real stateful decorations which in fact can be safely redrawn, like all other stuff on the screen.

Shall we use this issue to track that?

[theofabilous]

But my point was that usually update_screen() will be called anyways, likely in the same event loop cycle or soon after.

Ah, I see what you mean. Again, I can anecdotally say that my approach was working as intended, but that's because I wasn't using vim.ui_attach(...) (for reasons that are out of scope here). Thus update_screen() was never called automatically. I realize that no-one should do this though. A regular ext cmdline provider will have update_screen and friends called normally, so you're 100% right that what I suggested wouldn't work.

Regardless, I see the points everyone is making and completely agree. Getting rid of ephemeral screen state is a much more "treat the root cause"-type of solution than the approach I suggested.

The direction we have already chosen and should continue on is to make any fragile state which not must be overdrawn by accident into real stateful decorations which in fact can be safely redrawn

Agreed. I suppose this approach would solve many more issues (in addition to those I mentioned), like the fact that cmdpreview is cleared when resizing the editor or when <C-g>/<C-t> is pressed during :substitute preview.

[theofabilous]

I would be willing to work on this. I think the priority is addressing this for cmdpreview specifically, I don't know if people care about the visual selection thing. Anyway I can imagine a pretty simple solution for the visual highlight on the plugin side if there isn't a sufficient need/desire to merge this into master.

The only method that I can think of that maintains the cmdpreview state across redraws would be to re-invoke the associated preview callback when appropriate. Is this the proper approach?

My first thought was to store the window/buffer changes applied by the preview callback and reapply them when needed. But this assumes the preview callback's changes are independent of things like the visible region in the buffer, making it impossible for cmdpreview callbacks to, say, lazily only apply changes in the visible viewport (which, if I'm not mistaken, is done in substitute's preview callback).

[luukvbaal]

The only method that I can think of that maintains the cmdpreview state across redraws would be to re-invoke the associated preview callback when appropriate. Is this the proper approach?

No the point is to get rid of artificially maintaining cmdpreview state. What we want to do is instead of cmdpreview_prepare/restore_state() on each keypress, just prepare and restore once when entering/leaving the cmdpreview state.

[theofabilous]

I think what we want to do is open a (floating/split, depending on 'inccommand') window when cmdpreview starts, and only close it when it ends.

That's also something I considered, and is definitely the method that is most aligned with the "getting rid of fragile state" idea.

However wouldn't it be detrimental to performance? The benefit of having cmdpreview handled differently and it being intrinsically tied to redrawing is that it allows lazily showing changes based on what's visible.

Like I mentioned for :substitute preview, only substitutions on visible lines are shown. With the current implementation, I can :%s/<pat>/<sub> on a 20K-line file and have the changes appear instantly. With what you're suggesting, that's not possible because e.g. an editor resize could potentially expose more lines in the target buffer whose substitutions should logically be previewed, but by not re-triggering the preview callback, they wouldn't. This also applies for <C-g>/<C-t> search navigation which scrolls the window without triggering the callback again.

I'm not against this approach though. Definitely seems like it'd be easier to implement and would remove a lot of edge cases. Maybe explicit support for handling these kinds of redraws to a preview buffer could be added, or like another user-supplied callback that can decide if the preview callback should be invoked again.

[luukvbaal]

I don't see a reason why the cmdpreview buffer in this approach can't still contain only the visible lines. The point is just to keep that window around until cmdpreview ends. But yes, it looks like manipulating the buffer content in the cmdpreview window should happen more frequently, at which point the window dimensions could also be updated. Resizing Nvim currently also removes the cmdpreview window until the next keypress.

[theofabilous]

It could, but a preview callback applying changes lazily like this would need to be re-invoked when the visible lines change. The editor can be resized at any time.

E.g. suppose I have a buffer with 100 lines of the text hello, and currently lines 1-50 are visible. I type :%s/hello/hi, lazily applying the substitution preview to lines 1-50 so that now I see 50 lines of hi. Then I resize my editor such that the entire buffer is visible. If the callback isn't re-invoked, those 50 new lines won't be affected by cmdpreview, still displaying hello. I don't think that behaviour is desired

EDIT: didn't see you updated your comment before I posted this, sorry. So you see my point? For any redraw that changes what's actually visible on the affected buffer/window, the cmdpreview callback would either have to be re-invoked to support lazy previews, or support for such optimizations would have to be dropped. If the former is the chosen path, then in the worst case we'd have to invoke the callback on every redraw, which is essentially what I was proposing.

[luukvbaal]

Well yes but like I said the resize issue is separate from replacing the ephemeral window with a persistent one, and is already present in the status quo. If anything a persistent window should make resolving that easier.

[theofabilous]

Right, I don't disagree. I guess I misunderstood your comment then. The aspect of the approach I'm discussing is agnostic to window persistence. Obv for icm=split it would be wasteful to close and reopen the preview window for every redraw, the point I'm making is that we'd still (worst case) need to invoke the cmdpreview callback on every redraw.

Although I don't see why a new window would have to be opened for nosplit? I would have just moved the buffer restoration logic to whenever cmdpreview ends/before re-invoking, instead of restoring immediately after the preview callback invocation (something like that).

[luukvbaal]

I don't quite follow what you mean with "lazily applying the substitution preview". I think the command should (and currently does) just execute on the entire preview buffer. What is redrawn is probably cheap in comparison and I don't think keeping the cmdpreview state around will be detrimental to performance but only improve it.

the point I'm making is that we'd still (worst case) need to invoke the cmdpreview callback on every redraw.

I don't see why "the cmdpreview callback(cmdpreview_may_show()?)" would need to be invoked on every redraw, the entire point of making the preview state persistent is so that we don't have to do that.

Although I don't see why a new window would have to be opened for nosplit? I would have just moved the buffer restoration logic to whenever cmdpreview ends/before re-invoking, instead of restoring immediately after the preview callback invocation (something like that).

Yes that sounds right, sorry for the confusion. Floating windows and moving the implementation to Lua were mentioned recently when discussing this on Matrix but I don't think it's actually relevant. If you're willing to work on this I would suggest just trying to do cmdpreview_prepare/restore_state() once when entering/leaving the cmdpreview state, instead of on each key press. The resize issue is separate but it seems some variant of cmdpreview_may_show() would have to be called after resizing.

[theofabilous]

"the cmdpreview callback(cmdpreview_may_show()?)"

Sorry, I was using that term pretty loosely. I specifically meant either the user-supplied lua preview callback or the one associated to a builtin command (cmd_preview_func field in CommandDefinition). Yeah both are handled and executed in cmdpreview_may_show() but the distinction is important: when I say that the "cmdpreview callback can lazily apply the changes", I mean the callback/function associated with a specific command (i.e. either lua preview callback or cmd_preview_func), not the actual general cmdpreview handling functions (like cmdpreview_may_show()).

I don't quite follow what you mean with "lazily applying the substitution preview". I think the command should (and currently does) just execute on the entire preview buffer

It doesn't. The :substitute preview callback (AFAIK the only builtin cmdpreview callback) specifically only applies changes to lines that are visible in the viewport at the time of its invocation:

neovim/src/nvim/ex_cmds.c

Lines 3517 to 3527 in 435dee7

	// Check for a match on each line.
	// If preview: limit to max('cmdwinheight', viewport).
	linenr_T line2 = eap->line2;
	for (linenr_T lnum = eap->line1;
	lnum <= line2 && !got_quit && !aborting()
	&& (cmdpreview_ns <= 0 || preview_lines.lines_needed <= (linenr_T)p_cwh
	|| lnum <= curwin->w_botline);
	lnum++) {
	int nmatch = vim_regexec_multi(&regmatch, curwin, curbuf, lnum,
	0, NULL, NULL);

Hopefully that explains why I was saying the cmdpreview callback would potentially have to be re-invoked on every redraw (assuming every redraw is a resize or scroll). I think the above snippet makes it clear that resizing/scrolling a window that is showing a :sub preview can't just be redrawn regularly.

If you're willing to work on this I would suggest just trying to do cmdpreview_prepare/restore_state() once when entering/leaving the cmdpreview state, instead of on each key press. The resize issue is separate but it seems some variant of cmdpreview_may_show() would have to be called after resizing.

Cool, that's what I was thinking.

[luukvbaal]

It doesn't. The :substitute preview callback (AFAIK the only builtin cmdpreview callback) specifically only applies changes to lines that are visible in the viewport at the time of its invocation:

I see, my bad(not actually familiar with the cmdpreview code, I just read some of the code in ex_getln.c). I guess that breaks down with wrapped lines when the command changes the line height.

Hopefully that explains why I was saying the cmdpreview callback would potentially have to be re-invoked on every redraw (assuming every redraw is a resize or scroll).

Not sure why we're assuming every redraw is a resize or scroll(why would a scroll happen during cmdpreview?) but yes, in that case the cmdpreview call back would have to be re-invoked to sync the viewport (perhaps also recursively when the command changes the line height to account for a new botline/topline). But is that a reasonable performance concern? I don't think scrolling or resizing happens frequently during cmdpreview. I would consider that a reasonable/necessary fix for a bug that's also present with the current ephemeral preview.

[theofabilous]

Not sure why we're assuming every redraw is a resize or scroll

I was assuming the "worst" case. But yes that is an unrealistic assumption, I was just using that to make my point about the necessity to re-invoke the preview callback.

why would a scroll happen during cmdpreview?

<C-g>/<C-t> during :substitute. But any cmap could do such a thing

But is that a reasonable performance concern? I don't think scrolling or resizing happens frequently during cmdpreview.

No its not, I agree. My concern with respect to performance wasn't related to redrawing. It's no longer a concern though, I brought it up earlier when you responded (in #28510 (comment)) to my initial implementation proposal but I think I made my point since then. I meant that your response, as I understood it, forced command-preview callbacks to draw on the entire buffer because e.g. the callback would not be re-invoked between redraws even if e.g. the viewport changes.

So the perf concern was more about preview implementors having to drop the assumption they could previously make that they can lazily only show changes on visible lines (see the :substitute preview snippet above). But again not a concern anymore because I see that you agree that the callback should be reinvoked in some cases.