[PATCH 0/2] MR5561: winewayland.drv: Numpad key handling
This MR updates the Wayland driver keyboard code to emit the appropriate numpad virtual keys (depending on NumLock state) and also translate them to the matching WCHARs. This is the first step in fixing https://bugs.winehq.org/show_bug.cgi?id=56397. The next step (in an upcoming MR) involves syncing of modifier (and other key) state, so we can deal with cases where the modifiers are set while the application doesn't have the keyboard focus (e.g., before it starts). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5561
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/wayland_keyboard.c | 31 ++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/dlls/winewayland.drv/wayland_keyboard.c b/dlls/winewayland.drv/wayland_keyboard.c index 04a1c0a9fc3..8c6ba86d915 100644 --- a/dlls/winewayland.drv/wayland_keyboard.c +++ b/dlls/winewayland.drv/wayland_keyboard.c @@ -647,6 +647,35 @@ static void release_all_keys(HWND hwnd) } } +static UINT scan2vkey(UINT scan) +{ + UINT vkey = NtUserMapVirtualKeyEx(scan, MAPVK_VSC_TO_VK_EX, keyboard_hkl); + BYTE state[256]; + UINT num_vkey; + + switch (scan) + { + case KEY_KP0: num_vkey = VK_NUMPAD0; break; + case KEY_KP1: num_vkey = VK_NUMPAD1; break; + case KEY_KP2: num_vkey = VK_NUMPAD2; break; + case KEY_KP3: num_vkey = VK_NUMPAD3; break; + case KEY_KP4: num_vkey = VK_NUMPAD4; break; + case KEY_KP5: num_vkey = VK_NUMPAD5; break; + case KEY_KP6: num_vkey = VK_NUMPAD6; break; + case KEY_KP7: num_vkey = VK_NUMPAD7; break; + case KEY_KP8: num_vkey = VK_NUMPAD8; break; + case KEY_KP9: num_vkey = VK_NUMPAD9; break; + case KEY_KPDOT: num_vkey = VK_DECIMAL; break; + default: num_vkey = vkey; break; + } + + if (num_vkey != vkey && get_async_key_state(state) && + (state[VK_NUMLOCK] & 1) && !(state[VK_SHIFT] & 0x80)) + return num_vkey; + + return vkey; +} + /********************************************************************** * Keyboard handling */ @@ -825,7 +854,7 @@ static void keyboard_handle_key(void *data, struct wl_keyboard *wl_keyboard, input.type = INPUT_KEYBOARD; input.ki.wScan = scan & 0xff; - input.ki.wVk = NtUserMapVirtualKeyEx(scan, MAPVK_VSC_TO_VK_EX, keyboard_hkl); + input.ki.wVk = scan2vkey(scan); if (scan & ~0xff) input.ki.dwFlags |= KEYEVENTF_EXTENDEDKEY; if (state == WL_KEYBOARD_KEY_STATE_RELEASED) input.ki.dwFlags |= KEYEVENTF_KEYUP; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5561
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/winewayland.drv/wayland_keyboard.c | 35 ++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/dlls/winewayland.drv/wayland_keyboard.c b/dlls/winewayland.drv/wayland_keyboard.c index 8c6ba86d915..1631964ae53 100644 --- a/dlls/winewayland.drv/wayland_keyboard.c +++ b/dlls/winewayland.drv/wayland_keyboard.c @@ -358,7 +358,7 @@ static void add_xkb_layout(const char *xkb_layout, struct xkb_keymap *xkb_keymap unsigned int mod, keyc, len, names_len, min_keycode, max_keycode; struct xkb_state *xkb_state = xkb_state_new(xkb_keymap); - xkb_mod_mask_t shift_mask, control_mask, altgr_mask, capslock_mask; + xkb_mod_mask_t shift_mask, control_mask, altgr_mask, capslock_mask, numlock_mask; VSC_LPWSTR *names_entry, *names_ext_entry; VSC_VK *vsc2vk_e0_entry, *vsc2vk_e1_entry; VK_TO_WCHARS8 *vk2wchars_entry; @@ -493,6 +493,7 @@ static void add_xkb_layout(const char *xkb_layout, struct xkb_keymap *xkb_keymap control_mask = 1 << xkb_keymap_mod_get_index(xkb_keymap, XKB_MOD_NAME_CTRL); capslock_mask = 1 << xkb_keymap_mod_get_index(xkb_keymap, XKB_MOD_NAME_CAPS); altgr_mask = 1 << xkb_keymap_mod_get_index(xkb_keymap, "Mod5"); + numlock_mask = 1 << xkb_keymap_mod_get_index(xkb_keymap, XKB_MOD_NAME_NUM); for (keyc = min_keycode; keyc <= max_keycode; keyc++) { @@ -502,6 +503,38 @@ static void add_xkb_layout(const char *xkb_layout, struct xkb_keymap *xkb_keymap uint32_t caps_ret, shift_ret; unsigned int mod; + if (vkey & KBDNUMPAD) + { + VK_TO_WCHARS8 num_vkey2wch; + + switch (vkey & 0xff) + { + case VK_INSERT: num_vkey2wch.VirtualKey = VK_NUMPAD0; break; + case VK_END: num_vkey2wch.VirtualKey = VK_NUMPAD1; break; + case VK_DOWN: num_vkey2wch.VirtualKey = VK_NUMPAD2; break; + case VK_NEXT: num_vkey2wch.VirtualKey = VK_NUMPAD3; break; + case VK_LEFT: num_vkey2wch.VirtualKey = VK_NUMPAD4; break; + case VK_CLEAR: num_vkey2wch.VirtualKey = VK_NUMPAD5; break; + case VK_RIGHT: num_vkey2wch.VirtualKey = VK_NUMPAD6; break; + case VK_HOME: num_vkey2wch.VirtualKey = VK_NUMPAD7; break; + case VK_UP: num_vkey2wch.VirtualKey = VK_NUMPAD8; break; + case VK_PRIOR: num_vkey2wch.VirtualKey = VK_NUMPAD9; break; + case VK_DELETE: num_vkey2wch.VirtualKey = VK_DECIMAL; break; + default: num_vkey2wch.VirtualKey = 0; break; + } + + if (num_vkey2wch.VirtualKey) + { + xkb_state_update_mask(xkb_state, 0, 0, numlock_mask, 0, 0, xkb_group); + if (!(num_vkey2wch.wch[0] = xkb_state_key_get_utf32(xkb_state, keyc))) + num_vkey2wch.wch[0] = WCH_NONE; + for (mod = 1; mod < 8; ++mod) num_vkey2wch.wch[mod] = WCH_NONE; + num_vkey2wch.Attributes = 0; + TRACE("vkey %#06x -> %s\n", num_vkey2wch.VirtualKey, debugstr_wn(num_vkey2wch.wch, 8)); + *vk2wchars_entry++ = num_vkey2wch; + } + } + for (mod = 0; mod < 8; ++mod) { xkb_mod_mask_t mod_mask = 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5561
This doesn't seem to be something driver specific, and instead I suspect that this would better be done somewhere in win32u, with the KBDNUMPAD flag. Fwiw, it'll probably require more work in the stack but IMO the drivers shouldn't have to provide a vkey in the events they send to wineserver, and the NtUserMapVirtualKeyEx call would also probably be better done on the receiving side of win32u. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5561#note_69435
Actually I had something related in https://gitlab.winehq.org/rbernon/wine/-/commit/0d119e5f7d7fd5dc0cd2a890c232..., could it help your case? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5561#note_69436
@rbernon I have a WIP prototype at https://gitlab.winehq.org/afrantzis/wine/-/commits/wayland-numlock-win32u which partially moves in the direction you suggested. It still performs the scan -> vkey mapping on the sending side, but at least it's now done internally in win32u so will benefit all drivers that use KBDTABLES. The prototype currently doesn't handle 0xE1 extended scancode since I am not sure what's the best way to pass the info. Perhaps we can just pass the full scancode directly in KEYBDINPUT.wScan, for the scan->vkey purposes of `NtUserSendHardwareInput` at least, and the internals can mask it after we perform the mapping.
From what I see, wineserver is extensively based on vkeys (internal state, requests, messages), so a potential transition to scan codes would indeed be non-trivial.
Actually I had something related in rbernon/wine(a)0d119e5f, could it help your case?
Thanks, in the prototype I incorporated this but removed the check for numlock, since it doesn't seem to be relevant for the conversion from vkey -> WCHAR (I verified on win10). The trade-off of moving this to win32u is that we are effectively hardcoding the conversion and any user customizations will not be applied (e.g, if a user has an XKB mapping of <numlock + numpad 5> to another symbol). Not sure how important this is, and whether the policy is to remain closer to the windows layouts rather than allowing such customizations. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5561#note_69537
From what I see, wineserver is extensively based on vkeys (internal state, requests, messages), so a potential transition to scan codes would indeed be non-trivial.
That's what I meant. Some logic could perhaps be moved to win32u but I haven't really looked at it so I'm not sure how hard it would be.
Perhaps we can just pass the full scancode directly in KEYBDINPUT.wScan, for the scan-\>vkey purposes of `NtUserSendHardwareInput` at least, and the internals can mask it after we perform the mapping.
Yeah, something like that would work. Instead of a new `map_scan_to_vkey` helper, I would rather use the existing `kbd_tables_init_vsc2vk` and make it return a UINT16 array which would include the vkey flags. Then the vkey replacement could be done on wineserver side (with the other replacements) where you have the keystate handy.
The trade-off of moving this to win32u is that we are effectively hardcoding the conversion and any user customizations will not be applied (e.g, if a user has an XKB mapping of \<numlock + numpad 5\> to another symbol). Not sure how important this is, and whether the policy is to remain closer to the windows layouts rather than allowing such customizations.
The `kbd_tables_vkey_to_wchar` hardcoding was mostly an experiment, although I think it matches `ToUnicodeEx` behavior. We can probably drop it if it doesn't end up being useful. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5561#note_69546
On Wed May 8 12:03:56 2024 +0000, Rémi Bernon wrote:
From what I see, wineserver is extensively based on vkeys (internal state, requests, messages), so a potential transition to scan codes would indeed be non-trivial. That's what I meant. Some logic could perhaps be moved to win32u but I haven't really looked at it so I'm not sure how hard it would be. Perhaps we can just pass the full scancode directly in KEYBDINPUT.wScan, for the scan-\>vkey purposes of `NtUserSendHardwareInput` at least, and the internals can mask it after we perform the mapping. Yeah, something like that would work. Instead of a new `map_scan_to_vkey` helper, I would rather use the existing `kbd_tables_init_vsc2vk` and make it return a UINT16 array which would include the vkey flags. Then the vkey replacement could be done on wineserver side (with the other replacements) where you have the keystate handy. The trade-off of moving this to win32u is that we are effectively hardcoding the conversion and any user customizations will not be applied (e.g, if a user has an XKB mapping of \<numlock + numpad 5\> to another symbol). Not sure how important this is, and whether the policy is to remain closer to the windows layouts rather than allowing such customizations. The `kbd_tables_vkey_to_wchar` hardcoding was mostly an experiment, although I think it matches `ToUnicodeEx` behavior. We can probably drop it if it doesn't end up being useful. @rbernon I have created the alternative MR https://gitlab.winehq.org/wine/wine/-/merge_requests/5601 based on the suggestions above. Thanks!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5561#note_69886
This merge request was closed by Alexandros Frantzis. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5561
The functionality was implemented in !5601 instead. Closing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5561#note_71104
participants (3)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon