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

gtk(wayland): add support for background blur on KDE Plasma #4403

Merged
merged 8 commits into from
Jan 5, 2025

Conversation

pluiedev
Copy link
Contributor

@pluiedev pluiedev commented Jan 2, 2025

Also establishes a foundation for Wayland support and fixes a minor bug (GTK windows remaining opaque when background-opacity is set to 1 on startup and later updated to less than 1 with a config reload)

Can't update the Zig cache hash myself since I'm currently in China and my proxy's broken for some reason :(

See also #4361, part of #4626

@pluiedev pluiedev force-pushed the push-vruxlyxvvwpx branch 5 times, most recently from a701ca2 to 4ab0a82 Compare January 2, 2025 15:53
src/config/Config.zig Outdated Show resolved Hide resolved
Copy link
Collaborator

@gabydd gabydd left a comment

Choose a reason for hiding this comment

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

We should consider resource management here for example do we need to destroy the blur manager on close. And do we need to handle the display disconnect or does gtk do that I think you can use WAYLAND_DEBUG=1 to see the Wayland messages which should have the destroy and disconnect stuff there

src/apprt/gtk/Window.zig Outdated Show resolved Hide resolved
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

This is looking extremely good. The code quality here is good and the complexity is very manageable. Great work. I've requested some changes and left some comments.

nix/package.nix Show resolved Hide resolved
src/apprt/gtk/Window.zig Outdated Show resolved Hide resolved
src/apprt/gtk/wayland.zig Outdated Show resolved Hide resolved
src/apprt/gtk/wayland.zig Outdated Show resolved Hide resolved
src/apprt/gtk/wayland.zig Outdated Show resolved Hide resolved
src/apprt/gtk/wayland.zig Outdated Show resolved Hide resolved
src/config/Config.zig Outdated Show resolved Hide resolved
@pluiedev
Copy link
Contributor Author

pluiedev commented Jan 3, 2025

We should consider resource management here for example do we need to destroy the blur manager on close.

You can't do that, actually. There isn't a destructor. I've looked at how winit does it (Apache 2.0 code so I think it's fine) and they only clean up the blur token.

And do we need to handle the display disconnect or does gtk do that I think you can use WAYLAND_DEBUG=1 to see the Wayland messages which should have the destroy and disconnect stuff there

They're all managed by GTK (GDK, to be more precise), so we don't have to worry about that at all

@pluiedev pluiedev force-pushed the push-vruxlyxvvwpx branch 2 times, most recently from 27affa9 to 2e6ddd1 Compare January 4, 2025 05:46
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

I apologize but a few more requests here. This is close! Sorry for the delay.

build.zig Show resolved Hide resolved
src/apprt/gtk/wayland.zig Outdated Show resolved Hide resolved
src/apprt/gtk/wayland.zig Outdated Show resolved Hide resolved
Currently the `background` CSS class is added once on startup and never removed
or re-added. This is problematic as that if Ghostty was started with an opaque
window but then its config was reloaded with a `background-opacity` less than 1,
the window won't actually become translucent, and it would only appear as if the
background colors had become faded (because the window is still styled to be
opaque).
@mitchellh mitchellh enabled auto-merge January 5, 2025 20:36
@mitchellh
Copy link
Contributor

Automerge is on. Send it.

@mitchellh mitchellh merged commit f14c0f5 into ghostty-org:main Jan 5, 2025
24 checks passed
@github-actions github-actions bot added this to the 1.0.2 milestone Jan 5, 2025
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.

6 participants