fix: Completely event driven update loop

[fredizzimo]

What kind of change does this PR introduce?

• This changes all update loop events to be sent through the winit event queue. This reduces the latency, since the rendering starts immediately when the event is received, rather than waiting for the neovide_refresh_rate_idle wakeup.
• The way the cursor blink is updated has also changed, there was a regression due to some merge conflicts earlier. Now the cursor blink controls how long the event loop is waiting.
• The test window_separator_modifies_grid_and_sends_draw_command had to be removed since supporting it would have been too complex requiring an event loop and everything.
• The DrawCommandBatcher has been greatly simplified
• The EventAggregator has been eliminated, and replaced by either raw channels or the winit event_loop_proxy for winit events. UICommands are now sent with send_ui
• The overall communication architecture has been improved, so that there's a clear flow between the components, which much less risk of mistakes, since there's no global EventAggregator that allows almost any kind of communication any longer. These changes have been documented in the architecture comment in main.rs

NOTE: The changes are rather big and a bit more testing would be good. So far I have only done basic testing on Wayland and Windows.

• Fixes "blinkon" doesn't blink #2164
• Fixes Neovide transparency wont update until scrolling #2131
• Not completely sure about Performance regression on Wayland with 6c9b9219dbbd43e3eb6be9fe725d68e8e9ee2561 #2162

Did this PR introduce a breaking change?

A breaking change includes anything that breaks backwards compatibility either at compile or run time.

• No

[9mm]

I'm not sure if im doing something wring but I tried this:

vim.opt.guicursor = "i:blinkon500"

No blinking

[fredizzimo]

You need all three values: https://neovim.io/doc/user/options.html#'guicursor'

blinkoff{N} blink times for cursor: blinkwait is the delay before the cursor starts blinking, blinkon is the time that the cursor is shown and blinkoff is the time that the cursor is not shown. Times are in msec. When one of the numbers is zero, there is no blinking

I implemented it according to that specification

[9mm]

Awesome, now it works!

vim.opt.guicursor = "n-v-c-sm:block,r-cr-o:hor20,i-ci-ve:blinkwait250-blinkoff500-blinkon500"

[fredizzimo]

@9mm, since this also changes how the event loop is run, could you also test if it makes any difference when you run with vsync?

[AThePeanut4]

I'm getting what I believe is the same issue as #2162, so I've just tried this branch to see if it fixes it.

The easiest symptom to spot is that animations are "delayed" after being idle, sometimes to the point of being cut off entirely, meaning that the cursor "teleports" to its new location. It seems like Neovide is realising it should be animating the movement way too late - which fits perfectly with "rendering having to wait for the neovide_refresh_rate_idle wakeup".

As suggested in #2162 (comment), I set neovide_refresh_rate_idle to 144 (my monitor's refresh rate), and that seems to fix the issue entirely for me. But I assume that isn't ideal, as then there's no "idling" at all.

On this branch, the issue is almost entirely gone, but not entirely. When using the keyboard to navigate, everything is good. But when using a mouse to click, the animations are still sometimes a bit delayed. Not always, but it is still happening.

I've included three screen recordings below which I hope demonstrate the issue: on the latest main commit, on this PR branch, and on the commit before 6c9b921, which is where this issue first started occurring.

On the latest main (b637426)

current.mp4

On this branch

pr.mp4

On c2c2179 (the commit before 6c9b921)

old.mp4

[AThePeanut4]

I think I've discovered why the mouse seems to trigger it on this branch but the keyboard doesn't. When using the keyboard, the showcmd info box thing seems to update just before the cursor is actually moved, triggering a "wakeup", and so Neovide is "ready" to start the animation on time. If I set noshowcmd, which I have done in the screen recording below, then I can trigger the delayed animations with the keyboard as well. It still doesn't happen every time, but it does happen.

noshowcmd.mp4

[9mm]

@9mm, since this also changes how the event loop is run, could you also test if it makes any difference when you run with vsync?

cargo run --release -- --vsync

On OSX on this PR i get the same performance as before, so I'd say that's good. It may be even slightly better but it's hard to tell, it is definitely not worse though.

[fredizzimo]

@AThePeanut4, I don’t have those problems on my system with wayland. And it’s hard to tell without actual profiling data.
But it’s possible that wayland does not give us ”permission” to render when we request it. I will try to change it tomorrow to always render the first frame after being idle.

[fredizzimo]

That is, don’t take this branch when num_consecutive_rendered is 0, if you want to try to modify it yourself. I’m not at the computer myself.

neovide/src/window/update_loop.rs

Line 175 in aeaab02

if window_wrapper.vsync.uses_winit_throttling() {

[AThePeanut4]

That is, don’t take this branch when num_consecutive_rendered is 0, if you want to try to modify it yourself. I’m not at the computer myself.

neovide/src/window/update_loop.rs

Line 175 in aeaab02

if window_wrapper.vsync.uses_winit_throttling() {

I made this change:

diff --git a/src/window/update_loop.rs b/src/window/update_loop.rs
index b67147a..cab0449 100644
--- a/src/window/update_loop.rs
+++ b/src/window/update_loop.rs
@@ -172,7 +172,7 @@ impl UpdateLoop {
             Ok(Event::AboutToWait) | Err(false) => {
                 self.should_render.update(window_wrapper.prepare_frame());
                 if self.should_render == ShouldRender::Immediately || !self.idle {
-                    if window_wrapper.vsync.uses_winit_throttling() {
+                    if self.num_consecutive_rendered > 0 && window_wrapper.vsync.uses_winit_throttling() {
                         window_wrapper.windowed_context.window().request_redraw();
                     } else {
                         self.render(window_wrapper);

but it didn't appear to make a difference unfortunately.

FYI, my system info:

• Compositor: Hyprland (latest commit hyprwm/Hyprland@3bb9c7c)
• OS: Arch Linux, with the linux-zen kernel (perhaps this is relevant?).
• CPU: Ryzen 7 5700X
• GPU: RX 6700
• And a 144Hz monitor, as already mentioned.

I've attached tracy captures, one with the change above (latency_change.tracy.gz) and one without (latency.tracy.gz), following these instructions from #2162 (comment):

Could you install tracy https://github.com/wolfpld/tracy and run tracy-capture -o latency.tracy & cargo run --release --features profiling -- --no-fork? And attach latency.tacy here.

I hope they're useful - please let me know if you need me to do anything else.

[fredizzimo]

@AThePeanut4, thanks for the traces. I can definitely see a change between those two captures. I reduces the latency, but it's probably not the latency you are seeing, since it's at most one frame at 120 FPS, so 8ms.

The changed versions renders as soon as the data is received from Neovim, which makes me think that the the latency is something else.

I added some more profiling that should allow us to determine the exact delay from the input until it's rendered. But there's some delay until it's shown on the screen out of our control (the compositor and graphics pipeline). @AThePeanut4, so it would be great if you could take new captures.

BTW, interestingly hyprland seem to reduce the frame rate to 60 FPS when the window is not active. Which could mess a bit with the frame dt detection we currently have, but I don't think that's the main problem here.

[fredizzimo]

I changed this to draft, while figuring out the problems and cleaning up the code a bit.

[fredizzimo]

I was able to repeat it, and I think it should be fixed now.

There was a bug in the latency optimization, it did not request a new frame from wayland. So it was waiting until the idle frame rate had passed or another event from winit was received. And key up is such an event, so that explains why it glitches less when you release the key immediately.

If showcmd is on, then Neovim will send an additional screen draw, which explains why it does not glitch in that situation.

[AThePeanut4]

@fredizzimo Can confirm it is now fixed. Thanks again!

[fredizzimo]

@AThePeanut4. I actually force pushed a slightly different fix now. The previous fix could render an extra frame in some cases.

But I think the result should be the same.

[fredizzimo]

That fix was also slightly wrong.. Time to go to bed.

[fredizzimo]

Hm.. I think something else is broken now.. the scrolling is not smooth

[fredizzimo]

@AThePeanut4, that was actually your configuration that I still had enabled by mistake.
vim.g.neovide_scroll_animation_length = 0.1 is too short, at least on my system, and is unable to smooth out the uneven repeat rate of the keys. 0.2 is still not completely smooth, only 0.3 feels completely smooth.

I'm not sure how to solve that, I think we need some kind of filtering, but that also adds some latency. And predictive solutions can be wrong.

[AThePeanut4]

@fredizzimo Can confirm that on e34d9d9 the command mode animation issue and the initial issue are both still fixed.

As for the scroll animation length, I think I understand what you mean (I'm holding down <C-D> on a really long file, and yeah the scrolling isn't perfectly smooth). The reason I set it to 0.1 is because I don't like how long the scrolling animation is when only pressing <C-D> once, and that is what I find myself doing way more often than holding it down. If I need to find something that I can't search for (e.g. I'd forgotten its name) and thus need to scroll through an entire file, I'd probably tap <C-D> successively rather than hold it down anyway, so that I can actually see what I'm scrolling over.

Perhaps animations could be different when holding keys down, if that's something that would even be possible. Maybe a different animation type (something more linear, perhaps?) or just a different animation length. That would be a separate feature request though - I wouldn't exactly call the current behaviour a bug.

[Kethku]

nit: I believe the only remaining lazy static is settings? Maybe this can be reworded to not imply that there are multiple

[fredizzimo]

I added documentation for the running tracker, which is still remaining.

[9mm]

neovide_backtraces.log

I got this backtrace, im 90% sure it was when using this branch. Not sure if this is directly related though, I dont remember what I did i just know neovide froze for a long time and i had to force quit it, then i noticed it was there

like i said though, its never happened any other time, and neovide never crashes, so might not be worth trying to chase down

[fredizzimo]

@9mm. I don't think it's related to this, but by studying the code there appears to be a bug where it will crash like that if you drag the window on top of a window that is closed while dragging. It would probably be better to report a new issue, and also check if you can repeat it like that.

You probably need some window that autocloses with mouse actions.

[MOldtime]

One suggestion is to animate the cursor blink, such as vsocde

2023-12-17.12-34-21.mp4

[Kethku]

One suggestion is to animate the cursor blink, such as vsocde

2023-12-17.12-34-21.mp4

This is a reasonable feature request but likely out of scope for this PR

[Kethku]

@MOldtime if you were interested in contributing such a change, I'd be happy to give pointers for how to do it. I dont think itd be too hard. First step would be to make an issue with the feature request to discuss further

[MOldtime]

@MOldtime if you were interested in contributing such a change, I'd be happy to give pointers for how to do it. I dont think itd be too hard. First step would be to make an issue with the feature request to discuss further

Thank you for your kindness. This is a great project, but I don't have the experience of developing Rust. I decided to learn. Rust looks like many advantages. This takes a while

[mati865]

This PR has almost solved my issue with Neovide fully utilizing single core after about a minute of being unfocused on Ubuntu 22.04 running in VirtualBox and with Wayland enabled. Previously this has happened every time, now it's just "sometimes".
I haven't had time to debug it but when I do I'll create an issue (unless some other PR solves it fully).

[fredizzimo]

@mati865, there's a small chance that #2188 improves or fixes it, but if not, then please open an issue. I think we will need a log preferably from that PR, which has more diagnostics.

I gave instructions for the tracy usage in this message #2162 (comment)

[mati865]

Yeah, I've been watching #2188 already but planned to wait until it's merged.
However I have tested it today and haven't encountered high CPU usage when unfocused even once for the whole day.