Merge pull request #3432 from orestisf1993/aspect-ratio

Fix aspect ratio bugs

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

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.
+ * Called when a floating window is created or resized.  This function resizes
- * This function resizes the window if its size is higher or lower than the
+ * the window if its size is higher or lower than the configured maximum/minimum
- * configured maximum/minimum size, respectively.
+ * 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.
@@ -152,7 +157,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

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

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).
  *

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. */
@@ -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);
 

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

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.
+ * Called when a floating window is created or resized.  This function resizes
- * This function resizes the window if its size is higher or lower than the
+ * the window if its size is higher or lower than the configured maximum/minimum
- * configured maximum/minimum size, respectively.
+ * 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;
@@ -83,43 +88,89 @@ void floating_check_size(Con *floating_con) {
         border_rect.height += render_deco_height();
     }
 
-    if (focused_con->window != NULL) {
+    i3Window *window = focused_con->window;
-        if (focused_con->window->min_width) {
+    if (window != NULL) {
+        /* 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, focused_con->window->min_width);
+            floating_con->rect.width = max(floating_con->rect.width, min_width);
             floating_con->rect.width += border_rect.width;
         }
 
-        if (focused_con->window->min_height) {
+        if (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, 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 &&
+        /* Obey the aspect ratio, if any, unless we are in fullscreen mode.
-            floating_con->rect.height >= focused_con->window->base_height + border_rect.height) {
+         *
-            floating_con->rect.height -= focused_con->window->base_height + border_rect.height;
+         * The spec isn’t explicit on whether the aspect ratio hints should be
-            floating_con->rect.height -= floating_con->rect.height % focused_con->window->height_increment;
+         * respected during fullscreen mode. Other WMs such as Openbox don’t do
-            floating_con->rect.height += focused_con->window->base_height + border_rect.height;
+         * 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 (focused_con->window->width_increment &&
+        if (window->height_increment &&
-            floating_con->rect.width >= focused_con->window->base_width + border_rect.width) {
+            floating_con->rect.height >= base_height + border_rect.height) {
-            floating_con->rect.width -= focused_con->window->base_width + border_rect.width;
+            floating_con->rect.height -= base_height + border_rect.height;
-            floating_con->rect.width -= floating_con->rect.width % focused_con->window->width_increment;
+            floating_con->rect.height -= floating_con->rect.height % window->height_increment;
-            floating_con->rect.width += focused_con->window->base_width + border_rect.width;
+            floating_con->rect.height += base_height + border_rect.height;
+        }
+
+        if (window->width_increment &&
+            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 += base_width + border_rect.width;
         }
     }
 
@@ -313,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
@@ -361,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(nc);
-    render_con(con, false);
 
     if (set_focus)
         con_activate(con);
@@ -517,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);
 
@@ -611,7 +661,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 */
@@ -624,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
+    render_con(con);
-     * coordinates of a floating window */
-    tree_render();
     x_push_changes(croot);
 }
 
@@ -907,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;
@@ -920,7 +968,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);
@@ -930,6 +978,7 @@ void floating_resize(Con *floating_con, int x, int 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)
@@ -937,7 +986,7 @@ void floating_resize(Con *floating_con, int x, int 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)

src/handlers.c

@@ -974,134 +974,17 @@ 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");
+        Con *floating = con_inside_floating(con);
+        if (floating) {
+            floating_check_size(con, false);
+            render_con(con);
+            x_push_changes(croot);
         }
     }
 
-    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();
-    }
-
     FREE(reply);
     return true;
 }

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) {

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;
+    window_update_normal_hints(cwindow, xcb_get_property_reply(conn, wm_normal_hints_cookie, NULL), geom);
-    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));
     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 &&
+        (cwindow->max_width > 0 && cwindow->max_height > 0 &&
-         wm_size_hints.flags & XCB_ICCCM_SIZE_HINT_P_MIN_SIZE &&
+         cwindow->min_height == cwindow->max_height &&
-         wm_size_hints.min_height == wm_size_hints.max_height &&
+         cwindow->min_width == cwindow->max_width)) {
-         wm_size_hints.min_width == wm_size_hints.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/…
@@ -595,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);

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",
+    DLOG("Rendering node %p / %s / layout %d / children %d\n", con, con->name,
-         (render_fullscreen ? "fullscreen " : ""), con, con->name, con->layout,
+         con->layout, params.children);
-         params.children);
 
     int i = 0;
     con->mapped = true;
@@ -57,42 +56,14 @@ 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);
         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
@@ -110,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,
@@ -153,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++;
         }
 
@@ -166,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)
@@ -215,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);
         }
     }
 
@@ -281,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);
         }
     }
 }
@@ -331,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;
     }
 
@@ -371,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);
     }
 }
 

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

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");

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).
  *

testcases/t/133-size-hints.t

@@ -16,31 +16,97 @@
 #
 # Checks if size hints are interpreted correctly.
 #
-use i3test;
+use i3test i3_config => <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
 
-my $tmp = fresh_workspace;
+default_floating_border none
+floating_minimum_size -1 x -1
+floating_maximum_size -1 x -1
+EOT
 
-ok(@{get_ws_content($tmp)} == 0, 'no containers yet');
+sub open_with_aspect {
-
+    my ($min_num, $min_den, $max_num, $max_den) = @_;
-my $win = open_window({ dont_map => 1 });
+    open_floating_window(
-# XXX: we should check screen size. in screens with an AR of 2.0,
+        rect => [0, 0, 100, 100],
-# this is not a good idea.
+        before_map => sub {
+            my ($window) = @_;
             my $aspect = X11::XCB::Sizehints::Aspect->new;
-$aspect->min_num(600);
+            $aspect->min_num($min_num);
-$aspect->min_den(300);
+            $aspect->min_den($min_den);
-$aspect->max_num(600);
+            $aspect->max_num($max_num);
-$aspect->max_den(300);
+            $aspect->max_den($max_den);
-$win->_create;
+            $window->hints->aspect($aspect);
-$win->map;
+        });
-wait_for_map $win;
+}
-$win->hints->aspect($aspect);
-$x->flush;
 
-sync_with_i3;
+################################################################################
+# Test aspect ratio set exactly to 2.0
+################################################################################
+
+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");
+cmp_float($ar, 2, 'Window set to floating with aspect ratio 2.0');
-ok(($ar > 1.90) && ($ar < 2.10), 'Aspect ratio about 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;