From a3013969976c0f2572e6137f8a6b0ed306287b5e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ingo=20B=C3=BCrk?= Date: Mon, 28 Nov 2016 22:09:39 +0100 Subject: [PATCH] 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 | 4 ++ src/floating.c | 86 +++++++++++++++++---------- src/handlers.c | 70 +++++++++++++++------- src/manage.c | 6 ++ testcases/t/141-resize.t | 4 +- testcases/t/221-floating-type-hints.t | 12 ++-- 6 files changed, 123 insertions(+), 59 deletions(-) diff --git a/include/data.h b/include/data.h index 410f2e0d..3d67e315 100644 --- 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 0a8b6957..f2994339 100644 --- 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 5e589e9c..9b248058 100644 --- 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 81ee16fd..86a361c3 100644 --- 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 44ad9bbf..64e33bc3 100644 --- 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 01c73a75..60590904 100644 --- 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; -- 2.39.5