Respect minimum size hints for floating windows. (#2508)

This commit introduces proper support for the minimum size on floating
windows by ensuring that it is respected during mapping, later changes as
well as resizes.

Furthermore, this commit fixes minor issues with how the hints are handled
during calculations.

fixes #2436

diff --git a/include/data.h b/include/data.h
index 410f2e0deba01202ad17292b5e1df211ae208777..3d67e315733893a85be3ef1124f6d145eb76cea8 100644 (file)
--- a/include/data.h
+++ b/include/data.h
@@ -450,6 +450,10 @@ struct Window {
     int width_increment;
     int height_increment;
 
+    /* Minimum size specified for the window. */
+    int min_width;
+    int min_height;
+
     /* aspect ratio from WM_NORMAL_HINTS (MPlayer uses this for example) */
     double aspect_ratio;
 };

diff --git a/src/floating.c b/src/floating.c
index 0a8b6957c32635a5e0c7f7205d7db4584df28dc8..f29943393c208c60913449fedbfe04036ba5fb2d 100644 (file)
--- a/src/floating.c
+++ b/src/floating.c
@@ -72,18 +72,29 @@ void floating_check_size(Con *floating_con) {
     Rect floating_sane_max_dimensions;
     Con *focused_con = con_descend_focused(floating_con);
 
-    /* obey size increments */
-    if (focused_con->window != NULL && (focused_con->window->height_increment || focused_con->window->width_increment)) {
-        Rect border_rect = con_border_style_rect(focused_con);
-
-        /* We have to do the opposite calculations that render_con() do
-         * to get the exact size we want. */
-        border_rect.width = -border_rect.width;
-        border_rect.width += 2 * focused_con->border_width;
-        border_rect.height = -border_rect.height;
-        border_rect.height += 2 * focused_con->border_width;
-        if (con_border_style(focused_con) == BS_NORMAL)
-            border_rect.height += render_deco_height();
+    Rect border_rect = con_border_style_rect(focused_con);
+    /* We have to do the opposite calculations that render_con() do
+     * to get the exact size we want. */
+    border_rect.width = -border_rect.width;
+    border_rect.width += 2 * focused_con->border_width;
+    border_rect.height = -border_rect.height;
+    border_rect.height += 2 * focused_con->border_width;
+    if (con_border_style(focused_con) == BS_NORMAL) {
+        border_rect.height += render_deco_height();
+    }
+
+    if (focused_con->window != NULL) {
+        if (focused_con->window->min_width) {
+            floating_con->rect.width -= border_rect.width;
+            floating_con->rect.width = max(floating_con->rect.width, focused_con->window->min_width);
+            floating_con->rect.width += border_rect.width;
+        }
+
+        if (focused_con->window->min_height) {
+            floating_con->rect.height -= border_rect.height;
+            floating_con->rect.height = max(floating_con->rect.height, focused_con->window->min_height);
+            floating_con->rect.height += border_rect.height;
+        }
 
         if (focused_con->window->height_increment &&
             floating_con->rect.height >= focused_con->window->base_height + border_rect.height) {
@@ -100,36 +111,50 @@ void floating_check_size(Con *floating_con) {
         }
     }
 
-    /* Unless user requests otherwise (-1), ensure width/height do not exceed
-     * configured maxima or, if unconfigured, limit to combined width of all
-     * outputs */
+    /* Unless user requests otherwise (-1), raise the width/height to
+     * reasonable minimum dimensions */
     if (config.floating_minimum_height != -1) {
-        if (config.floating_minimum_height == 0)
+        floating_con->rect.height -= border_rect.height;
+        if (config.floating_minimum_height == 0) {
             floating_con->rect.height = max(floating_con->rect.height, floating_sane_min_height);
-        else
+        } else {
             floating_con->rect.height = max(floating_con->rect.height, config.floating_minimum_height);
+        }
+        floating_con->rect.height += border_rect.height;
     }
+
     if (config.floating_minimum_width != -1) {
-        if (config.floating_minimum_width == 0)
+        floating_con->rect.width -= border_rect.width;
+        if (config.floating_minimum_width == 0) {
             floating_con->rect.width = max(floating_con->rect.width, floating_sane_min_width);
-        else
+        } else {
             floating_con->rect.width = max(floating_con->rect.width, config.floating_minimum_width);
+        }
+        floating_con->rect.width += border_rect.width;
     }
 
-    /* Unless user requests otherwise (-1), raise the width/height to
-     * reasonable minimum dimensions */
+    /* Unless user requests otherwise (-1), ensure width/height do not exceed
+     * configured maxima or, if unconfigured, limit to combined width of all
+     * outputs */
     floating_sane_max_dimensions = total_outputs_dimensions();
     if (config.floating_maximum_height != -1) {
-        if (config.floating_maximum_height == 0)
+        floating_con->rect.height -= border_rect.height;
+        if (config.floating_maximum_height == 0) {
             floating_con->rect.height = min(floating_con->rect.height, floating_sane_max_dimensions.height);
-        else
+        } else {
             floating_con->rect.height = min(floating_con->rect.height, config.floating_maximum_height);
+        }
+        floating_con->rect.height += border_rect.height;
     }
+
     if (config.floating_maximum_width != -1) {
-        if (config.floating_maximum_width == 0)
+        floating_con->rect.width -= border_rect.width;
+        if (config.floating_maximum_width == 0) {
             floating_con->rect.width = min(floating_con->rect.width, floating_sane_max_dimensions.width);
-        else
+        } else {
             floating_con->rect.width = min(floating_con->rect.width, config.floating_maximum_width);
+        }
+        floating_con->rect.width += border_rect.width;
     }
 }
 
@@ -208,7 +233,8 @@ void floating_enable(Con *con, bool automatic) {
         }
     }
 
-    floating_check_size(nc);
+    TAILQ_INSERT_TAIL(&(nc->nodes_head), con, nodes);
+    TAILQ_INSERT_TAIL(&(nc->focus_head), con, focused);
 
     /* 3: attach the child to the new parent container. We need to do this
      * because con_border_style_rect() needs to access con->parent. */
@@ -227,13 +253,16 @@ void floating_enable(Con *con, bool automatic) {
     nc->rect.width -= border_style_rect.width;
 
     /* Add some more pixels for the title bar */
-    if (con_border_style(con) == BS_NORMAL)
+    if (con_border_style(con) == BS_NORMAL) {
         nc->rect.height += deco_height;
+    }
 
     /* Honor the X11 border */
     nc->rect.height += con->border_width * 2;
     nc->rect.width += con->border_width * 2;
 
+    floating_check_size(nc);
+
     /* Some clients (like GIMP’s color picker window) get mapped
      * to (0, 0), so we push them to a reasonable position
      * (centered over their leader) */
@@ -280,9 +309,6 @@ void floating_enable(Con *con, bool automatic) {
 
     DLOG("Corrected y = %d (deco_height = %d)\n", nc->rect.y, deco_height);
 
-    TAILQ_INSERT_TAIL(&(nc->nodes_head), con, nodes);
-    TAILQ_INSERT_TAIL(&(nc->focus_head), con, focused);
-
     /* render the cons to get initial window_rect correct */
     render_con(nc, false);
     render_con(con, false);

diff --git a/src/handlers.c b/src/handlers.c
index 5e589e9c91b51697d8e8c347dbebf9daf6c54217..9b248058e2e75140ce531b6e17f8d3643c5f9fe8 100644 (file)
--- a/src/handlers.c
+++ b/src/handlers.c
@@ -929,54 +929,72 @@ static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t stat
 
     xcb_size_hints_t size_hints;
 
-    //CLIENT_LOG(client);
-
     /* If the hints were already in this event, use them, if not, request them */
-    if (reply != NULL)
+    if (reply != NULL) {
         xcb_icccm_get_wm_size_hints_from_reply(&size_hints, reply);
-    else
+    } else {
         xcb_icccm_get_wm_normal_hints_reply(conn, xcb_icccm_get_wm_normal_hints_unchecked(conn, con->window->id), &size_hints, NULL);
+    }
+
+    int win_width = con->window_rect.width;
+    int win_height = con->window_rect.height;
 
     if ((size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MIN_SIZE)) {
-        // TODO: Minimum size is not yet implemented
         DLOG("Minimum size: %d (width) x %d (height)\n", size_hints.min_width, size_hints.min_height);
+
+        con->window->min_width = size_hints.min_width;
+        con->window->min_height = size_hints.min_height;
+    }
+
+    if (con_is_floating(con)) {
+        win_width = MAX(win_width, con->window->min_width);
+        win_height = MAX(win_height, con->window->min_height);
     }
 
     bool changed = false;
     if ((size_hints.flags & XCB_ICCCM_SIZE_HINT_P_RESIZE_INC)) {
-        if (size_hints.width_inc > 0 && size_hints.width_inc < 0xFFFF)
+        if (size_hints.width_inc > 0 && size_hints.width_inc < 0xFFFF) {
             if (con->window->width_increment != size_hints.width_inc) {
                 con->window->width_increment = size_hints.width_inc;
                 changed = true;
             }
-        if (size_hints.height_inc > 0 && size_hints.height_inc < 0xFFFF)
+        }
+
+        if (size_hints.height_inc > 0 && size_hints.height_inc < 0xFFFF) {
             if (con->window->height_increment != size_hints.height_inc) {
                 con->window->height_increment = size_hints.height_inc;
                 changed = true;
             }
+        }
 
-        if (changed)
+        if (changed) {
             DLOG("resize increments changed\n");
+        }
     }
 
-    int base_width = 0, base_height = 0;
+    bool has_base_size = false;
+    int base_width = 0;
+    int base_height = 0;
 
-    /* base_width/height are the desired size of the window.
-       We check if either the program-specified size or the program-specified
-       min-size is available */
+    /* The base width / height is the desired size of the window. */
     if (size_hints.flags & XCB_ICCCM_SIZE_HINT_BASE_SIZE) {
         base_width = size_hints.base_width;
         base_height = size_hints.base_height;
-    } else if (size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MIN_SIZE) {
-        /* TODO: is this right? icccm says not */
+        has_base_size = true;
+    }
+
+    /* If the window didn't specify a base size, the ICCCM tells us to fall
+     * back to the minimum size instead, if available. */
+    if (!has_base_size && size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MIN_SIZE) {
         base_width = size_hints.min_width;
         base_height = size_hints.min_height;
     }
 
-    if (base_width != con->window->base_width ||
-        base_height != con->window->base_height) {
+    // TODO XXX Should we only do this is the base size is > 0?
+    if (base_width != con->window->base_width || base_height != con->window->base_height) {
         con->window->base_width = base_width;
         con->window->base_height = base_height;
+
         DLOG("client's base_height changed to %d\n", base_height);
         DLOG("client's base_width changed to %d\n", base_width);
         changed = true;
@@ -989,9 +1007,13 @@ static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t stat
         goto render_and_return;
     }
 
-    /* XXX: do we really use rect here, not window_rect? */
-    double width = con->rect.width - base_width;
-    double height = con->rect.height - base_height;
+    /* The ICCCM says to subtract the base size from the window size for aspect
+     * ratio calculations. However, unlike determining the base size itself we
+     * must not fall back to using the minimum size in this case according to
+     * the ICCCM. */
+    double width = win_width - base_width * has_base_size;
+    double height = win_height - base_height * has_base_size;
+
     /* Convert numerator/denominator to a double */
     double min_aspect = (double)size_hints.min_aspect_num / size_hints.min_aspect_den;
     double max_aspect = (double)size_hints.max_aspect_num / size_hints.min_aspect_den;
@@ -1000,8 +1022,9 @@ static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t stat
     DLOG("width = %f, height = %f\n", width, height);
 
     /* Sanity checks, this is user-input, in a way */
-    if (max_aspect <= 0 || min_aspect <= 0 || height == 0 || (width / height) <= 0)
+    if (max_aspect <= 0 || min_aspect <= 0 || height == 0 || (width / height) <= 0) {
         goto render_and_return;
+    }
 
     /* Check if we need to set proportional_* variables using the correct ratio */
     double aspect_ratio = 0.0;
@@ -1009,8 +1032,9 @@ static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t stat
         aspect_ratio = min_aspect;
     } else if ((width / height) > max_aspect) {
         aspect_ratio = max_aspect;
-    } else
+    } else {
         goto render_and_return;
+    }
 
     if (fabs(con->window->aspect_ratio - aspect_ratio) > DBL_EPSILON) {
         con->window->aspect_ratio = aspect_ratio;
@@ -1018,8 +1042,10 @@ static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t stat
     }
 
 render_and_return:
-    if (changed)
+    if (changed) {
         tree_render();
+    }
+
     FREE(reply);
     return true;
 }

diff --git a/src/manage.c b/src/manage.c
index 81ee16fdcc7fc9f0ef77f00084659f7b1b080168..86a361c376a856a32d6a1d90c51178dbec163e12 100644 (file)
--- a/src/manage.c
+++ b/src/manage.c
@@ -491,6 +491,12 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki
         geom->height = wm_size_hints.height;
     }
 
+    if (wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MIN_SIZE) {
+        DLOG("Window specifies minimum size %d x %d\n", wm_size_hints.min_width, wm_size_hints.min_height);
+        nc->window->min_width = wm_size_hints.min_width;
+        nc->window->min_height = wm_size_hints.min_height;
+    }
+
     /* Store the requested geometry. The width/height gets raised to at least
      * 75x50 when entering floating mode, which is the minimum size for a
      * window to be useful (smaller windows are usually overlays/toolbars/…

diff --git a/testcases/t/141-resize.t b/testcases/t/141-resize.t
index 44ad9bbf97c2dfddb49e4fc2e3929098140d9aaa..64e33bc3d74659c15533c9276c2579359208bd7d 100644 (file)
--- a/testcases/t/141-resize.t
+++ b/testcases/t/141-resize.t
@@ -298,11 +298,11 @@ sub get_floating_rect {
 # focus is on the right window, so we resize the left one using criteria
 my $leftold = get_floating_rect($left->id);
 my $rightold = get_floating_rect($right->id);
-cmd '[id="' . $left->id . '"] resize shrink height 10px or 10ppt';
+cmd '[id="' . $left->id . '"] resize grow height 10px or 10ppt';
 
 my $leftnew = get_floating_rect($left->id);
 my $rightnew = get_floating_rect($right->id);
 is($rightnew->{height}, $rightold->{height}, 'height of right container unchanged');
-is($leftnew->{height}, $leftold->{height} - 10, 'height of left container changed');
+is($leftnew->{height}, $leftold->{height} + 10, 'height of left container changed');
 
 done_testing;

diff --git a/testcases/t/221-floating-type-hints.t b/testcases/t/221-floating-type-hints.t
index 01c73a75505a10eb81dd8dfc36376a0e9e48c096..605909042e59e76095999778ba43b6917b8986d1 100644 (file)
--- a/testcases/t/221-floating-type-hints.t
+++ b/testcases/t/221-floating-type-hints.t
@@ -62,10 +62,10 @@ sub open_with_fixed_size {
 
     my $flags = $XCB_ICCCM_SIZE_HINT_P_MIN_SIZE | $XCB_ICCCM_SIZE_HINT_P_MAX_SIZE;
 
-    my $min_width = 55;
-    my $max_width = 55;
-    my $min_height = 77;
-    my $max_height = 77;
+    my $min_width = 150;
+    my $max_width = 150;
+    my $min_height = 100;
+    my $max_height = 100;
 
     my $pad = 0x00;
 
@@ -82,7 +82,7 @@ sub open_with_fixed_size {
                 $atomtype->id,
                 32,
                 12,
-                pack('C5N8', $flags, $pad, $pad, $pad, $pad, 0, 0, 0, $min_width, $min_height, $max_width, $max_height),
+                pack('C5N7', $flags, $pad, $pad, $pad, $pad, 0, 0, 0, $min_width, $min_height, $max_width, $max_height),
             );
         },
     );
@@ -114,6 +114,8 @@ $window->unmap;
 
 $window = open_with_fixed_size;
 is(get_ws($ws)->{floating_nodes}[0]->{nodes}[0]->{window}, $window->id, 'Fixed size window opened floating');
+is(get_ws($ws)->{floating_nodes}[0]->{nodes}[0]->{window_rect}->{width}, 150, 'Fixed size window opened with minimum width');
+is(get_ws($ws)->{floating_nodes}[0]->{nodes}[0]->{window_rect}->{height}, 100, 'Fixed size window opened with minimum height');
 $window->unmap;
 
 done_testing;