Reserve space for panels without _NET_WM_STRUT (herbstluftwm#1080)

If a panel window has the type _NET_WM_WINDOW_TYPE_DOCK but does not
define in _NET_WM_STRUT from which borders of the screen it takes space,
then automatically detect it based on the window's geometry.

This also adds tests for the new functionality and also for the old
panel detection based on _NET_WM_STRUT

Author: t-wissmann

NEWS

@@ -15,6 +15,8 @@ Current git version
   * New command 'mirror'
   * New command 'apply_tmp_rule'
   * The 'apply_rules' command now reports parse errors
+  * Reserve space for panels that do not set _NET_WM_STRUT e.g. conky windows
+    of type 'dock'.
   * Only build json object doc if WITH_DOCUMENTATION is activated
   * Bug fixes:
     - When hiding windows, correctly set their WM_STATE to IconicState (we set

src/panelmanager.cpp

@@ -14,7 +14,7 @@ class Panel {
     Window winid_;
     PanelManager& pm_;
     Rectangle size_;
-    vector<long> wmStrut_ = {0, 0, 0, 0};
+    vector<long> wmStrut_ = {};
     int operator[](PanelManager::WmStrut idx) const {
         size_t i = static_cast<size_t>(idx);
         if (i < wmStrut_.size()) {
@@ -52,7 +52,7 @@ void PanelManager::registerPanel(Window win)
 {
     Panel* p = new Panel(win, *this);
     panels_.insert(make_pair(win, p));
-    updateReservedSpace(p);
+    updateReservedSpace(p, xcon_.windowSize(win));
     panels_changed_.emit();
 }
 
@@ -76,7 +76,24 @@ void PanelManager::propertyChanged(Window win, Atom property)
     auto it = panels_.find(win);
     if (it != panels_.end()) {
         Panel* p = it->second;
-        if (updateReservedSpace(p)) {
+        if (updateReservedSpace(p, xcon_.windowSize(win))) {
+            panels_changed_.emit();
+        }
+    }
+}
+
+/**
+ * @brief the geometry of a window was changed, where window
+ * is possibly a panel window
+ * @param the window
+ * @param its new geometry
+ */
+void PanelManager::geometryChanged(Window win, Rectangle geometry)
+{
+    auto it = panels_.find(win);
+    if (it != panels_.end()) {
+        Panel* p = it->second;
+        if (updateReservedSpace(p, geometry)) {
             panels_changed_.emit();
         }
     }
@@ -87,10 +104,12 @@ void PanelManager::injectDependencies(Settings* settings)
     settings_ = settings;
 }
 
-//! read the reserved space from the panel window and return if there are changes
-bool PanelManager::updateReservedSpace(Panel* p)
+/**
+ * read the reserved space from the panel window and return if there are changes
+ * - size is the geometry of the panel
+ */
+bool PanelManager::updateReservedSpace(Panel* p, Rectangle size)
 {
-    Rectangle size = xcon_.windowSize(p->winid_);
     auto optionalWmStrut = xcon_.getWindowPropertyCardinal(p->winid_, atomWmStrut_);
     if (!optionalWmStrut) {
         optionalWmStrut= xcon_.getWindowPropertyCardinal(p->winid_, atomWmStrutPartial_);
@@ -120,10 +139,44 @@ PanelManager::ReservedSpace PanelManager::computeReservedSpace(Rectangle mon)
             // monitor does not intersect with panel at all
             continue;
         }
-        rs.left_ = (p[WmStrut::left] > 0) ? intersection.width : 0;
-        rs.right_ = (p[WmStrut::right] > 0) ? intersection.width : 0;
-        rs.top_ = (p[WmStrut::top] > 0) ? intersection.height : 0;
-        rs.bottom_ = (p[WmStrut::bottom] > 0) ? intersection.height : 0;
+        // we only reserve space for the panel if the panel defines
+        // wmStrut_ or if the aspect ratio clearly indicates whether the
+        // panel is horizontal or vertical
+        if (!p.wmStrut_.empty() || intersection.height > intersection.width) {
+            // vertical panels
+            if (intersection.x == mon.x) {
+                rs.left_ = intersection.width;
+            }
+            if (intersection.br().x == mon.br().x) {
+                rs.right_ = intersection.width;
+            }
+        }
+        if (!p.wmStrut_.empty() || intersection.height < intersection.width) {
+            // horizontal panels
+            if (intersection.y == mon.y) {
+                rs.top_ = intersection.height;
+            }
+            if (intersection.br().y == mon.br().y) {
+                rs.bottom_ = intersection.height;
+            }
+        }
+        if (!p.wmStrut_.empty()) {
+            // if the panel explicitly defines wmStrut, then
+            // we only consider the sides for the reserved space
+            // that are explicitly mentioned in wmStrut
+            if (p[WmStrut::left] == 0) {
+                rs.left_ = 0;
+            }
+            if (p[WmStrut::right] == 0) {
+                rs.right_ = 0;
+            }
+            if (p[WmStrut::top] == 0) {
+                rs.top_ = 0;
+            }
+            if (p[WmStrut::bottom] == 0) {
+                rs.bottom_ = 0;
+            }
+        }
         for (size_t i = 0; i < 4; i++) {
             rsTotal[i] = std::max(rsTotal[i], rs[i]);
         }

src/panelmanager.h

@@ -37,13 +37,14 @@ class PanelManager {
     void registerPanel(Window win);
     void unregisterPanel(Window win);
     void propertyChanged(Window win, Atom property);
+    void geometryChanged(Window win, Rectangle size);
     void injectDependencies(Settings* settings);
     ReservedSpace computeReservedSpace(Rectangle monitorDimension);
     Signal panels_changed_;
     void rootWindowChanged(int width, int height);
 private:
     friend Panel;
-    bool updateReservedSpace(Panel* p);
+    bool updateReservedSpace(Panel* p, Rectangle geometry);
 
     std::unordered_map<Window, Panel*> panels_;
     Atom atomWmStrut_;

src/x11-types.h

@@ -98,6 +98,7 @@ struct Rectangle {
 
     bool operator<(const Rectangle& other) const;
     bool operator==(const Rectangle& other) const;
+    bool operator!=(const Rectangle& other) const { return !(this->operator==(other));}
 
     operator bool() const;
 

src/xmainloop.cpp

@@ -338,6 +338,9 @@ void XMainLoop::configurenotify(XConfigureEvent* event) {
             std::ostringstream void_output;
             root_->monitors->detectMonitorsCommand(input, void_output);
         }
+    } else {
+        Rectangle geometry = { event->x, event->y, event->width, event->height };
+        root_->panels->geometryChanged(event->window, geometry);
     }
     // HSDebug("name is: ConfigureNotify\n");
 }

tests/conftest.py

@@ -641,6 +641,7 @@ def create_client(self, urgent=False, pid=None,
                           wm_class=None,
                           window_type=None,
                           transient_for=None,
+                          pre_map=lambda wh: None,  # called before the window is mapped
                           ):
             w = self.root.create_window(
                 geometry[0],
@@ -679,6 +680,7 @@ def create_client(self, urgent=False, pid=None,
                                   32,
                                   [pid])
 
+            pre_map(w)
             w.map()
             self.display.sync()
             if sync_hlwm:

tests/test_panels.py

@@ -0,0 +1,65 @@
+import pytest
+from Xlib import Xatom
+
+
+@pytest.mark.parametrize("which_pad, pad_size, geometry", [
+    ("pad_left", 10, (-1, 0, 11, 400)),
+    ("pad_right", 20, (800 - 20, 23, 20, 400)),
+    ("pad_up", 40, (-5, 0, 805, 40)),
+    ("pad_down", 15, (4, 600 - 15, 200, 40)),
+    # cases where no pad is applied because the width/height ratio is ambiguous:
+    ("pad_left", 0, (0, 0, 40, 40)),
+    ("pad_down", 0, (800 - 40, 600 - 40, 40, 40)),
+])
+def test_panel_based_on_intersection(hlwm, x11, which_pad, pad_size, geometry):
+    # monitor 0 is affected by the panel, but the other monitor is not
+    hlwm.call('add othertag')
+    hlwm.call('set_monitors 800x600+0+0 800x600+800+0')
+    winhandle, _ = x11.create_client(geometry=geometry,
+                                     window_type='_NET_WM_WINDOW_TYPE_DOCK')
+
+    assert int(hlwm.attr.monitors[0][which_pad]()) == pad_size
+    for p in ["pad_left", "pad_right", "pad_up", "pad_down"]:
+        assert hlwm.attr.monitors[1][p]() == '0', \
+            "monitor 1 must never be affected"
+        if p == which_pad:
+            continue
+        assert hlwm.attr.monitors[0][p]() == '0'
+
+
+@pytest.mark.parametrize("which_pad, pad_size, geometry, wm_strut", [
+    ("pad_left", 20, (-1, 0, 21, 4), [20, 0, 0, 0]),
+    ("pad_right", 30, (800 - 30, -2, 40, 11), [0, 20, 0, 0]),
+    ("pad_up", 40, (0, 0, 30, 40), [0, 0, 40, 0]),
+    ("pad_down", 15, (80, 600 - 15, 2, 15), [0, 0, 0, 15]),
+])
+@pytest.mark.parametrize("strut_before_map", [True, False])
+def test_panel_based_on_wmstrut(hlwm, x11, which_pad, pad_size, geometry, wm_strut, strut_before_map):
+    # monitor 0 is affected by the panel, but the other monitor is not
+    hlwm.call('add othertag')
+    hlwm.call('set_monitors 800x600+0+0 800x600+800+0')
+
+    def set_wm_strut(winhandle):
+        xproperty = x11.display.intern_atom('_NET_WM_STRUT')
+        winhandle.change_property(xproperty, Xatom.CARDINAL, 32, wm_strut)
+
+    def noop(winhandle):
+        pass
+
+    winhandle, _ = x11.create_client(geometry=geometry,
+                                     window_type='_NET_WM_WINDOW_TYPE_DOCK',
+                                     pre_map=set_wm_strut if strut_before_map else noop,
+                                     )
+    if not strut_before_map:
+        set_wm_strut(winhandle)
+        # sync hlwm and x11
+        x11.display.sync()
+        hlwm.call('true')
+
+    assert int(hlwm.attr.monitors[0][which_pad]()) == pad_size
+    for p in ["pad_left", "pad_right", "pad_up", "pad_down"]:
+        assert hlwm.attr.monitors[1][p]() == '0', \
+            "monitor 1 must never be affected"
+        if p == which_pad:
+            continue
+        assert hlwm.attr.monitors[0][p]() == '0'