From e6c536ae45bb186f32afd37c438f97f34d7a9fdc Mon Sep 17 00:00:00 2001 From: Dudemanguy Date: Fri, 6 Sep 2024 10:38:14 -0500 Subject: [PATCH] wayland: fix vertical resizing Resizing in strictly the vertical direction has been broken forever on wayland. It's because of how the keepaspect calculation always resized with respect to the width. So if you moved the mouse strictly vertically with no change left/right, the mpv window would never change size since it just calculates with respect to the width that doesn't change. Fixing this is kind of weird and maybe someone should think of a better algorithm for preserving aspect ratio (we just multiply by the gcd basically), but whatever. You might naively try something like "resize with respect to what direction changes the most" at first, but this leads to very cludgy resizing since during a typical mouse dragging motion, it will flip constantly from "resize w/ respect to the width" and "resize w/ respect to the height". It "works" but it's really ugly and not worth it. The trick is to realize that we need to consider the resizing state as one continuous motion, choose a direction initially, stick to it throughout the entirety of the resize, and reset the relevant parameters after we finish the resize. This ensures the direction never unintuitively flips during a motion, but also allows for the strictly vertical case to still work. Might as well note it here in the commit, but mpv's resizing behavior technically violates xdg-shell. For the resizing event, "[t]he window geometry specified in the configure event is a maximum; the client cannot resize beyond it." Well, we do obviously go beyond the maximum in certain cases. For example consider resizing strictly horizontally. The width value increases from the compositor but not the height. Since mpv preserves aspect ratio by default, the height obviously must increase which will go over the prescribed bounds by the compositor. This happens before and after this commit, and I think everyone agrees is the desired behavior. With this commit, now vertical resizing works which violates xdg-shell in the same way. Before, it incidentally obeyed the protocol since it did not resize at all, but let's presume users expect the window to actually get bigger when they do a drag resize. *: https://wayland.app/protocols/xdg-shell#xdg_toplevel:enum:state:entry:resizing --- video/out/wayland_common.c | 41 ++++++++++++++++++++++++++++++++++++++ video/out/wayland_common.h | 2 ++ 2 files changed, 43 insertions(+) diff --git a/video/out/wayland_common.c b/video/out/wayland_common.c index ba2131c465..bc56134f5f 100644 --- a/video/out/wayland_common.c +++ b/video/out/wayland_common.c @@ -66,6 +66,11 @@ #define WAYLAND_SCALE_FACTOR 120.0 +enum resizing_constraint { + MP_WIDTH_CONSTRAINT = 1, + MP_HEIGHT_CONSTRAINT = 2, +}; + static const struct mp_keymap keymap[] = { /* Special keys */ {XKB_KEY_Pause, MP_KEY_PAUSE}, {XKB_KEY_Escape, MP_KEY_ESC}, @@ -1038,6 +1043,7 @@ static void handle_toplevel_config(void *data, struct xdg_toplevel *toplevel, bool is_maximized = false; bool is_fullscreen = false; bool is_activated = false; + bool is_resizing = false; bool is_suspended = false; bool is_tiled = false; enum xdg_toplevel_state *state; @@ -1047,6 +1053,7 @@ static void handle_toplevel_config(void *data, struct xdg_toplevel *toplevel, is_fullscreen = true; break; case XDG_TOPLEVEL_STATE_RESIZING: + is_resizing = true; break; case XDG_TOPLEVEL_STATE_ACTIVATED: is_activated = true; @@ -1077,6 +1084,11 @@ static void handle_toplevel_config(void *data, struct xdg_toplevel *toplevel, if (wl->hidden != is_suspended) wl->hidden = is_suspended; + if (wl->resizing != is_resizing) { + wl->resizing = is_resizing; + wl->resizing_constraint = 0; + } + if (opts->fullscreen != is_fullscreen) { wl->state_change = wl->reconfigured; opts->fullscreen = is_fullscreen; @@ -1553,10 +1565,39 @@ static void apply_keepaspect(struct vo_wayland_state *wl, int *width, int *heigh if (!wl->opts->keepaspect) return; + int phys_width = handle_round(wl->scaling, *width); + int phys_height = handle_round(wl->scaling, *height); + + // Ensure that the size actually changes before we start trying to actually + // calculate anything so the wrong constraint for the rezie isn't choosen. + if (wl->resizing && !wl->resizing_constraint && + phys_width == mp_rect_w(wl->geometry) && phys_height == mp_rect_h(wl->geometry)) + return; + + // We are doing a continuous resize (e.g. dragging with mouse), constrain the + // aspect ratio against the height if the change is only in the height + // coordinate. + if (wl->resizing && !wl->resizing_constraint && phys_width == mp_rect_w(wl->geometry) && + phys_height != mp_rect_h(wl->geometry)) { + wl->resizing_constraint = MP_HEIGHT_CONSTRAINT; + } else if (!wl->resizing_constraint) { + wl->resizing_constraint = MP_WIDTH_CONSTRAINT; + } + + if (wl->resizing_constraint == MP_HEIGHT_CONSTRAINT) { + MPSWAP(int, *width, *height); + MPSWAP(int, wl->reduced_width, wl->reduced_height); + } + double scale_factor = (double)*width / wl->reduced_width; *width = ceil(wl->reduced_width * scale_factor); if (wl->opts->keepaspect_window) *height = ceil(wl->reduced_height * scale_factor); + + if (wl->resizing_constraint == MP_HEIGHT_CONSTRAINT) { + MPSWAP(int, *width, *height); + MPSWAP(int, wl->reduced_width, wl->reduced_height); + } } static void free_dnd_data(struct vo_wayland_state *wl) diff --git a/video/out/wayland_common.h b/video/out/wayland_common.h index d7164c1c4e..105761698a 100644 --- a/video/out/wayland_common.h +++ b/video/out/wayland_common.h @@ -68,6 +68,7 @@ struct vo_wayland_state { bool locked_size; bool need_rescale; bool reconfigured; + bool resizing; bool scale_configured; bool state_change; bool tiled; @@ -79,6 +80,7 @@ struct vo_wayland_state { int pending_scaling; // base 120 int scaling; // base 120 double scaling_factor; // wl->scaling divided by 120 + int resizing_constraint; int timeout_count; int wakeup_pipe[2];