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

Only disable drop shadow on macOS when window background opacity is transparent #445

Merged
merged 2 commits into from
Jan 30, 2021

Conversation

fanzeyi
Copy link
Contributor

@fanzeyi fanzeyi commented Jan 30, 2021

as title.

see #310

Screen.Recording.2021-01-29.at.7.57.12.PM.mov

Copy link
Owner

@wez wez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with the spirit of this change but I'm not sure about the implementation.
There are two main things about it that don't sit well with me:

  • The added method to control the opacity doesn't change the opacity at all (that's a function of the alpha in the renderer)
  • There's some fan-out through to all of the different platforms from the change in the construction of the window

What do you think about exposing the opacity configuration through the relatively recently introduced WindowConfiguration trait for the window layer over here:

https://github.com/wez/wezterm/blob/master/window/src/configuration.rs#L20

That way we don't need to add more parameters to the constructor.
I think the problem then becomes with letting the window layer know that the config changed.

So, perhaps a method like config_did_change or advise_or_config_change or some better name could be added to WindowOps and WindowOpsMut would do the job?

In the macos implementation of that method you'd use the WindowConfiguration trait to see what the opacity is set to and then window.setHasShadow_ and window.setOpaque_ accordingly.

window/src/os/macos/window.rs Outdated Show resolved Hide resolved
@wez wez merged commit 6b94013 into wez:master Jan 30, 2021
wez added a commit that referenced this pull request Jan 30, 2021
wez added a commit that referenced this pull request Jan 30, 2021
wez added a commit that referenced this pull request Jan 30, 2021
@wez
Copy link
Owner

wez commented Jan 30, 2021

Thanks!

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.

2 participants