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

tray: Embed in bar window #2609

Merged
merged 69 commits into from Mar 25, 2023
Merged

Conversation

patrick96
Copy link
Member

@patrick96 patrick96 commented Feb 27, 2022

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Other: Replace this with a description of the type of this PR

Description

This is the replacement for #1615 with a different approach.
Instead of trying to force the tray to somehow be a sibling of the bar window, we simply make the tray a child window of the main bar window. Then the tray should never appear in the wrong position or behind the bar or above fullscreen windows when the bar isn't.

There are still some issues. The main one is that if a tray client has a ParentRelative background and a different depth than the bar window, it can't be reparented:

A BadMatch error results if:

  • The new parent window is not on the same screen as the old parent window.
  • The new parent window is the specified window or an inferior of the specified window.
  • The new parent is InputOnly , and the window is not.
  • The specified window has a ParentRelative background, and the new parent window is not the same depth as the specified window.

https://tronche.com/gui/x/xlib/window-and-session-manager/XReparentWindow.html

For now, I just remove the background pixmap from all tray clients for testing. However, in for this to be merged, we need to keep the pixmap set by the client.

TODOs:

  • Factor out old tray manager + client and use it for tray-position trays and reimplement the whole thing for modules. Allows us to make breaking changes.
    • Make tray module start tray_manager
    • Replace signals with direct interaction with tray module
  • Centrally collect visuals and depths and use them in all other places. This way, both the bar and the tray windows can use the same visual and depth (if needed)
  • Move all the client configuration into tray_client
  • Better track tray manager and tray client states
  • We receive the XCB_MAP_NOTIFY events delayed (when the next X event is received). Fixed in fix: Handle X events before polling for IO #2820
  • Store tray clients in smart pointers (and make non-copyable and non-movable)
  • Set _NET_SYSTEM_TRAY_VISUAL
  • Correctly apply pseudo-transparency to wrapper windows
  • Consider supporting true transparency as in the regular bar window -> No. This requires manual compositing using Xrender. Could be done in the future, but for now I don't want to invest more time into that.
  • Address TODOs
  • Test with pseudo-transparency
  • Test borders/rounded corners with tray
    • Rounded corners: ✔️, rounded corners only visible with enough padding
    • Borders: ✔️
  • Test without wallpaper -> Just uses the tray background
  • Test with X server without 32-bit visual (is this even possible nowadays? -> disabled COMPOSITE extension?) -> Polybar can't run without composite extension
  • Check which referenced issues in [WIP] Fix tray restacking issues #1615 are fixed by this.
  • Go over all tray-related issues and see which this PR applies to.

Tray Window Background

Writing this down for later...

To get the proper background for the main tray window (background for the wrapper windows is something for later), we should be able to simply use a ParentRelative back pixmap.
However, currently the bar doesn't render anything in the place where the tray is (except for tray-position = center).
To fix this, we have to also render the background in that excluded rectangle.

EDIT: This doesn't directly work because the bar window does not have a background pixmap. Every render cycle (and Expose event), it copies the pixmap into the window.
We may need to do something similar for the tray.
What we can do is use ParentRelative for the tray and run xcb_clear_area on the tray window (because that copies the pixmap from the bar window) whenever the bar window content updates.

It seems it is not possible (or at least not using the usual tools) to create a transparent child window, even with a compositor.

EDIT2: We are now using pseudo-transparency (or a solid color) for each tray icon individually (each wrapper window has its own back pixmap).
We cannot easily use ParentRelative here since the depths may not match. An option would be to copy the appropriate regions of the bar pixmap into the wrapper pixmap on each render cycle.
Unsure how this plays with true transparency in the bar window.

Regarding Reparenting

When not using _NET_SYSTEM_TRAY_VISUAL and using a 32-bit visual, reparenting to the bar window (not the wrapper with appropriate depth) often fails. But if the visual is switched to 24-bit, reparenting seems to always work (also for windows with different depths).

Setting the client pixmap to ParentRelative results in a BadMatch error for 32-bit clients even if the wrapper has the same depth and uses the same visual.

The dropbox icons

The dropbox icon behaves weirdly. I'm guessing for some reason its background pixmap is undefined (that's why it sometimes contains image data from other tray icons).
When using a larger tray-maxsize, the background image seems to be only applied to a small square on the top-left.

32-bit Visuals + Transparency

It seems we can't get _NET_SYSTEM_TRAY_VISUAL to work with a 32-bit visual without a ton of extra work. We basically need to redirect the client's window contents and then composite them over the bar's (or the wrapper window's) pixmap.
For now, we should not set _NET_SYSTEM_TRAY_VISUAL and still only support pseudo-transparency for the tray.

In case we want to do this in the future, this tutorial (wayback machine) seems to show the basics (using Xlib, but XCB has equivalent functions).
tint2 also seems to do this in systray_render_icon_composited.

Related Issues & Documents

Part of #2689
Closes #1615
Fixes #2433
Fixes #1537

Tray rendering issues:

It is unclear whether these are actually fixed. There are still some (seemingly unsolvable) problems with the dropbox icons

We should create a new issue where we collect applications that behave this way, maybe we can figure out what's causing this.
These issues should all be closed and redirected to the new issues in case the problem is still happening with the tray module:

We have recently implemented a new tray that runs as a module where we rewrote a lot of the rendering code (#2609, #2689).

Some of these rendering issues may be fixed by this, but there are still some that may occur (I have been struggling to fix the dropbox icon using arbitrary images as its background data).
I am going to close this issue, if you experience rendering issues like these while using the new tray module (the old tray using `tray-position` still has the same issues), please report the application and your polybar version (`polybar -v`) together with a screenshot in #2933.

TODO unlock issue #2933

Restacking Issues

Fixes #425
Fixes #789
Fixes #1995
Fixes #898
Fixes #1355
Fixes #2035
Fixes #2460
Fixes #2212 (Likely same issue as #2460)

References:

Documentation (check all applicable)

  • This PR requires changes to the Wiki documentation (describe the changes)
  • This PR requires changes to the documentation inside the git repo (please add them to the PR).
  • Does not require documentation changes

A complete set of doc changes will be made as part of finishing #2689

@codecov
Copy link

codecov bot commented Feb 27, 2022

Codecov Report

Merging #2609 (921e2d0) into master (8cc1b4f) will decrease coverage by 0.56%.
The diff coverage is 0.21%.

@@            Coverage Diff             @@
##           master    #2609      +/-   ##
==========================================
- Coverage   13.17%   12.61%   -0.56%     
==========================================
  Files         162      162              
  Lines       11509    12040     +531     
==========================================
+ Hits         1516     1519       +3     
- Misses       9993    10521     +528     
Flag Coverage Δ
unittests 12.61% <0.21%> (-0.56%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/components/controller.hpp 0.00% <ø> (ø)
include/components/eventloop.hpp 0.00% <0.00%> (ø)
include/components/logger.hpp 52.38% <ø> (ø)
include/events/signal.hpp 0.00% <ø> (ø)
include/modules/ipc.hpp 0.00% <ø> (ø)
include/modules/meta/inotify_module.hpp 0.00% <ø> (ø)
include/modules/tray.hpp 0.00% <0.00%> (ø)
include/x11/connection.hpp 0.00% <0.00%> (ø)
include/x11/extensions/randr.hpp 0.00% <ø> (ø)
include/x11/winspec.hpp 0.00% <ø> (ø)
... and 23 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@sheepy0125
Copy link

thank you for this! i can finally play full screen games without having the tray in the way 👍

@patrick96 patrick96 force-pushed the tray-child-window branch 2 times, most recently from 99bb0a5 to 9c6aca7 Compare April 4, 2022 19:05
Currently requires a dirty workaround to prevent tray icons with
different depths from crashing
The client window has to be added to the save set after it has been
reparented. Otherwise if the reparenting fails weird stuff happens
(windows in the save set have to be child windows of windows created by
the current connection).
The wrapper window must define a border background if the depth doesn't
match the parent window.
If an exception is thrown earlier, stopping the eventloop produces an
error.
Not doign this. Using the desired background as the X window background
color would require us to always first check before using the pixmap or
cairo context.
Currently, we don't support 32-bit visuals and don't set
_NET_SYSTEM_TRAY_VISUAL

It is unclear what happens if the default visual (which is used as a
fallback if _NET_SYSTEM_TRAY_VISUAL is not set) is 32-bit.
In that case, we may need to explicitly use a 24-bit visual.
Unclear why it is needed, neither i3bar, nor stalonetray do delayed
notifications
The border size was not taken into account when calculating the tray
icon's y-position
@patrick96 patrick96 marked this pull request as ready for review March 25, 2023 20:54
@quantenzitrone
Copy link
Contributor

nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment