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

Make visual bell flash much more gentle #2937

Closed
wants to merge 10 commits into from

Conversation

keynslug
Copy link
Contributor

Right now visual bell makes background flash sharply with bright white (when configured with darkish color theme). This causes eye strain, especially prominent in unlit environments.

This change makes background bounce smoothly between regular bg color and highlight (selection) bg color for the configured visual bell duration. Intensity is animated with cubic easing functions. It currently peaks at 20% of the duration, this is hardcoded.


Overall I acknowledge this change as quick and somewhat dirty starting point. For example I chose not to introduce any new configuration options yet. I'm wholly open to your suggestions on how to improve it.

Right now visual bell makes background flash sharply with bright white
(when configured with darkish color theme). This causes eye strain,
especially prominent in unlit environments.

This change makes background bounce smoothly between regular bg color
and highlight (selection) bg color for the configured visual bell
duration. Intensity is animated with cubic easing functions. It
currently peaks at 20% of the duration, this is hardcoded.
@kovidgoyal
Copy link
Owner

In principle, looks ok. Some comments:

  1. How well does this work in inverted text? Or when using a fullscreen program like vim that sets its own colors. With background_opacity?

  2. Make the easing and intensity functions static inline.

  3. I dont think this needs config options, lets just pick the numbers for now and config options can always be added later if there is actual demand.

@keynslug
Copy link
Contributor Author

keynslug commented Aug 27, 2020

  1. How well does this work in inverted text? Or when using a fullscreen program like vim that sets its own colors.

Spot on, not great at all. I guess light theme users are also affected. I'll come up with some robust solution.

With background_opacity?

That seems fine.

  1. Make the easing and intensity functions static inline.

Done.

Blend highlight color with pegtop's softlight mode over both background
and half as much over foreground. This should help with flash visibility
in light themed and inverted colors contexts.
@kovidgoyal
Copy link
Owner

Doesnt seem very smooth to me, see attached screencast
screencast.zip

@kovidgoyal
Copy link
Owner

You know, stepping outside the box, a little. I dont see why we need to
render it via some kind fo reverse video effect at all. Why not just
render a semi-transparent overlay color over the entire window? Might
look much better and have zero performance impact when a visual bell is
not occurring.

@keynslug
Copy link
Contributor Author

Why not just
render a semi-transparent overlay color over the entire window? Might
look much better and have zero performance impact when a visual bell is
not occurring.

I have no objections. Thought of something similar, had a question though: when a bell fires in an inactive tab kitty does not start visual bell duration timer, is that right?

@kovidgoyal
Copy link
Owner

kovidgoyal commented Aug 27, 2020 via email

@keynslug
Copy link
Contributor Author

Doesnt seem very smooth to me, see attached screencast
screencast.zip

MacOS? Seems to me this is happening because screen repaints are effectively limited by cursor blink frequency. Is there some way I can request repaints more often? Also wondering why am I not seeing the same under Linux + Wayland.

@keynslug
Copy link
Contributor Author

Yes, I think that is the case instead it shows a bell in the tab bar. Try it and see.

Great, I'll try to add a seprate pass for the flash then.

@kovidgoyal
Copy link
Owner

kovidgoyal commented Aug 27, 2020 via email

@kovidgoyal
Copy link
Owner

Another idea: rather than an overlay color, maybe just draw a big exclamation mark or bell symbol centered over the window. It should be fairly easy to draw an exclamation mark in GLSL.

@kovidgoyal
Copy link
Owner

To make it less disruptive maybe just a red rectangle over the window edges, so the content is not obsucred. There are lots of options now that we are thinking outside the box :)

@keynslug
Copy link
Contributor Author

keynslug commented Sep 1, 2020

But I think just using a single overlay color with no need for these
complications would be easier. And look fine.

Made an attempt. Looks nice and a bit more consistent.

There's one little issue I found though. No matter what I try this effect seems to affect background opacity for some reason. For example if you run ./kitty/launcher/kitty -o background_opacity=0.0 -o visual_bell_duration=1.0 -o selection_background=#a0a0a0 the flash would make window not fully transparent for a flash duration.

To make it less disruptive maybe just a red rectangle over the window edges, so the content is not obsucred.

This may be even better, that's right. I would propose to flash color of a screen border though, this way it's much clearer which screen in a multiscreen layout wants your attention. My only doubt is that user might have turned off border completely so this wouldn't work.

I want to give it a try, so we have at least two options to choose from.

@gshpychka
Copy link

@kovidgoyal did you mean to close this PR?

@kovidgoyal
Copy link
Owner

Yes, since it is merged.

@gshpychka
Copy link

gshpychka commented Aug 11, 2021

@kovidgoyal not sure what you mean. It's closed, not merged. Am I missing something about how GitHub works?

@kovidgoyal
Copy link
Owner

Read the commit that closed it.

@Luflosi
Copy link
Contributor

Luflosi commented Aug 11, 2021

screenshot
The commit adds a new entry to the changelog. Previous commits add the changes from this PR if you look at the commit history. The pull request is marked as "closed" since it was not merged directly but the commit was rebased and maybe improved/changed.

@gshpychka
Copy link

Read the commit that closed it.

I did, and have no idea what it means. It's linking to this PR, which is closed (not merged).

The pull request is marked as "closed" since it was not merged directly but the commit was rebased and maybe improved/changed.

Oof, interesting.

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

4 participants