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

2935 System tray icons overlap when changing status #2947

Closed
wants to merge 4 commits into from
Closed

2935 System tray icons overlap when changing status #2947

wants to merge 4 commits into from

Conversation

krive001
Copy link
Contributor

Bar background set.

@elParaguayo
Copy link
Member

Odd. I commented on the openweather widget pr and the comment's here.

As noted above, we shouldn't change the default Bar colour.

@krive001
Copy link
Contributor Author

Sorry. Merge two PR.

@elParaguayo
Copy link
Member

Ok. Changing the background isn't a fix for the systray issue. We need to work out what's actually going on.

@krive001
Copy link
Contributor Author

Not because you can not paint because it is transparent, but because here (e3487ba) who was led from the internal window to the bar binding.

@elParaguayo
Copy link
Member

That commit is for Wayland which should have no impact at all on SysTray (which only runs on an X11 backend) . Any file in libqtile/backend/wayland will not be run on any X11 backend.

Can you highlight a particular line that you think's the issue.

@krive001
Copy link
Contributor Author

xprop to the bar or qtile cmd-obj -o cmd -f windows
before:

WM_STATE(WM_STATE):
		window state: Normal
		icon window: 0x0
QTILE_INTERNAL(CARDINAL) = 1

After:

QTILE_INTERNAL(CARDINAL) = 1
WM_STATE(WM_STATE):
		window state: Normal
		icon window: 0x0

@krive001
Copy link
Contributor Author

The wayland is a rudimentary thing you may never mind. X has been in development for decades.

@krive001
Copy link
Contributor Author

That's why the sway was created from the i3. The two cannot be considered as one thing.

@elParaguayo
Copy link
Member

elParaguayo commented Nov 15, 2021

I'm sorry, I just have no idea what you are trying to say. You've said there's an issue with e3487ba but that commit is Wayland only and should have no impact on SysTray which will only ever run on X11.

As for the xprop output, you've pasted that before, what's the issue? They say the same thing just in a different order.

Yes, I know sway is different from i3, what is your point? We don't consider them as one thing, we do the complete opposite: they are separate backends which the user decides which one to use.

@krive001
Copy link
Contributor Author

The fact that it is not the bar that handles the window, but the X from here.
There are settings that qtile cannot access.
It would have been more noble to branch qtile before the introduction of wayland.

@krive001
Copy link
Contributor Author

Because X and Wayland do not treat windows the same way.
There are completely different API calls.

@m-col
Copy link
Member

m-col commented Nov 15, 2021

They are not treated the same way.

X11 internal windows: https://github.com/qtile/qtile/blob/master/libqtile/backend/x11/window.py#L1078
Wayland internal windows: https://github.com/qtile/qtile/blob/master/libqtile/backend/wayland/window.py#L691

The two backends are split into separate folders and only one of these is used at a time: https://github.com/qtile/qtile/tree/master/libqtile/backend

@krive001
Copy link
Contributor Author

And the event mask?
They were thus ignored.

@m-col
Copy link
Member

m-col commented Nov 15, 2021

Are you missing events?

@krive001
Copy link
Contributor Author

From this commit, it all entrusts window operation to X11, not the bar.

@m-col
Copy link
Member

m-col commented Nov 15, 2021

Are you missing events?

@krive001
Copy link
Contributor Author

My problem is that from here all the windows on the bar will get out. Which I don't think is right.
From now on, the bar will have no effect on the window

@krive001
Copy link
Contributor Author

I don't think visual_id should be changed to qtile,

@elParaguayo
Copy link
Member

What do mean the windows "will get out"?

What effect should the bar have on the windows? (which windows?)

@elParaguayo
Copy link
Member

elParaguayo commented Nov 15, 2021

I don't think visual_id should be changed to qtile,

Fine. Then you're welcome to play around with it and see what works for you. If you get a fix that works then we'd be very happy to merge it.

@krive001
Copy link
Contributor Author

krive001 commented Nov 15, 2021

Why should the systemtray appear on root window?

@elParaguayo
Copy link
Member

Why do you say they're on the root window? SysTray creates a new window.

From my testing: if you set a bar with a 32 bit colour depth then the system tray icon windows have a transparent background and you can't set this to the bar background. The fix for this was, where the user wanted an opaque bar, to set the bar colour depth to 24 bit and tell the SysTray to advertise that visual_id so windows know what depth to use. Replacing the backpixmap on 24 bit windows does seem to work.

@krive001
Copy link
Contributor Author

Because he was taken out with this commit. Therefore, it is not inky to paint. Nert is not part of the bar.

@krive001
Copy link
Contributor Author

qtile cmd-obj -o cmd -f windows | grep "'id'" | awk '{print $2}'

@elParaguayo
Copy link
Member

If "this commit" is that Wayland one, it has nothing to do with the issue here. It is a completely separate backend.

@krive001
Copy link
Contributor Author

Because it has to be part of the bar.

@elParaguayo
Copy link
Member

It's not part of the bar. It's a separate window. That's the whole reason it's a problem and why other window managers also have issues.

@krive001
Copy link
Contributor Author

Add Core.create_internal as single point to create Internal windows
This adds a method to the base `Core` class that can be implemented in
backends to create and return `Internal` windows. The method is not
abstract, instead raising `NotImplementedError` when called, so backends
could hypothetically not bother with `Internal`s but otherwise work.

All instances within libqtile of creating `Internal` windows (`Popup`,
`Bar` & `TreeTab`) manually put the window ID into `qtile.windows_map`.
Instead, `Internal` has been updated to do a `qtile.manage` with the new
window so that there is no need, and `Qtile.manage` remains The One Way
of managing windows.

`Core.create_internal` can be accessed the same independently on which
backend is being used, whereas the class method `Internal.create`
requires importing `Internal` directly, which is not backend-agnostic.
This commit replaces the latter with the former.

A nice side-effect of this is that the super ugly hack we used to test
`Popup` has been changed so that it is eval-ed (so still kinda ugly, but
not _as_ bad imo) within the running manager using the proper interface
for it.

@krive001
Copy link
Contributor Author

Maybe @tych0 understands.

@elParaguayo
Copy link
Member

We're going round in circles.

I need you to explain what issue you think that causes. You've said twice that the windows "get out" of the bar but you haven't yet explained what that means (at least, not in a way that I understand).

Also, note that the SysTray window is not an Internal window. It is an instance of libqtile.backend.x11.window._Window.

The Bar is an Internal window but that commit doesn't really change anything. Whether or not the id is in windows_map is not relevant to the System Tray.

@krive001
Copy link
Contributor Author

After:

QTILE_INTERNAL(CARDINAL) = 1
WM_STATE(WM_STATE):
window state: Normal
icon window: 0x0

Before:

WM_STATE(WM_STATE):
window state: Normal
icon window: 0x0
QTILE_INTERNAL(CARDINAL) = 1

@elParaguayo
Copy link
Member

You've shown me that twice and neither time have you explained what the problem is.

The two properties are the same. The order doesn't matter.

@krive001
Copy link
Contributor Author

Which do you think is the bar and the internal window?
Why did it work well in the meantime?
It doesn't matter.
It belongs to Bar or not.

@krive001
Copy link
Contributor Author

Everything has gone wrong since then.

@elParaguayo
Copy link
Member

elParaguayo commented Nov 15, 2021

Which do you think is the bar and the internal window?

All of that is the bar which is an Internal window. The QTILE_INTERNAL property is just a flag that we add to windows to identify them as ones that Qtile has created. It has no relevance to how that window is rendered.

win.set_property("QTILE_INTERNAL", 1)

Why did it work well in the meantime?

That's a good question but it depends what you mean by "works well".

I believe the Systray worked before we merged the commit which introduced transparency for the Bar. With that commit, SysTray works with a fully transparent or fully opaque bar (at least it did in my testing). However, please note that I cannot replicate the overlapping icons issue so I can't verify the cause which is very frustrating.

Everything has gone wrong since then.

What is "everything"?

Show me something working before that commit and after it and I'll have something to try and work with.

@krive001
Copy link
Contributor Author

That the internal window should continue to be handled by the bar.

@krive001
Copy link
Contributor Author

And not the root window.

@elParaguayo
Copy link
Member

That the internal window should continue to be handled by the bar.

The Internal window (that you're seeing with xprop) is the Bar. The window that SysTray creates is handled by the widget. What do you want the Bar to do to it?

One other question for you: have you done a git bisect (or other test) where you can show the issue (whatever it is, I'm still not 100% certain) started? If so, post much, much more detail (screenshots, logs, explanations about what you're seeing etc.). One sentence replies aren't sufficient.

If you can't do that for me then, unfortunately, there's not much more I can do for you. I can't keep replying to this.

@krive001
Copy link
Contributor Author

If you do not have a systemtray widget, you still have an internal window.

@krive001
Copy link
Contributor Author

Commit:

Add Core.create_internal as single point to create Internal windows
This adds a method to the base `Core` class that can be implemented in
backends to create and return `Internal` windows. The method is not
abstract, instead raising `NotImplementedError` when called, so backends
could hypothetically not bother with `Internal`s but otherwise work.

All instances within libqtile of creating `Internal` windows (`Popup`,
`Bar` & `TreeTab`) manually put the window ID into `qtile.windows_map`.
Instead, `Internal` has been updated to do a `qtile.manage` with the new
window so that there is no need, and `Qtile.manage` remains The One Way
of managing windows.

`Core.create_internal` can be accessed the same independently on which
backend is being used, whereas the class method `Internal.create`
requires importing `Internal` directly, which is not backend-agnostic.
This commit replaces the latter with the former.

@elParaguayo
Copy link
Member

elParaguayo commented Nov 15, 2021

If you do not have a systemtray widget, you still have an internal window.

I cannot say this enough times: the Internal window is the Bar. It is nothing to do with the System Tray.

You can see that here:

qtile/libqtile/bar.py

Lines 260 to 265 in 802b82e

self.window = self.qtile.core.create_internal(
self.x, self.y, width, height, depth
)
else:
self.window = self.qtile.core.create_internal(self.x, self.y, width, height)

As also mentioned above, the System Tray window is not an instance of Internal.

@krive001
Copy link
Contributor Author

krive001 commented Nov 15, 2021

All instances within libqtile of creating `Internal` windows (`Popup`,
`Bar` & `TreeTab`) manually put the window ID into `qtile.windows_map`.
Instead, `Internal` has been updated to do a `qtile.manage` with the new
window so that there is no need, and `Qtile.manage` remains The One Way
of managing windows.

@elParaguayo
Copy link
Member

elParaguayo commented Nov 15, 2021

Yes. The bar is created via create_internal which calls qtile.manage (line 555 below).

def create_internal(self, x: int, y: int, width: int, height: int,
desired_depth: Optional[int] = 32) -> base.Internal:
assert self.qtile is not None
win = self.conn.create_window(x, y, width, height, desired_depth)
internal = window.Internal(win, self.qtile, desired_depth)
internal.place(x, y, width, height, 0, None)
self.qtile.manage(internal)
return internal

The system tray window is not an internal window and its id is added to the map by the widget:

qtile.windows_map[win.wid] = self

I'm still not seeing why you think any of this is relevant to how the SysTray is rendered. Perhaps you overestimate what qtile.manage does.

def manage(self, win: base.WindowType) -> None:
if isinstance(win, base.Internal):
self.windows_map[win.wid] = win
return
if win.wid in self.windows_map:
return
hook.fire("client_new", win)
# Window may be defunct because
# it's been declared static in hook.
if win.defunct:
return
self.windows_map[win.wid] = win
if self.current_screen and not isinstance(win, base.Static):
# Window may have been bound to a group in the hook.
if not win.group and self.current_screen.group:
self.current_screen.group.add(win, focus=win.can_steal_focus)
self.core.update_client_list(self.windows_map)
hook.fire("client_managed", win)

Which bit of that do you think is relevant for the System Tray? NB, for Internal windows, the method returns after adding the ID to the map. Nothing else is done.

@krive001
Copy link
Contributor Author

krive001 commented Nov 15, 2021

It doesn't matter if it's the root child or the bar.
root->window-> bar-> window
root-> bar -> window

@elParaguayo
Copy link
Member

elParaguayo commented Nov 15, 2021

It doesn't matter if it's the root child or the bar manufacturer. root-> bar-> window root-> window

Right. I'm stopping here. We're getting nowhere and I've taken too much time trying to get the information I need.

I appreciate that you're trying to help fix an issue but you're just not giving me enough information. Maybe someone else in the team can understand what you're trying to say.

Thank you for your effort and I'm sorry I can't help more at this point.

@krive001
Copy link
Contributor Author

It turned out to me here that the applet running on the systray doesn’t even belong to the bar.

 root = self.qtile.core._root.wid
        for wid in self.icons:
            self.conn.conn.core.ReparentWindow(wid, root, 0, 0)
        self.conn.conn.flush()

        del self.qtile.windows_map[self.wid]
        self.conn.conn.core.DestroyWindow(self.wid)

@m-col
Copy link
Member

m-col commented Nov 15, 2021

Yes, there are multiple windows involved:

  • the bar (Internal)
  • the systray
  • many icons

@krive001
Copy link
Contributor Author

The systray applet is below the bar and not on the bar.

@m-col
Copy link
Member

m-col commented Nov 15, 2021

Could you post a screenshot?

@krive001
Copy link
Contributor Author

2021-11-15_23:15:46

@m-col
Copy link
Member

m-col commented Nov 15, 2021

Is this the systray?
image

@krive001
Copy link
Contributor Author

Yes.

@m-col
Copy link
Member

m-col commented Nov 15, 2021

Just so I'm understanding correctly - is the screenshot with your change, or without? What difference does the change make?

@krive001
Copy link
Contributor Author

This main ..
I didn't change anything.

@m-col
Copy link
Member

m-col commented Nov 15, 2021

What difference does the change make? The one from the PR

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.

None yet

3 participants