-
Notifications
You must be signed in to change notification settings - Fork 107
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
Theme integration jammy #198
Conversation
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 am much more happy with this draft, that looks really good. :)
Would you mind to rebase this on top of #194? That should actually simplify the code in shell/workspace.rs
a bit, since you don't need the theme any more for maximizing/fullscreening (I think) and I hope I can merge that PR soon anyway. (If not we can still merge this into the branch and have me maintain it.)
Ok, I'll rebase on #194 today |
refactor: only apply updates if there is a change in the theme refactor: include theme in state cleanup: theme integration
fb5382a
to
ca8bdb3
Compare
src/backend/render/mod.rs
Outdated
renderer, | ||
age, | ||
&elements, | ||
crate::theme::clear_color(state.theme.cosmic()), |
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.
Testing an implementation of ext-session-lock, I noticed if I passed a different color to damage_tracker.render_output
when the session lock was enabled, it ended up flickering between that and the previous color. The damage tracker I guess doesn't automatically recognize a change to the clear color as damage.
So I think to change clear_color
dynamically without possible damage tracking issues, Smithay or cosmic-comp may need some change here to mark everything as damaged when this changes?
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.
ok, I can make the clear color static for now.
} | ||
} | ||
|
||
pub fn watch_theme(handle: LoopHandle<'_, State>) -> Result<(), cosmic_config::Error> { |
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.
Hm. We could use ConfigWatchSource
from libcosmic here, but I guess that would mean 3 channels and calloop sources instead of 1...
I guess we end up spawning a thread here for each watch
call? Probably the overhead doesn't matter that much.
So that should be fine for now, though it would be nice to figure out how to use epoll+inotify and kqueue to monitor for changes without threads and channels.
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.
LGTM!
No description provided.