60 fps progress spinner

[pickfire]

Previous spinner kinda look a bit weird given that it needs to wait for progress to spin, makes the editor feels laggy.

Now it spins at 60 fps which makes the editor look fast.

Not sure if there is a better way to do the interval timer at commit fb572fb5f604023a8f2e19b2909761ff32a197ec, not sure if there are any performance issues either. I did see a 2% cpu usage when idle but not sure how to prevent that, maybe render only within a period of time rather than all 8 frames?

The pause at the end to remove the progress message looks weird due to the idle timeout, I was thinking to remove it, but then I figure out if the last message is an error, user should see it, but so far I only see rust-analyzer outputting completed message, so not removing it as I am not sure.

cc @vv9k

[pascalkuthe]

Looks very cool a nice visual improvement 👍 However I am a bit iffy about running the rendering code this often.

Right now it's mostly fine to always redraw everything because usually only the editor is visible and that needs to be redrawn on every command. But with animations that's. a lot of wasted CPU cycles especially for larger files where TS can get slow or even just transversing the document at all for rendering could be slow (altough that will be improved with chunked softwrapping). These cass would also make the animation laggy.

This issue actually already exists in a limited capacity. Scrolling in the picker or typing in command mode can lag if a very large file is opened in the background (altough ebersince the TS query performance improvements that's less of an issue in practice).

I think we can learn from UI libraries here: The compositor should support invalidating components so they can be redraw individually. The easiest implementation right now would simply redraw all componenents that come after the first invalidated componenent (om top of the last render). In most cases we would redraw everything because most cases where we redraw right now invalidate the editor. For compositor events we can simply invalidate the components that consume them.

In this specific case that wouldn't quite work because the status line is not it's own componemnnt. That is kind of unfortunate but we can probably do a workaround inside the editor rendering code

[CptPotato]

I like the consistent spin interval. Though I think having it refresh at a high rate could cause issues when using helix via ssh.

[pascalkuthe]

I like the consistent spin interval. Though I think having it refresh at a high rate could cause issues when using helix via ssh.

Helix computes a diff when rendering and only.emits events for cells that actually changed so during the animation only a single cell should update which would take very little bandwidth

[pickfire]

But I am surprised that even though I limited the render rate, having just sending the emitting a regular event every 16ms is causing a 2% cpu usage even though it only renders 8 frame per second. Maybe somewhere in tokio select it is using the cpu.

[archseer]

Either way it's a lot of operations per cycle since the entire screen is recalculated (but then the render diffing just updates a single character). This is a 👎🏻 for me: it'll waste battery life and the stuttering is actually useful to me too as a sort of a progress bar: it means the LSP is stuck on a certain step.

This would at the very least need the "damage tracking"/region invalidation updates to the compositor that @pascalkuthe is recommending, but I also think it's fine to not have a 60 FPS spinner on the terminal frontend.

[pickfire]

I am thinking of putting 8 fps just enough to render the progress bar and with the event being sent only when there are any progress bar. Okay, I will add region invalidation too.

[sbromberger]

Please test this over a mosh connection as well - my experience with mosh is that it does some weird optimizations with character rendering over high-latency connections. Thanks,

[dpc]

This is a 👎🏻 for me: it'll waste battery life and the stuttering is actually useful to me too as a sort of a progress bar: it means the LSP is stuck on a certain step.

FWIW. If the spinner is running your LSP is probably busy at work so CPUs are not going to be idling.

But I think 60 fps in a CLI is a complete overkill. 8fps seems more reasonable, but I'd be fine even with 1fps, just as long as I can tell that LSP is still spinning instead of scratching my head.