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

Quicktile is not detecting IceWM taskbar #118

Open
PetteriAimonen opened this issue May 22, 2020 · 12 comments
Open

Quicktile is not detecting IceWM taskbar #118

PetteriAimonen opened this issue May 22, 2020 · 12 comments
Labels

Comments

@PetteriAimonen
Copy link

Currently quicktile places windows so that they overlap the IceWM taskbar.

The Taskbar does have _NET_WM_STRUT set as 0, 0, 0, 49. However, the taskbar window is not listed _NET_CLIENT_LIST because it is not a framed window (i.e. it is not movable). It is listed in xwininfo -tree -root but only several levels deep.

IceWM does set _WIN_WORKAREA and _NET_WORKAREA so that the task bar is excluded. Looking at the version history, it appears that _NET_WORKAREA used to be used but no longer is, apparently to better support setups with multiple differently sized monitors.

I don't really know how I should approach fixing this, whether to modify IceWM or to modify quicktile. Modifying IceWM would require a bit of a hack to handle task bar specially. I'm not sure if it would get accepted by upstream, as the specifications are not exactly clear on whether _NET_CLIENT_LIST should include such stationary windows or not.

Alternatively quicktile could be modified to either search all windows for _NET_WM_STRUT (sounds unnecessarily slow), or to take some kind of intersection of _NET_WORKAREA and individual monitor sizes. But I'm not sure if that might mess up multi-monitor support on some window managers.

I've also been considering just adding a configuration options to add a configurable amount of margin at top/left/right/bottom of each monitor. This could be used to work around all kinds of issues like this, even though it is not a very clean way to do it.

@ssokolow
Copy link
Owner

ssokolow commented May 22, 2020

Yeah. the spec defines both that and _NET_CLIENT_LIST_STACKING and says "These arrays contain all X Windows managed by the Window Manager. " so other WMs like Openbox and Kwin interpreted that as "include the panels in _NET_CLIENT_LIST".

QuickTile was specifically written to be as "works everywhere" as feasibly possible (as opposed to things like the Compiz Grid plugin) so I'd like to try to come up with a solution that works with stock IceWM. What distro and version are you running? I'll try to grab a VM and experiment with it.

As for _NET_WORKAREA, it's too restricted in its utility to be an acceptable solution because it's the largest rectangle that fits within the usable region, not the smallest rectangle that encompasses it. That's why I got rid of it.

(ie. If you have a 1280x1024 monitor next to a 1920x1080 monitor, _NET_WORKAREA will be 3200x1024, not 3200x1080, and you'll have empty space at the top/bottom of the 1080p monitor. _NET_WORKAREA is no help in distinguishing panel reservation from the difference between adjacent monitor heights.)

As for manual overrides, I actually considered doing something like that for #117 (though it'd have to wait for the planned redesign of the config file to support hierarchical data) but it's looking like GNOME Shell makes up for not setting any _NET_WM_STRUT or _NET_WM_STRUT_PARTIAL information by setting a non-standard derivative of _NET_WORKAREA that stores per-monitor workarea information.

@PetteriAimonen
Copy link
Author

Ah, thanks for the clarification on _NET_WORKAREA.

I'm using Ubuntu 20.04, if you install the icewm package and select icewm session from the lower right corner of login screen, I think the issue should be reproducible.

I think I'll ask IceWM maintainers whether a patch adding the panel to _NET_CLIENT_LIST, _WIN_CLIENT_LIST and _NET_CLIENT_LIST_STACKING would be accepted: ice-wm/icewm#18

@PetteriAimonen
Copy link
Author

I made a quick test and this brute-force approach does work with unmodified IceWM:

diff --git a/quicktile/wm.py b/quicktile/wm.py
index 020539b..15ec403 100644
--- a/quicktile/wm.py
+++ b/quicktile/wm.py
@@ -99,6 +99,15 @@ class WindowManager(object):
         # TODO: Hook monitor-added and monitor-removed and regenerate this
         # TODO: Hook changes to strut reservations and regenerate this
 
+    def iterate_all_windows(self, start = None):
+        """Iterate through the whole window hierarchy."""
+        if start is None:
+            start = self.x_root
+
+        yield start
+        for window in start.query_tree().children:
+            yield from self.iterate_all_windows(window)
+
     def update_geometry_cache(self):
         """Update the internal cache of monitor & panel shapes by querying
         them from the desktop and processing them into a
@@ -139,9 +148,7 @@ class WindowManager(object):
 
         # Gather all struts
         struts = []
-        for wid in [self.x_root.id] + list(self.get_property(
-                self.x_root.id, '_NET_CLIENT_LIST', Xatom.WINDOW, [])):
-            win = self.x_display.create_resource_object('window', wid)
+        for win in self.iterate_all_windows(self.x_root):
             result = self.get_property(
                 win, '_NET_WM_STRUT_PARTIAL', Xatom.CARDINAL)
             if result:

If I understand correctly, the code only runs when monitor layout changes, so perhaps the performance impact is not important?

@PetteriAimonen
Copy link
Author

Hmm no, based on the debug output it does run on every command. It does not cause a detectable delay with a small amount of windows, but potentially could do so with a huge amount.

@ssokolow
Copy link
Owner

ssokolow commented May 22, 2020

Yeah. I haven't had time to look up how to efficiently and reliably set a watch on the creation, modification, or removal of _NET_WM_STRUT_PARTIAL properties across all windows on the system and I've had reports that some people's desktops fire up QuickTile before the panels get created if asked to launch it on startup.

Forcing a refresh on each command run was a return to the approach used in the GTK+ 2.x codebase to work around that regression.

@PetteriAimonen
Copy link
Author

One possible approach could be to check it on first command run, and after that only if _NET_CLIENT_LIST has changed.

@ssokolow
Copy link
Owner

Given how often I create and remove windows, that still sounds risky. Possibly something to be gated behind a config key.

@ssokolow
Copy link
Owner

ssokolow commented May 23, 2020

I'll still need to spin up a test VM, but maybe, if I can't think of something better, it could specifically detect IceWM and then enable a more thorough complement to _NET_CLIENT_LIST which uses the xlsclients algorithm.

(Call window.query_tree().children at each level of the X11 window heirarchy and stop descending at windows with WM_STATE or another indicative property set.)

@PetteriAimonen
Copy link
Author

If one does IceWM-specific detection, it can use the window names to find the taskbar, like in this code hinted by the IceWM maintainers: https://github.com/ice-wm/icewm/blob/master/src/icesh.cc#L901

@ssokolow
Copy link
Owner

I prefer desktop-specific detection to not be treated as something that's intended to be anything more than an irritating hack that I'm going to want to get rid of every time I work on the code.

This'd be more a case of "Implement a ThoroughPanelDetection config boolean and force it on if IceWM is detected." (Basically, the GTK 3.x-era analogue to how GTK+ 2.x-era QuickTile allowed users to opt into using _NET_WORKAREA.)

@samurailostinjapan
Copy link

Any update on this issue? - experiencing the same on our Arcolinux icewm build. As the built-in icewm tiling function is really not usable - would like to add quicktile to our build.

@ssokolow
Copy link
Owner

Unfortunately not. The past year has been a real mess for me for reasons beyond COVID and I'm currently climbing my way out of the pit of backlogged work that left me in.

No promises, but I hope I'll be able to do at least something for QuickTile in the next few months.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants