Skip to content

Commit

Permalink
wayland: fix vertical resizing
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Dudemanguy committed Sep 8, 2024
1 parent 5edc897 commit e6c536a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 0 deletions.
41 changes: 41 additions & 0 deletions video/out/wayland_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions video/out/wayland_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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];

Expand Down

0 comments on commit e6c536a

Please sign in to comment.