winewayland.drv: Update key state from Wayland information.
Recent changes allowed the Wayland driver to properly deal with numpad keys depending on numlock status. This MR ensures that numlock and other lock state is properly synced, and concludes the fix for https://bugs.winehq.org/show_bug.cgi?id=56397.
This MR also syncs the key press state when a window gains keyboard focus, including any modifier press state. Note that we currently ignore the modifier press information from the modifier
event, since: 1. it doesn't differentiate between left-right keys, 2. mod press state changes will be normally preceded by a matching key event, so we are able to sync properly. However, if we find that we need to handle mod press state changes without corresponding key events, we will need to implement some sensible way to sync these, too.
Although I would like for the driver work exclusively in terms of scancodes, I still needed to use vkeys in this case, since this is how wineserver expresses keyboard state at the moment. This means that I had to introduce/duplicate some extra scan->vkey logic in the driver (e.g., numpad keys based on numlock state) to get things right.
Merge request reports
Activity
requested review from @rbernon
684 /* If the vkey already has the requested lock state there is nothing to do. */ 685 if (prev_lock == lock) return; 686 687 TRACE_(key)("vkey=0x%03x lock=%d state=0x%02x\n", vkey, lock, keystate[vkey]); 688 689 send_vkey(hwnd, vkey, 0); 690 send_vkey(hwnd, vkey, KEYEVENTF_KEYUP); 691 692 /* Ensure we have the proper state in case key events were blocked by hooks. */ 693 if (get_async_key_state(keystate) && !!(keystate[vkey] & 0x01) == prev_lock) 694 { 695 WARN("keystate %x not changed (%#.2x), probably blocked by hooks\n", 696 vkey, keystate[vkey]); 697 keystate[vkey] ^= 0x01; 698 set_async_key_state(keystate); 699 } 1019 if ((have_pressed_keys = keyboard->have_pressed_keys_on_enter)) 1020 { 1021 pressed_keys = keyboard->pressed_keys_on_enter; 1022 wl_array_init(&keyboard->pressed_keys_on_enter); 1023 keyboard->have_pressed_keys_on_enter = FALSE; 1024 } 922 1025 pthread_mutex_unlock(&keyboard->mutex); 923 1026 924 1027 set_current_xkb_group(xkb_group); 925 1028 1029 /* Update the modifier key press state first, so that subsequent pressed 1030 * keys will be modified accordingly and the lock state state logic, which 1031 * depends on having mod key events before modifiers, will work properly. */ 1032 if (have_pressed_keys) update_keyboard_pressed_keys(hwnd, &pressed_keys, TRUE); 926 1033 update_keyboard_lock_state(hwnd, keyboard->xkb_state); 1034 if (have_pressed_keys) update_keyboard_pressed_keys(hwnd, &pressed_keys, FALSE); 648 658 /* Skip left/right-agnostic modifier vkeys. */ 649 659 if (vkey == VK_SHIFT || vkey == VK_CONTROL || vkey == VK_MENU) continue; 650 660 651 if (state[vkey] & 0x80) 652 { 653 UINT scan = NtUserMapVirtualKeyEx(vkey, MAPVK_VK_TO_VSC_EX, 654 keyboard_hkl); 655 input.ki.wVk = vkey; 656 input.ki.wScan = scan & 0xff; 657 input.ki.dwFlags = KEYEVENTF_KEYUP; 658 if (scan & ~0xff) input.ki.dwFlags |= KEYEVENTF_EXTENDEDKEY; 659 NtUserSendHardwareInput(hwnd, 0, &input, 0); 660 } So the concern (in this and other cases) is strictly about the key events, and you would prefer if we just updated the async keyboard state?
Although that should work fine in many cases, one issue is autorepeat functionality, since it is handled (at the moment) in win32u based on key messages. We would like to:
- Stop autorepeat when a window loses focus.
- Start autorepeat of key if it is pressed when gaining focus.
(1) is critical to get right, but seems easy enough to achieve without key event involvement. If this matches Windows behavior, it would make sense to integrate a "has focus" check into the win32u repeat timer callback. At the moment it needs to happen from the driver too because we don't update window focus when handling the keyboard leave event (but also a special
NtUserSendHardwareInput
invocation with all scancodes up (see below) could deal with that in win32u).(2) seems less straightforward, since there isn't currently any feedback from the wineserver to hwnd when the async key state changes. But if we use a special
NtUserSendHardwareInput
invocation as you suggest below, perhaps wineserver can detect the scenario and do something sensible here.Hmm... after looking a bit more into it seems like I'm maybe wrong, but the trick may actually be that Windows apparently handles key repeat on the driver or hardware level. (Still, it doesn't send KEYUP on focus loss)
So, the way I've implemented it, with a process timer that repeats WM_KEYDOWN message isn't correct, and in particular it won't trigger repetitions of rawinput messages (which could cause other input issues with winewayland).
Maybe it would be better to implement it the same way as we now have pointer input repeat (see
queue_pointer_message
inserver/queue.c
), repeating input for any key that is pressed.Then, assuming input is dispatched correctly, and assuming wineserver is informed of focus loss so that the old window doesn't receive the repeated input, you could simply keep the repeating active until another window gains focus and can tell wineserver of the updated key state, clearing repeat timer or starting repeat again if any keys are still pressed.
Edited by Rémi BernonThen, assuming input is dispatched correctly, and assuming wineserver is informed of focus loss so that the old window doesn't receive the repeated input, you could simply keep the repeating active until another window gains focus and can tell wineserver of the updated key state, clearing repeat timer or starting repeat again if any keys are still pressed.
Alright, let me experiment a bit with these ideas. A couple of early thoughts:
-
Concerning giving up focus on the keyboard leave event, I remember that we don't do it because of unfortunate side-effects, but I will need to revisit to remember the details and see if/how we can make some progress here.
-
Wouldn't keeping the repeat active in wineserver cause unnecessary cpu usage? Should we effectively pause the repeat until we have a new focused window?
-
Wouldn't keeping the repeat active in wineserver cause unnecessary cpu usage? Should we effectively pause the repeat until we have a new focused window?
I don't think we need to worry about a couple of wake-ups for key input for the rare cases where keys are kept pressed on focus loss, but sure, that's a possible optimization.
Note that Windows applications can listen to input while in the background, and although that doesn't work very well for the moment, it's still supposedly the correct approach.
OTOH we also don't receive any input when no Wine window is focused, so we could consider that it's not worth trying to be correct here. We also currently kind of miss the ability to tell whether no Wine window is active or whether the desktop window really is: winex11 focuses the desktop window when all windows are losing focus, but the virtual desktop window can very well be focused on its own.
Edited by Rémi Bernon
In order to avoid having to duplicate the scan2vk mapping in the drivers, I would suggest maybe extending the
NtUserSendHardwareInput
call, for instance with a custom flag and add support for syncing the full keystate, passing the state array in lparam (and some dummy INPUT_KEYBOARD structure).You could then probably re-use the
send_hardware_message
request HID report data to pass the new vkey state to wineserver, and handle the key updates there, that would save someget_async_keystate
roundtrips.If the driver needs to pass vkeys (that's what I understood by 'state array'), it already needs to do some translation along the lines of
dlls/win32u/input.c:map_scan_to_kbd_vkey
. Perhaps we can directly send scan codes instead and win32u can perform the translation to vkey[256] itself?:NtUserSendHardwareInput(SEND_HWMSG_SCANCODES, lparam=<array of pressed scancodes>)
827 915 828 916 pthread_mutex_lock(&keyboard->mutex); 829 917 keyboard->focused_hwnd = hwnd; 918 /* Store pressed keys to be processed after we receive the modifier state. */