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.

Sign up for GitHub

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

Jump to bottom

Changing background-opacity in config changes unfocused window fill #3109

Open
johnm opened this issue Dec 25, 2024 · 1 comment
Open

Changing background-opacity in config changes unfocused window fill #3109

johnm opened this issue Dec 25, 2024 · 1 comment
Labels
contributor friendly A well-scoped, approachable issue for someone looking to contributor. gui GUI or app issue regardless of platform (i.e. Swift, GTK) os/macos

Comments

@johnm
Copy link
Collaborator

johnm commented Dec 25, 2024

Editing the background-opacity and then reloading the ghostty configuration changes the background color fill that gets applied to the unfocused split.

I have to restart ghostty to fix the problem.

Before:
Image

After:
Image
@mitchellh mitchellh added os/macos contributor friendly A well-scoped, approachable issue for someone looking to contributor. gui GUI or app issue regardless of platform (i.e. Swift, GTK) labels Dec 25, 2024
@mitchellh
Copy link
Contributor

This is probably an issue in the ghosttyConfigDidChange sequence of events for a window or surface (macOS specific). Probably not too difficult to fix.

@mitchellh mitchellh modified the milestone: 1.0 Public Release Dec 25, 2024
@trevor-e trevor-e mentioned this issue Dec 26, 2024
Merged
mitchellh added a commit that referenced this issue Dec 27, 2024
@mitchellh
Fix notification handling from publishing while View is updating (#3177)
f0b77e8 
I was originally looking into this issue:
#3109

When running the logic and triggering a config reload, Xcode warns us
about updating publishable properties from within the notification
callback functions:
<img width="924" alt="Screenshot 2024-12-26 at 5 46 19 PM"
src="https://github.com/user-attachments/assets/38000a09-ffad-4dda-9e2d-a37e5283ff89"
/>

I believe this is because `SurfaceView` is being used as both a bridged
NSView (inside `Surface`) and also an `ObservableObject`, so it's
possible for the notification callback to happen while a SwiftUI render
loop is occurring. The notification delivery happens on whatever thread
posted the message. The better solution long-term is likely to separate
the `ObservableObject` logic into its own class to avoid mixing with the
View logic.

The solution here is to simply move the publishable mutation out of the
current loop via `DispatchQueue.main.async`. I confirmed the warning
goes away with this, and I didn't notice any odd behavior while
reloading config changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
contributor friendly A well-scoped, approachable issue for someone looking to contributor. gui GUI or app issue regardless of platform (i.e. Swift, GTK) os/macos
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mitchellh @johnm