From 01960f956f498ee81124e80d96bdc1e521518583 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Mon, 1 Oct 2018 15:42:53 +0300 Subject: [PATCH 1/5] floating_check_size: Use window variable --- src/floating.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/floating.c b/src/floating.c index a99d0970..c9600eac 100644 --- a/src/floating.c +++ b/src/floating.c @@ -83,43 +83,44 @@ void floating_check_size(Con *floating_con) { border_rect.height += render_deco_height(); } - if (focused_con->window != NULL) { - if (focused_con->window->min_width) { + i3Window *window = focused_con->window; + if (window != NULL) { + if (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 = max(floating_con->rect.width, window->min_width); floating_con->rect.width += border_rect.width; } - if (focused_con->window->min_height) { + if (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 = max(floating_con->rect.height, window->min_height); floating_con->rect.height += border_rect.height; } - if (focused_con->window->max_width) { + if (window->max_width) { floating_con->rect.width -= border_rect.width; - floating_con->rect.width = min(floating_con->rect.width, focused_con->window->max_width); + floating_con->rect.width = min(floating_con->rect.width, window->max_width); floating_con->rect.width += border_rect.width; } - if (focused_con->window->max_height) { + if (window->max_height) { floating_con->rect.height -= border_rect.height; - floating_con->rect.height = min(floating_con->rect.height, focused_con->window->max_height); + floating_con->rect.height = min(floating_con->rect.height, window->max_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) { - floating_con->rect.height -= focused_con->window->base_height + border_rect.height; - floating_con->rect.height -= floating_con->rect.height % focused_con->window->height_increment; - floating_con->rect.height += focused_con->window->base_height + border_rect.height; + if (window->height_increment && + floating_con->rect.height >= window->base_height + border_rect.height) { + floating_con->rect.height -= window->base_height + border_rect.height; + floating_con->rect.height -= floating_con->rect.height % window->height_increment; + floating_con->rect.height += window->base_height + border_rect.height; } - if (focused_con->window->width_increment && - floating_con->rect.width >= focused_con->window->base_width + border_rect.width) { - floating_con->rect.width -= focused_con->window->base_width + border_rect.width; - floating_con->rect.width -= floating_con->rect.width % focused_con->window->width_increment; - floating_con->rect.width += focused_con->window->base_width + border_rect.width; + if (window->width_increment && + floating_con->rect.width >= window->base_width + border_rect.width) { + floating_con->rect.width -= window->base_width + border_rect.width; + floating_con->rect.width -= floating_con->rect.width % window->width_increment; + floating_con->rect.width += window->base_width + border_rect.width; } } From f397698d43e3dd4f4625e1fed1b1d0470d0acdf6 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Tue, 2 Oct 2018 01:52:31 +0300 Subject: [PATCH 2/5] floating_resize: Use uint32_t --- include/floating.h | 2 +- src/floating.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/floating.h b/include/floating.h index 4382437b..368183bc 100644 --- a/include/floating.h +++ b/include/floating.h @@ -152,7 +152,7 @@ bool floating_reposition(Con *con, Rect newrect); * window's size hints. * */ -void floating_resize(Con *floating_con, int x, int y); +void floating_resize(Con *floating_con, uint32_t x, uint32_t y); /** * Fixes the coordinates of the floating window whenever the window gets diff --git a/src/floating.c b/src/floating.c index c9600eac..0c7b43b2 100644 --- a/src/floating.c +++ b/src/floating.c @@ -921,7 +921,7 @@ bool floating_reposition(Con *con, Rect newrect) { * window's size hints. * */ -void floating_resize(Con *floating_con, int x, int y) { +void floating_resize(Con *floating_con, uint32_t x, uint32_t y) { DLOG("floating resize to %dx%d px\n", x, y); Rect *rect = &floating_con->rect; Con *focused_con = con_descend_focused(floating_con); From 29f2510fa9484a5fa235de9a1c5c937292151888 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Mon, 1 Oct 2018 19:47:33 +0300 Subject: [PATCH 3/5] Fix aspect ratio bugs - ICCCM says: > If a base size is not provided, the minimum size is to be used in its place and vice versa. i3 didn't obey the "vice versa" part. Min size and base size are both saved without replacements in window_update_normal_hints, floating_check_size makes the needed replacements if either was not provided. - Aspect ratio is now saved correctly in manage_window because window_update_normal_hints is called. - i3 didn't save the aspect ratio if the window conformed the given aspect ratio range when handle_normal_hints was called. If the window was resized to a size outside of the given bounds, i3 didn't correct it. - Aspect ratio now affects only tiling windows, like the rest of the normal size hints - The aspect ratio calculation is now done without a loop A real life example of how these changes affect the workflow: An mpv window, when playing a video, sets its min == max aspect ratio during mapping. i3 ignored these hints. When resized, the window's aspect ratio was not preserved. With this commit, resizing floating mpv windows will always preserve the aspect ratio. --- include/data.h | 3 +- include/floating.h | 13 ++-- include/window.h | 6 ++ src/commands.c | 2 +- src/con.c | 1 - src/floating.c | 85 ++++++++++++++++++----- src/handlers.c | 130 ++--------------------------------- src/load_layout.c | 2 +- src/manage.c | 34 ++------- src/render.c | 29 -------- src/scratchpad.c | 2 +- src/window.c | 121 ++++++++++++++++++++++++++++++++ testcases/t/133-size-hints.t | 104 +++++++++++++++++++++++----- 13 files changed, 304 insertions(+), 228 deletions(-) diff --git a/include/data.h b/include/data.h index f55e003d..d2b501b9 100644 --- a/include/data.h +++ b/include/data.h @@ -482,7 +482,8 @@ struct Window { int max_height; /* aspect ratio from WM_NORMAL_HINTS (MPlayer uses this for example) */ - double aspect_ratio; + double min_aspect_ratio; + double max_aspect_ratio; }; /** diff --git a/include/floating.h b/include/floating.h index 368183bc..a7813099 100644 --- a/include/floating.h +++ b/include/floating.h @@ -94,12 +94,17 @@ void floating_drag_window(Con *con, const xcb_button_press_event_t *event); void floating_resize_window(Con *con, const bool proportional, const xcb_button_press_event_t *event); /** - * Called when a floating window is created or resized. - * This function resizes the window if its size is higher or lower than the - * configured maximum/minimum size, respectively. + * Called when a floating window is created or resized. This function resizes + * the window if its size is higher or lower than the configured maximum/minimum + * size, respectively or when adjustments are needed to conform to the + * configured size increments or aspect ratio limits. + * + * When prefer_height is true and the window needs to be resized because of the + * configured aspect ratio, the width is adjusted first, preserving the previous + * height. * */ -void floating_check_size(Con *floating_con); +void floating_check_size(Con *floating_con, bool prefer_height); /** * This is the return value of a drag operation like drag_pointer. diff --git a/include/window.h b/include/window.h index 77e3f48f..b03f9c14 100644 --- a/include/window.h +++ b/include/window.h @@ -70,6 +70,12 @@ void window_update_role(i3Window *win, xcb_get_property_reply_t *prop, bool befo */ void window_update_type(i3Window *window, xcb_get_property_reply_t *reply); +/** + * Updates the WM_NORMAL_HINTS + * + */ +bool window_update_normal_hints(i3Window *win, xcb_get_property_reply_t *reply, xcb_get_geometry_reply_t *geom); + /** * Updates the WM_HINTS (we only care about the input focus handling part). * diff --git a/src/commands.c b/src/commands.c index 57dc58b4..ee402846 100644 --- a/src/commands.c +++ b/src/commands.c @@ -467,7 +467,7 @@ static void cmd_resize_floating(I3_CMD, const char *way, const char *direction_s } else { floating_con->rect.width += px; } - floating_check_size(floating_con); + floating_check_size(floating_con, orientation == VERT); /* Did we actually resize anything or did the size constraints prevent us? * If we could not resize, exit now to not move the window. */ diff --git a/src/con.c b/src/con.c index 51c2c48b..764419dc 100644 --- a/src/con.c +++ b/src/con.c @@ -46,7 +46,6 @@ Con *con_new_skeleton(Con *parent, i3Window *window) { new->current_border_width = -1; if (window) { new->depth = window->depth; - new->window->aspect_ratio = 0.0; } else { new->depth = root_depth; } diff --git a/src/floating.c b/src/floating.c index 0c7b43b2..4f2760a7 100644 --- a/src/floating.c +++ b/src/floating.c @@ -60,12 +60,17 @@ static void floating_set_hint_atom(Con *con, bool floating) { } /* - * Called when a floating window is created or resized. - * This function resizes the window if its size is higher or lower than the - * configured maximum/minimum size, respectively. + * Called when a floating window is created or resized. This function resizes + * the window if its size is higher or lower than the configured maximum/minimum + * size, respectively or when adjustments are needed to conform to the + * configured size increments or aspect ratio limits. + * + * When prefer_height is true and the window needs to be resized because of the + * configured aspect ratio, the width is adjusted first, preserving the previous + * height. * */ -void floating_check_size(Con *floating_con) { +void floating_check_size(Con *floating_con, bool prefer_height) { /* Define reasonable minimal and maximal sizes for floating windows */ const int floating_sane_min_height = 50; const int floating_sane_min_width = 75; @@ -85,15 +90,22 @@ void floating_check_size(Con *floating_con) { i3Window *window = focused_con->window; if (window != NULL) { - if (window->min_width) { + /* ICCCM says: If a base size is not provided, the minimum size is to be used in its place + * and vice versa. */ + int min_width = (window->min_width ? window->min_width : window->base_width); + int min_height = (window->min_height ? window->min_height : window->base_height); + int base_width = (window->base_width ? window->base_width : window->min_width); + int base_height = (window->base_height ? window->base_height : window->min_height); + + if (min_width) { floating_con->rect.width -= border_rect.width; - floating_con->rect.width = max(floating_con->rect.width, window->min_width); + floating_con->rect.width = max(floating_con->rect.width, min_width); floating_con->rect.width += border_rect.width; } - if (window->min_height) { + if (min_height) { floating_con->rect.height -= border_rect.height; - floating_con->rect.height = max(floating_con->rect.height, window->min_height); + floating_con->rect.height = max(floating_con->rect.height, min_height); floating_con->rect.height += border_rect.height; } @@ -109,18 +121,56 @@ void floating_check_size(Con *floating_con) { floating_con->rect.height += border_rect.height; } + /* Obey the aspect ratio, if any, unless we are in fullscreen mode. + * + * The spec isn’t explicit on whether the aspect ratio hints should be + * respected during fullscreen mode. Other WMs such as Openbox don’t do + * that, and this post suggests that this is the correct way to do it: + * https://mail.gnome.org/archives/wm-spec-list/2003-May/msg00007.html + * + * Ignoring aspect ratio during fullscreen was necessary to fix MPlayer + * subtitle rendering, see https://bugs.i3wm.org/594 */ + const double min_ar = window->min_aspect_ratio; + const double max_ar = window->max_aspect_ratio; + if (floating_con->fullscreen_mode == CF_NONE && (min_ar > 0 || max_ar > 0)) { + /* 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 = floating_con->rect.width - window->base_width - border_rect.width; + double height = floating_con->rect.height - window->base_height - border_rect.height; + const double ar = (double)width / (double)height; + double new_ar = -1; + if (min_ar > 0 && ar < min_ar) { + new_ar = min_ar; + } else if (max_ar > 0 && ar > max_ar) { + new_ar = max_ar; + } + if (new_ar > 0) { + if (prefer_height) { + width = round(height * new_ar); + height = round(width / new_ar); + } else { + height = round(width / new_ar); + width = round(height * new_ar); + } + floating_con->rect.width = width + window->base_width + border_rect.width; + floating_con->rect.height = height + window->base_height + border_rect.height; + } + } + if (window->height_increment && - floating_con->rect.height >= window->base_height + border_rect.height) { - floating_con->rect.height -= window->base_height + border_rect.height; + floating_con->rect.height >= base_height + border_rect.height) { + floating_con->rect.height -= base_height + border_rect.height; floating_con->rect.height -= floating_con->rect.height % window->height_increment; - floating_con->rect.height += window->base_height + border_rect.height; + floating_con->rect.height += base_height + border_rect.height; } if (window->width_increment && - floating_con->rect.width >= window->base_width + border_rect.width) { - floating_con->rect.width -= window->base_width + border_rect.width; + floating_con->rect.width >= base_width + border_rect.width) { + floating_con->rect.width -= base_width + border_rect.width; floating_con->rect.width -= floating_con->rect.width % window->width_increment; - floating_con->rect.width += window->base_width + border_rect.width; + floating_con->rect.width += base_width + border_rect.width; } } @@ -314,7 +364,7 @@ void floating_enable(Con *con, bool automatic) { nc->rect.height += con->border_width * 2; nc->rect.width += con->border_width * 2; - floating_check_size(nc); + floating_check_size(nc, false); /* Some clients (like GIMP’s color picker window) get mapped * to (0, 0), so we push them to a reasonable position @@ -612,7 +662,7 @@ DRAGGING_CB(resize_window_callback) { con->rect = (Rect){dest_x, dest_y, dest_width, dest_height}; /* Obey window size */ - floating_check_size(con); + floating_check_size(con, false); /* If not the lower right corner is grabbed, we must also reposition * the client by exactly the amount we resized it */ @@ -931,6 +981,7 @@ void floating_resize(Con *floating_con, uint32_t x, uint32_t y) { } int wi = focused_con->window->width_increment; int hi = focused_con->window->height_increment; + bool prefer_height = (rect->width == x); rect->width = x; rect->height = y; if (wi) @@ -938,7 +989,7 @@ void floating_resize(Con *floating_con, uint32_t x, uint32_t y) { if (hi) rect->height += (hi - 1 - rect->height) % hi; - floating_check_size(floating_con); + floating_check_size(floating_con, prefer_height); /* If this is a scratchpad window, don't auto center it from now on. */ if (floating_con->scratchpad_state == SCRATCHPAD_FRESH) diff --git a/src/handlers.c b/src/handlers.c index b9677917..c4b8cd08 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -974,132 +974,14 @@ static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t stat return false; } - xcb_size_hints_t size_hints; + bool changed = window_update_normal_hints(con->window, reply, NULL); - /* If the hints were already in this event, use them, if not, request them */ - if (reply != NULL) { - xcb_icccm_get_wm_size_hints_from_reply(&size_hints, reply); - } 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)) { - 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 ((size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MAX_SIZE)) { - DLOG("Maximum size: %d (width) x %d (height)\n", size_hints.max_width, size_hints.max_height); - - con->window->max_width = size_hints.max_width; - con->window->max_height = size_hints.max_height; - } - - if (con_is_floating(con)) { - win_width = MAX(win_width, con->window->min_width); - win_height = MAX(win_height, con->window->min_height); - win_width = MIN(win_width, con->window->max_width); - win_height = MIN(win_height, con->window->max_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 (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 (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"); - } - } - - bool has_base_size = false; - int base_width = 0; - 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; - 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; - } - - // 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; - } - - /* If no aspect ratio was set or if it was invalid, we ignore the hints */ - if (!(size_hints.flags & XCB_ICCCM_SIZE_HINT_P_ASPECT) || - (size_hints.min_aspect_num <= 0) || - (size_hints.min_aspect_den <= 0)) { - goto render_and_return; - } - - /* 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.max_aspect_den; - - DLOG("Aspect ratio set: minimum %f, maximum %f\n", min_aspect, max_aspect); - 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) { - goto render_and_return; - } - - /* Check if we need to set proportional_* variables using the correct ratio */ - double aspect_ratio = 0.0; - if ((width / height) < min_aspect) { - aspect_ratio = min_aspect; - } else if ((width / height) > max_aspect) { - aspect_ratio = max_aspect; - } else { - goto render_and_return; - } - - if (fabs(con->window->aspect_ratio - aspect_ratio) > DBL_EPSILON) { - con->window->aspect_ratio = aspect_ratio; - changed = true; - } - -render_and_return: if (changed) { - tree_render(); + Con *floating = con_inside_floating(con); + if (floating) { + floating_check_size(con, false); + tree_render(); + } } FREE(reply); diff --git a/src/load_layout.c b/src/load_layout.c index b4d2a688..003affee 100644 --- a/src/load_layout.c +++ b/src/load_layout.c @@ -152,7 +152,7 @@ static int json_end_map(void *ctx) { } } - floating_check_size(json_node); + floating_check_size(json_node, false); } if (num_marks > 0) { diff --git a/src/manage.c b/src/manage.c index c4706b0d..c222d351 100644 --- a/src/manage.c +++ b/src/manage.c @@ -185,9 +185,7 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki window_update_hints(cwindow, xcb_get_property_reply(conn, wm_hints_cookie, NULL), &urgency_hint); border_style_t motif_border_style = BS_NORMAL; window_update_motif_hints(cwindow, xcb_get_property_reply(conn, motif_wm_hints_cookie, NULL), &motif_border_style); - xcb_size_hints_t wm_size_hints; - if (!xcb_icccm_get_wm_size_hints_reply(conn, wm_normal_hints_cookie, &wm_size_hints, NULL)) - memset(&wm_size_hints, '\0', sizeof(xcb_size_hints_t)); + window_update_normal_hints(cwindow, xcb_get_property_reply(conn, wm_normal_hints_cookie, NULL), geom); xcb_get_property_reply_t *type_reply = xcb_get_property_reply(conn, wm_type_cookie, NULL); xcb_get_property_reply_t *state_reply = xcb_get_property_reply(conn, state_cookie, NULL); @@ -437,10 +435,9 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki xcb_reply_contains_atom(type_reply, A__NET_WM_WINDOW_TYPE_TOOLBAR) || xcb_reply_contains_atom(type_reply, A__NET_WM_WINDOW_TYPE_SPLASH) || xcb_reply_contains_atom(state_reply, A__NET_WM_STATE_MODAL) || - (wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MAX_SIZE && - wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MIN_SIZE && - wm_size_hints.min_height == wm_size_hints.max_height && - wm_size_hints.min_width == wm_size_hints.max_width)) { + (cwindow->max_width > 0 && cwindow->max_height > 0 && + cwindow->min_height == cwindow->max_height && + cwindow->min_width == cwindow->max_width)) { LOG("This window is a dialog window, setting floating\n"); want_floating = true; } @@ -499,29 +496,6 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki if (cwindow->dock) want_floating = false; - /* Plasma windows set their geometry in WM_SIZE_HINTS. */ - if ((wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_US_POSITION || wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_P_POSITION) && - (wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_US_SIZE || wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_P_SIZE)) { - DLOG("We are setting geometry according to wm_size_hints x=%d y=%d w=%d h=%d\n", - wm_size_hints.x, wm_size_hints.y, wm_size_hints.width, wm_size_hints.height); - geom->x = wm_size_hints.x; - geom->y = wm_size_hints.y; - geom->width = wm_size_hints.width; - 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; - } - - if (wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MAX_SIZE) { - DLOG("Window specifies maximum size %d x %d\n", wm_size_hints.max_width, wm_size_hints.max_height); - nc->window->max_width = wm_size_hints.max_width; - nc->window->max_height = wm_size_hints.max_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/src/render.c b/src/render.c index 8ea21f27..518d436b 100644 --- a/src/render.c +++ b/src/render.c @@ -64,35 +64,6 @@ void render_con(Con *con, bool render_fullscreen) { inset->width -= (2 * con->border_width); inset->height -= (2 * con->border_width); - /* Obey the aspect ratio, if any, unless we are in fullscreen mode. - * - * The spec isn’t explicit on whether the aspect ratio hints should be - * respected during fullscreen mode. Other WMs such as Openbox don’t do - * that, and this post suggests that this is the correct way to do it: - * https://mail.gnome.org/archives/wm-spec-list/2003-May/msg00007.html - * - * Ignoring aspect ratio during fullscreen was necessary to fix MPlayer - * subtitle rendering, see https://bugs.i3wm.org/594 */ - if (!render_fullscreen && con->window->aspect_ratio > 0.0) { - DLOG("aspect_ratio = %f, current width/height are %d/%d\n", - con->window->aspect_ratio, inset->width, inset->height); - double new_height = inset->height + 1; - int new_width = inset->width; - - while (new_height > inset->height) { - new_height = (1.0 / con->window->aspect_ratio) * new_width; - - if (new_height > inset->height) - new_width--; - } - /* Center the window */ - inset->y += ceil(inset->height / 2) - floor((new_height + .5) / 2); - inset->x += ceil(inset->width / 2) - floor(new_width / 2); - - inset->height = new_height + .5; - inset->width = new_width; - } - /* NB: We used to respect resize increment size hints for tiling * windows up until commit 0db93d9 here. However, since all terminal * emulators cope with ignoring the size hints in a better way than we diff --git a/src/scratchpad.c b/src/scratchpad.c index d564bf32..a679f20a 100644 --- a/src/scratchpad.c +++ b/src/scratchpad.c @@ -196,7 +196,7 @@ bool scratchpad_show(Con *con) { Con *output = con_get_output(con); con->rect.width = output->rect.width * 0.5; con->rect.height = output->rect.height * 0.75; - floating_check_size(con); + floating_check_size(con, false); floating_center(con, con_get_workspace(con)->rect); } diff --git a/src/window.c b/src/window.c index 61282556..bec4c691 100644 --- a/src/window.c +++ b/src/window.c @@ -272,6 +272,127 @@ void window_update_type(i3Window *window, xcb_get_property_reply_t *reply) { run_assignments(window); } +/* + * Updates the WM_NORMAL_HINTS + * + */ +bool window_update_normal_hints(i3Window *win, xcb_get_property_reply_t *reply, xcb_get_geometry_reply_t *geom) { + bool changed = false; + xcb_size_hints_t size_hints; + + /* If the hints were already in this event, use them, if not, request them */ + bool success; + if (reply != NULL) { + success = xcb_icccm_get_wm_size_hints_from_reply(&size_hints, reply); + } else { + success = xcb_icccm_get_wm_normal_hints_reply(conn, xcb_icccm_get_wm_normal_hints_unchecked(conn, win->id), &size_hints, NULL); + } + if (!success) { + DLOG("Could not get WM_NORMAL_HINTS\n"); + return false; + } + +#define ASSIGN_IF_CHANGED(original, new) \ + do { \ + if (original != new) { \ + original = new; \ + changed = true; \ + } \ + } while (0) + + if ((size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MIN_SIZE)) { + DLOG("Minimum size: %d (width) x %d (height)\n", size_hints.min_width, size_hints.min_height); + + ASSIGN_IF_CHANGED(win->min_width, size_hints.min_width); + ASSIGN_IF_CHANGED(win->min_height, size_hints.min_height); + } + + if ((size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MAX_SIZE)) { + DLOG("Maximum size: %d (width) x %d (height)\n", size_hints.max_width, size_hints.max_height); + + int max_width = max(0, size_hints.max_width); + int max_height = max(0, size_hints.max_height); + + ASSIGN_IF_CHANGED(win->max_width, max_width); + ASSIGN_IF_CHANGED(win->max_height, max_height); + } else { + DLOG("Clearing maximum size \n"); + + ASSIGN_IF_CHANGED(win->max_width, 0); + ASSIGN_IF_CHANGED(win->max_height, 0); + } + + if ((size_hints.flags & XCB_ICCCM_SIZE_HINT_P_RESIZE_INC)) { + DLOG("Size increments: %d (width) x %d (height)\n", size_hints.width_inc, size_hints.height_inc); + + if (size_hints.width_inc > 0 && size_hints.width_inc < 0xFFFF) { + ASSIGN_IF_CHANGED(win->width_increment, size_hints.width_inc); + } else { + ASSIGN_IF_CHANGED(win->width_increment, 0); + } + + if (size_hints.height_inc > 0 && size_hints.height_inc < 0xFFFF) { + ASSIGN_IF_CHANGED(win->height_increment, size_hints.height_inc); + } else { + ASSIGN_IF_CHANGED(win->height_increment, 0); + } + } else { + DLOG("Clearing size increments\n"); + + ASSIGN_IF_CHANGED(win->width_increment, 0); + ASSIGN_IF_CHANGED(win->height_increment, 0); + } + + /* The base width / height is the desired size of the window. */ + if (size_hints.flags & XCB_ICCCM_SIZE_HINT_BASE_SIZE && + (win->base_width >= 0) && (win->base_height >= 0)) { + DLOG("Base size: %d (width) x %d (height)\n", size_hints.base_width, size_hints.base_height); + + ASSIGN_IF_CHANGED(win->base_width, size_hints.base_width); + ASSIGN_IF_CHANGED(win->base_height, size_hints.base_height); + } else { + DLOG("Clearing base size\n"); + + ASSIGN_IF_CHANGED(win->base_width, 0); + ASSIGN_IF_CHANGED(win->base_height, 0); + } + + if (geom != NULL && + (size_hints.flags & XCB_ICCCM_SIZE_HINT_US_POSITION || size_hints.flags & XCB_ICCCM_SIZE_HINT_P_POSITION) && + (size_hints.flags & XCB_ICCCM_SIZE_HINT_US_SIZE || size_hints.flags & XCB_ICCCM_SIZE_HINT_P_SIZE)) { + DLOG("Setting geometry x=%d y=%d w=%d h=%d\n", size_hints.x, size_hints.y, size_hints.width, size_hints.height); + geom->x = size_hints.x; + geom->y = size_hints.y; + geom->width = size_hints.width; + geom->height = size_hints.height; + } + + /* If no aspect ratio was set or if it was invalid, we ignore the hints */ + if (size_hints.flags & XCB_ICCCM_SIZE_HINT_P_ASPECT && + (size_hints.min_aspect_num >= 0) && (size_hints.min_aspect_den > 0) && + (size_hints.max_aspect_num >= 0) && (size_hints.max_aspect_den > 0)) { + /* 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.max_aspect_den; + DLOG("Aspect ratio set: minimum %f, maximum %f\n", min_aspect, max_aspect); + if (fabs(win->min_aspect_ratio - min_aspect) > DBL_EPSILON) { + win->min_aspect_ratio = min_aspect; + changed = true; + } + if (fabs(win->max_aspect_ratio - max_aspect) > DBL_EPSILON) { + win->max_aspect_ratio = max_aspect; + changed = true; + } + } else { + DLOG("Clearing aspect ratios\n"); + + ASSIGN_IF_CHANGED(win->min_aspect_ratio, 0.0); + ASSIGN_IF_CHANGED(win->max_aspect_ratio, 0.0); + } + + return changed; +} + /* * Updates the WM_HINTS (we only care about the input focus handling part). * diff --git a/testcases/t/133-size-hints.t b/testcases/t/133-size-hints.t index a9a43cb5..a16c5399 100644 --- a/testcases/t/133-size-hints.t +++ b/testcases/t/133-size-hints.t @@ -16,31 +16,97 @@ # # Checks if size hints are interpreted correctly. # -use i3test; +use i3test i3_config => < [0, 0, 100, 100], + before_map => sub { + my ($window) = @_; + my $aspect = X11::XCB::Sizehints::Aspect->new; + $aspect->min_num($min_num); + $aspect->min_den($min_den); + $aspect->max_num($max_num); + $aspect->max_den($max_den); + $window->hints->aspect($aspect); + }); +} -my $win = open_window({ dont_map => 1 }); -# XXX: we should check screen size. in screens with an AR of 2.0, -# this is not a good idea. -my $aspect = X11::XCB::Sizehints::Aspect->new; -$aspect->min_num(600); -$aspect->min_den(300); -$aspect->max_num(600); -$aspect->max_den(300); -$win->_create; -$win->map; -wait_for_map $win; -$win->hints->aspect($aspect); -$x->flush; +################################################################################ +# Test aspect ratio set exactly to 2.0 +################################################################################ -sync_with_i3; +fresh_workspace; +my $win = open_with_aspect(600, 300, 600, 300); my $rect = $win->rect; my $ar = $rect->width / $rect->height; -diag("Aspect ratio = $ar"); -ok(($ar > 1.90) && ($ar < 2.10), 'Aspect ratio about 2.0'); +cmp_float($ar, 2, 'Window set to floating with aspect ratio 2.0'); + +cmd 'resize set 100'; +$rect = $win->rect; +$ar = $rect->width / $rect->height; +cmp_float($ar, 2, 'Window resized with aspect ratio kept to 2.0'); + +cmd 'resize set 400 100'; +$rect = $win->rect; +$ar = $rect->width / $rect->height; +cmp_float($ar, 2, 'Window resized with aspect ratio kept to 2.0'); + +# Also check that it is possible to resize by height only +cmd 'resize set height 400'; +$rect = $win->rect; +$ar = $rect->width / $rect->height; +is($rect->height, 400, 'Window height is 400px'); +cmp_float($ar, 2, 'Window resized with aspect ratio kept to 2.0'); + +cmd 'resize grow height 10'; +$rect = $win->rect; +$ar = $rect->width / $rect->height; +is($rect->height, 410, 'Window grew by 10px'); +cmp_float($ar, 2, 'Window resized with aspect ratio kept to 2.0'); + +################################################################################ +# Test aspect ratio between 0.5 and 2.0 +################################################################################ + +fresh_workspace; +$win = open_with_aspect(1, 2, 2, 1); + +$rect = $win->rect; +$ar = $rect->width / $rect->height; +cmp_float($ar, 1, 'Window set to floating with aspect ratio 1.0'); + +cmd 'resize set 200'; +$rect = $win->rect; +$ar = $rect->width / $rect->height; +is($rect->width, 200, 'Window width is 200px'); +is($rect->height, 100, 'Window height stayed 100px'); +cmp_float($ar, 2, 'Window resized, aspect ratio changed to 2.0'); + +cmd 'resize set 100 200'; +$rect = $win->rect; +$ar = $rect->width / $rect->height; +is($rect->width, 100, 'Window width is 100px'); +is($rect->height, 200, 'Window height is 200px'); +cmp_float($ar, 0.5, 'Window resized, aspect ratio changed to 0.5'); + +cmd 'resize set 500'; +$rect = $win->rect; +$ar = $rect->width / $rect->height; +cmp_float($ar, 2, 'Window resized, aspect ratio changed to maximum 2.0'); + +cmd 'resize set 100 400'; +$rect = $win->rect; +$ar = $rect->width / $rect->height; +cmp_float($ar, 0.5, 'Window resized, aspect ratio changed to minimum 0.5'); done_testing; From 100d05a2a662c339da616c6bdc3a1f7e3d95d355 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Tue, 2 Oct 2018 02:13:51 +0300 Subject: [PATCH 4/5] render_con: Get rid of render_fullscreen argument Only true for the fullscreen container and doesn't affect any of its children. Thus, we can get the same result by checking ->fullscreen_mode. --- include/render.h | 2 +- src/commands.c | 2 +- src/floating.c | 5 ++--- src/manage.c | 4 ++-- src/render.c | 24 ++++++++++++------------ src/tree.c | 2 +- 6 files changed, 19 insertions(+), 20 deletions(-) diff --git a/include/render.h b/include/render.h index 2b2c8dad..03751c01 100644 --- a/include/render.h +++ b/include/render.h @@ -40,7 +40,7 @@ typedef struct render_params { * updated in X11. * */ -void render_con(Con *con, bool render_fullscreen); +void render_con(Con *con); /** * Returns the height for the decorations diff --git a/src/commands.c b/src/commands.c index ee402846..ed5e482d 100644 --- a/src/commands.c +++ b/src/commands.c @@ -831,7 +831,7 @@ void cmd_append_layout(I3_CMD, const char *cpath) { // is not executed yet and will be batched with append_layout’s // needs_tree_render after the parser finished. We should check if that is // necessary at all. - render_con(croot, false); + render_con(croot); restore_open_placeholder_windows(parent); diff --git a/src/floating.c b/src/floating.c index 4f2760a7..c8b436b4 100644 --- a/src/floating.c +++ b/src/floating.c @@ -412,8 +412,7 @@ void floating_enable(Con *con, bool automatic) { DLOG("Corrected y = %d (deco_height = %d)\n", nc->rect.y, deco_height); /* render the cons to get initial window_rect correct */ - render_con(nc, false); - render_con(con, false); + render_con(nc); if (set_focus) con_activate(con); @@ -568,7 +567,7 @@ DRAGGING_CB(drag_window_callback) { con->rect.x = old_rect->x + (new_x - event->root_x); con->rect.y = old_rect->y + (new_y - event->root_y); - render_con(con, false); + render_con(con); x_push_node(con); xcb_flush(conn); diff --git a/src/manage.c b/src/manage.c index c222d351..63cadc0c 100644 --- a/src/manage.c +++ b/src/manage.c @@ -569,13 +569,13 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki * workspace at all. However, just calling render_con() on the * workspace isn’t enough either — it needs the rect. */ ws->rect = ws->parent->rect; - render_con(ws, true); + render_con(ws); /* Disable setting focus, otherwise we’d move focus to an invisible * workspace, which we generally prevent (e.g. in * con_move_to_workspace). */ set_focus = false; } - render_con(croot, false); + render_con(croot); /* Send an event about window creation */ ipc_send_window_event("new", nc); diff --git a/src/render.c b/src/render.c index 518d436b..e9e38ae0 100644 --- a/src/render.c +++ b/src/render.c @@ -37,16 +37,15 @@ int render_deco_height(void) { * updated in X11. * */ -void render_con(Con *con, bool render_fullscreen) { +void render_con(Con *con) { render_params params = { .rect = con->rect, .x = con->rect.x, .y = con->rect.y, .children = con_num_children(con)}; - DLOG("Rendering %snode %p / %s / layout %d / children %d\n", - (render_fullscreen ? "fullscreen " : ""), con, con->name, con->layout, - params.children); + DLOG("Rendering node %p / %s / layout %d / children %d\n", con, con->name, + con->layout, params.children); int i = 0; con->mapped = true; @@ -57,8 +56,9 @@ void render_con(Con *con, bool render_fullscreen) { * needs to be smaller */ Rect *inset = &(con->window_rect); *inset = (Rect){0, 0, con->rect.width, con->rect.height}; - if (!render_fullscreen) + if (con->fullscreen_mode == CF_NONE) { *inset = rect_add(*inset, con_border_style_rect(con)); + } /* Obey x11 border */ inset->width -= (2 * con->border_width); @@ -81,7 +81,7 @@ void render_con(Con *con, bool render_fullscreen) { if (fullscreen) { fullscreen->rect = params.rect; x_raise_con(fullscreen); - render_con(fullscreen, true); + render_con(fullscreen); /* Fullscreen containers are either global (underneath the CT_ROOT * container) or per-output (underneath the CT_CONTENT container). For * global fullscreen containers, we cannot abort rendering here yet, @@ -124,7 +124,7 @@ void render_con(Con *con, bool render_fullscreen) { DLOG("child at (%d, %d) with (%d x %d)\n", child->rect.x, child->rect.y, child->rect.width, child->rect.height); x_raise_con(child); - render_con(child, false); + render_con(child); i++; } @@ -137,7 +137,7 @@ void render_con(Con *con, bool render_fullscreen) { * that we have a non-leaf-container inside the stack. In that * case, the children of the non-leaf-container need to be raised * as well. */ - render_con(child, false); + render_con(child); } if (params.children != 1) @@ -186,7 +186,7 @@ static void render_root(Con *con, Con *fullscreen) { Con *output; if (!fullscreen) { TAILQ_FOREACH(output, &(con->nodes_head), nodes) { - render_con(output, false); + render_con(output); } } @@ -252,7 +252,7 @@ static void render_root(Con *con, Con *fullscreen) { DLOG("floating child at (%d,%d) with %d x %d\n", child->rect.x, child->rect.y, child->rect.width, child->rect.height); x_raise_con(child); - render_con(child, false); + render_con(child); } } } @@ -302,7 +302,7 @@ static void render_output(Con *con) { if (fullscreen) { fullscreen->rect = con->rect; x_raise_con(fullscreen); - render_con(fullscreen, true); + render_con(fullscreen); return; } @@ -342,7 +342,7 @@ static void render_output(Con *con) { DLOG("child at (%d, %d) with (%d x %d)\n", child->rect.x, child->rect.y, child->rect.width, child->rect.height); x_raise_con(child); - render_con(child, false); + render_con(child); } } diff --git a/src/tree.c b/src/tree.c index e3849873..99b03619 100644 --- a/src/tree.c +++ b/src/tree.c @@ -453,7 +453,7 @@ void tree_render(void) { mark_unmapped(croot); croot->mapped = true; - render_con(croot, false); + render_con(croot); x_push_changes(croot); DLOG("-- END RENDERING --\n"); From d2d6d6e0a86e479ab799e84a1ba15b0c13d288f0 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Tue, 2 Oct 2018 02:15:59 +0300 Subject: [PATCH 5/5] Re-render floating cons alone when possible --- src/floating.c | 6 ++---- src/handlers.c | 3 ++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/floating.c b/src/floating.c index c8b436b4..bfb130fb 100644 --- a/src/floating.c +++ b/src/floating.c @@ -674,9 +674,7 @@ DRAGGING_CB(resize_window_callback) { con->rect.x = dest_x; con->rect.y = dest_y; - /* TODO: don’t re-render the whole tree just because we change - * coordinates of a floating window */ - tree_render(); + render_con(con); x_push_changes(croot); } @@ -957,7 +955,7 @@ bool floating_reposition(Con *con, Rect newrect) { /* Workspace change will already result in a tree_render. */ if (!reassigned) { - render_con(con, false); + render_con(con); x_push_node(con); } return true; diff --git a/src/handlers.c b/src/handlers.c index c4b8cd08..47abe2b4 100644 --- a/src/handlers.c +++ b/src/handlers.c @@ -980,7 +980,8 @@ static bool handle_normal_hints(void *data, xcb_connection_t *conn, uint8_t stat Con *floating = con_inside_floating(con); if (floating) { floating_check_size(con, false); - tree_render(); + render_con(con); + x_push_changes(croot); } }