Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Smooth cursor blink animation option #2421

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

agraven
Copy link
Contributor

@agraven agraven commented Mar 16, 2024

Add a new g:neovide_cursor_smooth_blink setting which will make the cursor smoothly transition between its on and off state when enabled.

Screencast.from.2024-03-16.16-33-33.webm

What kind of change does this PR introduce?

  • Feature

Did this PR introduce a breaking change?

  • No

@agraven agraven force-pushed the smooth-cursor-blink branch 2 times, most recently from c552db1 to 00d544f Compare March 16, 2024 17:33
@agraven agraven marked this pull request as ready for review March 16, 2024 17:34
@agraven agraven changed the title feat: Smooth blink animation feat: Smooth cursor blink animation option Mar 16, 2024
@MultisampledNight
Copy link
Contributor

Thank you!!

@MultisampledNight
Copy link
Contributor

When testing, I've found that the new option causes the cursor to disappear when configured to be non-blinking/static. Minimal repro is

  1. Launch neovide -- --clean
  2. :let neovide_cursor_smooth_blink=v:true

Causing the cursor to disappear.

This is an issue, since in some configurations, it might be desirable to have the cursor static in one mode but not in another.

self.get_delay() in src/renderer/cursor_renderer/blink.rs:93 seems to return 0 in that case, causing remaining / total to be actually 0 / 0NaN. I'd guess it might make sense to add a guard condition for is_static in fn opacity?

@agraven
Copy link
Contributor Author

agraven commented Mar 22, 2024

The change

@@ -63,7 +63,7 @@ impl BlinkStatus {
         let current_cursor = self.current_cursor.as_ref().unwrap();
 
         if is_static(current_cursor) {
-            self.state = BlinkState::On;
+            self.state = BlinkState::Waiting;
             ShouldRender::Wait
         } else {
             if self.transition_time <= now {

i.e. setting the BlinkState for when blinking is disabled to BlinkState::Waiting. This doesn't seem to cause any issue after some quick testing, and it would integrate better with the changes to the update loop, on top of feeling more semantically correct to me in general, since BlinkState::On implies there is an ongoing blinking process, while BlinkState::Waiting more accurately represents that blinking is inactive, just indefinitely instead of for blinkwait milliseconds

@MultisampledNight
Copy link
Contributor

Looks good to me, thank you again!! Merging...

@MultisampledNight MultisampledNight merged commit a11c8e3 into neovide:main Mar 22, 2024
3 checks passed
@agraven agraven deleted the smooth-cursor-blink branch March 22, 2024 21:40
@hudgins
Copy link

hudgins commented May 23, 2024

I love this, but I've noticed it causes Neovide to report 15% CPU usage, where it is normally at about 1%.

@agraven
Copy link
Contributor Author

agraven commented Jun 2, 2024

@hudgins That does seem like a bit much. Could you check if you get the same CPU usage when running neovide with --no-idle and smooth blink disabled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants