-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Improve transparency on macOS #7965
base: master
Are you sure you want to change the base?
Conversation
Window has opaque title bar and real border when config.window_opacity == 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing changelog entry.
// Set initial background opacity and transparency | ||
#[cfg(target_os = "macos")] | ||
set_titlebar_transparent(&window, config.window_opacity() < 1.); | ||
#[cfg(target_os = "macos")] | ||
set_background_opacity(&window, config.window_opacity() as _); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're not calling window.set_transparent
here anymore, why is that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's called in WindowBuilder::with_transparent
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was also called before though? Are you suggesting a winit update has removed the necessity for this on all platforms?
pub fn set_transparent(&self, transparent: bool) { | ||
self.window.set_transparent(transparent); | ||
#[cfg(target_os = "macos")] | ||
set_titlebar_transparent(&self.window, transparent); | ||
} | ||
|
||
pub fn set_background_opacity(&self, _opacity: f32) { | ||
#[cfg(target_os = "macos")] | ||
set_background_opacity(&self.window, _opacity as _) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there ever a scenario where you'd want to call set_background_opacity
without setting transparency? Seems like the two functions can just be combined.
// If transparent, fill background with NSColor.clearColor, | ||
// otherwise fill with NSColor.windowBackgroundColor so that MacOS | ||
// properly draws the window's border and opaque title bar. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like the window's background is filled, but surely that's not what is happening, right? Just the titlebar background?
@@ -496,3 +507,33 @@ fn use_srgb_color_space(window: &WinitWindow) { | |||
let _: () = msg_send![raw_window, setColorSpace: NSColorSpace::sRGBColorSpace(nil)]; | |||
} | |||
} | |||
|
|||
#[cfg(target_os = "macos")] | |||
fn set_background_opacity(window: &WinitWindow, opacity: CGFloat) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These macOS-specific functions should indicate their macOS-specificity. They're also lacking function documentation.
@@ -167,7 +168,7 @@ impl Window { | |||
.with_title(&identity.title) | |||
.with_theme(config.window.theme()) | |||
.with_visible(false) | |||
.with_transparent(true) | |||
.with_transparent(config.window_opacity() < 1.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we've had it this way in the past and that changed. Are you sure this is safe on all platforms? Because I'm not sure that this doesn't change Alacritty's behavior.
@@ -182,8 +183,11 @@ impl Window { | |||
window.set_ime_allowed(true); | |||
window.set_ime_purpose(ImePurpose::Terminal); | |||
|
|||
// Set initial transparency hint. | |||
window.set_transparent(config.window_opacity() < 1.); | |||
// Set initial background opacity and transparency |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Set initial background opacity and transparency | |
// Set initial background opacity and transparency. |
I'm seeing the same thing I observed in the discussion of #7928: No borders and a white title bar even though
|
Extension upon #7928, see that for discussion. I've improved a bit on the window background, by setting the alpha channel to follow the window opacity.
When
window.opacity < 1
, we do two things: