perf: mitigate the cursor flickering issue #298
+28
−47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of letting the `:hl` to redraw all things globally, use the `nvim__redraw` api to limit the redraw on the notification windows only. The api also handles the case of redrawing during blocking events by flushing the screen changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of letting the
:hlto redraw all things globally, use thenvim__redrawapi to limit the redraw on the notification windows only. The api also handles the case of redrawing during blocking events by flushing the screen changes.Mitigate the issues #56, #63 and #273.
After some investigation, it came down to two culprits:
:hlcommands (for fade in/out blending) that caused frequent global redraws.This PR tickles the first one but couldn't fully eliminate the issue. In situations when the redraws are intense, e.g. many/fat notifications fade in/out around the same time, the problem can still occur. However, even in those situations, the flickering issue would disappear as soon as all the fade in/out are done.
Side note: The flickering issue, at least for me, happened on Windows Terminal and WezTerm in Window. The nvim was run inside WSL. The flickering won't happen if I run WezTerm from WSL of VM. My theory is it is some throughput problem of the underneath ConPTY.
Comparisons:
nvim-notify-smallmsg-orig.mp4
nvim-notify-smallmsg-mitigated.mp4
nvim-notify-bigmsg-orig.mp4
nvim-notify-bigmsg-mitigated.mp4
Additional Change
This commit also removes the work done by 4144654. I checked the original discussion the commit referred to but failed to see how the change could help. So if I understand it correctly the commit was trying to solve the issue of the notification window (animation) getting blocked during blocking events. The
redrawcommand you added in the commit, however, is usually executed through the main loop path so it could only be executed once per new notification and wouldn't help the animation to advance further. Instead, since the recurringWindowAnimator:_apply_win_stateis already running outside of the main loop (becauseNotificationService:_runuses the deferring function), we could make use of it to just request a redraw with immediately flushing there to solve the blocking problem (tested with thegetcharcall followed immediately after a notification call).This specific change does not contribute to the performance but I feel like somewhat related. If I misunderstood anything or you don't like this to be part of the commit, let me know :).