wayland: handle modifier keys correctly

I don't know why we've been doing this wrong for so long or how I didn't
notice until now. Wayland specifically has an event for handling
modifiers. We even named it "keyboard_handle_modifiers" in the code.
What we should do is just get the modifier and save it after the xkb
state is updated. Then later if the user does something else (press
another key or clicks the mouse button), the saved modifier key is
applied. If you let go of the modifier at any point, the xkb will just
update its state again and we save a 0 again here (i.e. no modifier).

There is one bit of an edge case however. If a key is pressed BEFORE the
modifier, then we have to handle the mp_input_put_key in the modifier
event instead since the ordering is not guarenteed. What we do here is
keep track of the mpkey as well as the mpmod. However if we are unable
to find a matching mpkey and the key state is pressed down, assume it's
a modifery key that was pressed and don't update mpkey. That way
whenever the modifier event does happen, it can correctly handle this
and we know that the keys must be pressed down if we end up there in the
code path.

As another fun historical note, the xkb_keysym_to_utf8 line was actually
written by wm4 himself in 460ef9c7a4
nearly 10 years ago. As the commit shows, it was clearly intended to
handle modifiers (if lookupkey finds nothing, then try to find a mod
instead). Of course, this is extremely dated and wayland hasn't worked
like that in ages. This branch never actually did anything, and thus
we'll remove it here along with modifier lookup changes.

This solves bizarre issues with modifiers not working with random keys
while working fine with others (don't ask me why). Fixes #10286 and
fixes #11945.
This commit is contained in:
Dudemanguy 2023-07-15 14:20:27 -05:00
parent 347fbd6fa3
commit 1f8013ff3f
2 changed files with 22 additions and 20 deletions

View File

@ -240,8 +240,6 @@ static void pointer_handle_button(void *data, struct wl_pointer *wl_pointer,
uint32_t state) uint32_t state)
{ {
struct vo_wayland_state *wl = data; struct vo_wayland_state *wl = data;
int mpmod = 0;
state = state == WL_POINTER_BUTTON_STATE_PRESSED ? MP_KEY_STATE_DOWN state = state == WL_POINTER_BUTTON_STATE_PRESSED ? MP_KEY_STATE_DOWN
: MP_KEY_STATE_UP; : MP_KEY_STATE_UP;
@ -270,11 +268,8 @@ static void pointer_handle_button(void *data, struct wl_pointer *wl_pointer,
button = 0; button = 0;
} }
if (wl->keyboard)
mpmod = get_mods(wl);
if (button) if (button)
mp_input_put_key(wl->vo->input_ctx, button | state | mpmod); mp_input_put_key(wl->vo->input_ctx, button | state | wl->mpmod);
if (!mp_input_test_dragging(wl->vo->input_ctx, wl->mouse_x, wl->mouse_y) && if (!mp_input_test_dragging(wl->vo->input_ctx, wl->mouse_x, wl->mouse_y) &&
(!wl->vo_opts->fullscreen) && (!wl->vo_opts->window_maximized) && (!wl->vo_opts->fullscreen) && (!wl->vo_opts->window_maximized) &&
@ -297,20 +292,19 @@ static void pointer_handle_axis(void *data, struct wl_pointer *wl_pointer,
{ {
struct vo_wayland_state *wl = data; struct vo_wayland_state *wl = data;
int mpmod = get_mods(wl);
double val = wl_fixed_to_double(value) < 0 ? -1 : 1; double val = wl_fixed_to_double(value) < 0 ? -1 : 1;
switch (axis) { switch (axis) {
case WL_POINTER_AXIS_VERTICAL_SCROLL: case WL_POINTER_AXIS_VERTICAL_SCROLL:
if (value > 0) if (value > 0)
mp_input_put_wheel(wl->vo->input_ctx, MP_WHEEL_DOWN | mpmod, +val); mp_input_put_wheel(wl->vo->input_ctx, MP_WHEEL_DOWN | wl->mpmod, +val);
if (value < 0) if (value < 0)
mp_input_put_wheel(wl->vo->input_ctx, MP_WHEEL_UP | mpmod, -val); mp_input_put_wheel(wl->vo->input_ctx, MP_WHEEL_UP | wl->mpmod, -val);
break; break;
case WL_POINTER_AXIS_HORIZONTAL_SCROLL: case WL_POINTER_AXIS_HORIZONTAL_SCROLL:
if (value > 0) if (value > 0)
mp_input_put_wheel(wl->vo->input_ctx, MP_WHEEL_RIGHT | mpmod, +val); mp_input_put_wheel(wl->vo->input_ctx, MP_WHEEL_RIGHT | wl->mpmod, +val);
if (value < 0) if (value < 0)
mp_input_put_wheel(wl->vo->input_ctx, MP_WHEEL_LEFT | mpmod, -val); mp_input_put_wheel(wl->vo->input_ctx, MP_WHEEL_LEFT | wl->mpmod, -val);
break; break;
} }
} }
@ -441,18 +435,21 @@ static void keyboard_handle_key(void *data, struct wl_keyboard *wl_keyboard,
wl->keyboard_code = key + 8; wl->keyboard_code = key + 8;
xkb_keysym_t sym = xkb_state_key_get_one_sym(wl->xkb_state, wl->keyboard_code); xkb_keysym_t sym = xkb_state_key_get_one_sym(wl->xkb_state, wl->keyboard_code);
int mpkey = lookupkey(sym);
// Assume a modifier was pressed and handle it in the mod event instead.
if (!mpkey && MP_KEY_STATE_DOWN)
return;
state = state == WL_KEYBOARD_KEY_STATE_PRESSED ? MP_KEY_STATE_DOWN state = state == WL_KEYBOARD_KEY_STATE_PRESSED ? MP_KEY_STATE_DOWN
: MP_KEY_STATE_UP; : MP_KEY_STATE_UP;
int mpmod = get_mods(wl);
int mpkey = lookupkey(sym); if (mpkey)
if (mpkey) { mp_input_put_key(wl->vo->input_ctx, mpkey | state | wl->mpmod);
mp_input_put_key(wl->vo->input_ctx, mpkey | state | mpmod); if (state == MP_KEY_STATE_DOWN)
} else { wl->mpkey = mpkey;
char s[128]; if (wl->mpkey == mpkey && state == MP_KEY_STATE_UP)
if (xkb_keysym_to_utf8(sym, s, sizeof(s)) > 0) wl->mpkey = 0;
mp_input_put_key_utf8(wl->vo->input_ctx, state | mpmod, bstr0(s));
}
} }
static void keyboard_handle_modifiers(void *data, struct wl_keyboard *wl_keyboard, static void keyboard_handle_modifiers(void *data, struct wl_keyboard *wl_keyboard,
@ -465,6 +462,9 @@ static void keyboard_handle_modifiers(void *data, struct wl_keyboard *wl_keyboar
if (wl->xkb_state) { if (wl->xkb_state) {
xkb_state_update_mask(wl->xkb_state, mods_depressed, mods_latched, xkb_state_update_mask(wl->xkb_state, mods_depressed, mods_latched,
mods_locked, 0, 0, group); mods_locked, 0, 0, group);
wl->mpmod = get_mods(wl);
if (wl->mpkey)
mp_input_put_key(wl->vo->input_ctx, wl->mpkey | MP_KEY_STATE_DOWN | wl->mpmod);
} }
} }

View File

@ -147,6 +147,8 @@ struct vo_wayland_state {
struct xkb_keymap *xkb_keymap; struct xkb_keymap *xkb_keymap;
struct xkb_state *xkb_state; struct xkb_state *xkb_state;
uint32_t keyboard_code; uint32_t keyboard_code;
int mpkey;
int mpmod;
/* DND */ /* DND */
struct wl_data_device *dnd_ddev; struct wl_data_device *dnd_ddev;