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

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;
 };

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)
+    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,37 +111,51 @@ void floating_check_size(Con *floating_con) {
         }
     }
 
+    /* Unless user requests otherwise (-1), raise the width/height to
+     * reasonable minimum dimensions */
+    if (config.floating_minimum_height != -1) {
+        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 {
+            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) {
+        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 {
+            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), ensure width/height do not exceed
      * configured maxima or, if unconfigured, limit to combined width of all
      * outputs */
-    if (config.floating_minimum_height != -1) {
-        if (config.floating_minimum_height == 0)
-            floating_con->rect.height = max(floating_con->rect.height, floating_sane_min_height);
-        else
-            floating_con->rect.height = max(floating_con->rect.height, config.floating_minimum_height);
-    }
-    if (config.floating_minimum_width != -1) {
-        if (config.floating_minimum_width == 0)
-            floating_con->rect.width = max(floating_con->rect.width, floating_sane_min_width);
-        else
-            floating_con->rect.width = max(floating_con->rect.width, config.floating_minimum_width);
-    }
-
-    /* Unless user requests otherwise (-1), raise the width/height to
-     * reasonable minimum dimensions */
     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;
+    }
 }
 
 void floating_enable(Con *con, bool automatic) {
@@ -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);

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)
-            DLOG("resize increments changed\n");
         }
 
-    int base_width = 0, base_height = 0;
+        if (changed) {
+            DLOG("resize increments changed\n");
+        }
+    }
 
-    /* base_width/height are the desired size of the window.
+    bool has_base_size = false;
-       We check if either the program-specified size or the program-specified
+    int base_width = 0;
-       min-size is available */
+    int base_height = 0;
+
+    /* 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) {
+        has_base_size = true;
-        /* TODO: is this right? icccm says not */
+    }
+
+    /* 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 ||
+    // TODO XXX Should we only do this is the base size is > 0?
-        base_height != con->window->base_height) {
+    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? */
+    /* The ICCCM says to subtract the base size from the window size for aspect
-    double width = con->rect.width - base_width;
+     * ratio calculations. However, unlike determining the base size itself we
-    double height = con->rect.height - base_height;
+     * 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;
 }

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/…

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;

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 $min_width = 150;
-    my $max_width = 55;
+    my $max_width = 150;
-    my $min_height = 77;
+    my $min_height = 100;
-    my $max_height = 77;
+    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;