-
Notifications
You must be signed in to change notification settings - Fork 599
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Systray composition support #3664
base: master
Are you sure you want to change the base?
Conversation
d931a17
to
d3c857a
Compare
79b18a9
to
554a19d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3664 +/- ##
==========================================
+ Coverage 90.65% 91.18% +0.53%
==========================================
Files 854 927 +73
Lines 54774 59629 +4855
==========================================
+ Hits 49653 54372 +4719
- Misses 5121 5257 +136
Flags with carried forward coverage won't be shown. Click here to find out more.
|
c90bb35
to
6ec7630
Compare
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.
Basically 👍 but I do not understand where this "mess with the background pixel of tray windows" comes from. I do not think any spec allows this. Is there some program out there that required this specifically?
/* warn("Fixing the background of the systray window 0x%x possibly because the client does not support composition.", embed_win); */ | ||
xcb_change_window_attributes(globalconf.connection, embed_win, XCB_CW_BACK_PIXEL, | ||
(const uint32_t []){ globalconf.systray.background_pixel }); | ||
xcb_clear_area(globalconf.connection, 1, embed_win, 0, 0, 0, 0); |
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.
Uhm, what? I do not understand the reason behind this whole code block. If some window has a different depth.... so be it. I do not see anything in https://specifications.freedesktop.org/systemtray-spec/systemtray-spec-0.3.html or https://specifications.freedesktop.org/xembed-spec/xembed-spec-latest.html allowing to do this. Do other tray compositing managers do this?
(Basically same comment below to the code that handles changes to globalconf.systray.background_pixel
)
Oh also: This might warrant a comment in the code. It just occurred to me that:
- legacy systray clients use a ParentRelative backgroud
- reparenting such a window into a window with a different depth does not work
- thus, this is here to "make things work"
(But what about all the other window properties that are needed for a different depth window? According to https://stackoverflow.com/questions/3645632/how-to-create-a-window-with-a-bit-depth-of-32, one also needs to set a colormap and a border pixel/border pixmap. Why doesn't this code need that?)
tests/test-systray.c
Outdated
XCB_ATOM_VISUALID, 0, 1), | ||
NULL); | ||
tray_visual_id = screen->root_visual; | ||
if(xcb_get_property_value_length(tray_visual_r)) { |
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.
if(tray_visual_r != NULL && xcb_get_property_value_length(tray_visual_r)) {
In theory, this also needs to check for a 32 bit value and length > 0... but, whatever.
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.
Done
Ah, right after pressing "submit", I find it: https://specifications.freedesktop.org/systemtray-spec/systemtray-spec-0.3.html#visuals
Okay. (I do not think there is much of a difference between background pixmap and background pixel, so, whatever.) |
d765c49
to
7ed7b91
Compare
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.
Thanks!
…s without it. Add a beautiful.systray_skip_bg to skip background drawing, so it can blend nicely with other widgets.
7ed7b91
to
7464c8e
Compare
oh wow, wasn't expecting someone to actually pick that, many thanks 🔥 |
? get_color(conn, cm, 0xffff, 0x9999, 0x0000) | ||
: XCB_BACK_PIXMAP_PARENT_RELATIVE, | ||
screen->black_pixel, cm | ||
}); | ||
xcb_change_property(conn, XCB_PROP_MODE_REPLACE, window, _XEMBED_INFO, _XEMBED_INFO, 32, 2, (uint32_t[]) { 0, 1 }); | ||
|
||
// Make our window a systray icon |
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 guess the comment itself should move together with the corresponding line
restarting to unstuck the CI |
This allows systray to be rendered consistently in the widget hierarchy with proper shape and transparency.
The default appearance should stay the same, while the newly introduced beautiful.systray_skip_bg allows skipping the boring systray background.