[PATCH v3 0/4] MR5741: Draft: server: Implement key auto-repeat.
This is an early draft of an implementation of key auto-repeat in wineserver to get some feedback. Some open questions: 1. queue_keyboard_message requires a `current` thread, but we don't get one in timeout callbacks. At the moment I am manually setting `current` to the foreground thread, but I am wondering if that's acceptable or we should explore other ways forward (also see TODO in code). 2. This draft introduces a new server request to configure auto-repeat (`enable/delay/period`). I am thinking that for more straightforward integration with the keyboard repeat SPI parameters, the request should only support the `enable` flag and the server should query the SPI registry values to get `delay` and `period` when needed. I am wondering if there any caveats here since I don't see other code in the server querying registry values (well, except to implement the registry requests themselves). Also, I would hope that opening and caching the `HKCU\Control Panel\Keyboard` hkey would remove most of the cost of performing this operation (if that's even a concern at all). -- v3: win32u: Implement keyboard auto-repeat using new server request. server: Implement key auto-repeat request. server: Check message target process for raw keyboard device flags. https://gitlab.winehq.org/wine/wine/-/merge_requests/5741
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- server/hook.c | 4 ++-- server/queue.c | 2 +- server/user.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/hook.c b/server/hook.c index 5abdf39ad37..c22e3748b8b 100644 --- a/server/hook.c +++ b/server/hook.c @@ -372,10 +372,10 @@ unsigned int get_active_hooks(void) } /* return the thread that owns the first global hook */ -struct thread *get_first_global_hook( int id ) +struct thread *get_first_global_hook( struct desktop *desktop, int id ) { struct hook *hook; - struct hook_table *global_hooks = get_global_hooks( current ); + struct hook_table *global_hooks = desktop->global_hooks; if (!global_hooks) return NULL; if (!(hook = get_first_valid_hook( global_hooks, id - WH_MINHOOK, EVENT_MIN, 0, 0, 0 ))) return NULL; diff --git a/server/queue.c b/server/queue.c index e881e40271d..2e9460e4c0a 100644 --- a/server/queue.c +++ b/server/queue.c @@ -1745,7 +1745,7 @@ static int send_hook_ll_message( struct desktop *desktop, struct message *hardwa struct message *msg; timeout_t timeout = 2000 * -10000; /* FIXME: load from registry */ - if (!(hook_thread = get_first_global_hook( id ))) return 0; + if (!(hook_thread = get_first_global_hook( desktop, id ))) return 0; if (!(queue = hook_thread->queue)) return 0; if (is_queue_hung( queue )) return 0; diff --git a/server/user.h b/server/user.h index d805a179d16..e3a14466c9d 100644 --- a/server/user.h +++ b/server/user.h @@ -103,7 +103,7 @@ extern void cleanup_clipboard_thread( struct thread *thread ); extern void remove_thread_hooks( struct thread *thread ); extern unsigned int get_active_hooks(void); -extern struct thread *get_first_global_hook( int id ); +extern struct thread *get_first_global_hook( struct desktop *desktop, int id ); /* queue functions */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5741
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- server/queue.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/server/queue.c b/server/queue.c index 2e9460e4c0a..9d9bea706c1 100644 --- a/server/queue.c +++ b/server/queue.c @@ -1723,6 +1723,15 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg free_message( msg ); return; } + + if (thread->process->rawinput_kbd && + (thread->process->rawinput_kbd->flags & RIDEV_NOLEGACY) && + get_hardware_msg_bit( msg->msg ) == QS_KEY) + { + free_message( msg ); + return; + } + input = thread->queue->input; if (win != msg->win) always_queue = 1; @@ -2091,7 +2100,6 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c unsigned int origin, struct msg_queue *sender ) { struct hw_msg_source source = { IMDT_KEYBOARD, origin }; - const struct rawinput_device *device; struct hardware_msg_data *msg_data; struct message *msg; struct thread *foreground; @@ -2207,12 +2215,6 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c release_object( foreground ); } - if ((device = current->process->rawinput_kbd) && (device->flags & RIDEV_NOLEGACY)) - { - update_input_key_state( desktop, desktop->keystate, message_code, vkey ); - return 0; - } - if (!(msg = alloc_hardware_message( input->kbd.info, source, time, 0 ))) return 0; msg_data = msg->data; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5741
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- include/wine/server_protocol.h | 23 +++++++++++++- server/protocol.def | 12 +++++++ server/queue.c | 58 ++++++++++++++++++++++++++++++++-- server/request.h | 10 ++++++ server/trace.c | 17 ++++++++++ server/user.h | 11 +++++++ server/winstation.c | 2 ++ 7 files changed, 130 insertions(+), 3 deletions(-) diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index e08f656f882..9363cbf569b 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -5638,6 +5638,24 @@ struct get_next_thread_reply }; + +struct set_keyboard_repeat_request +{ + struct request_header __header; + int enable; + int delay; + int period; +}; +struct set_keyboard_repeat_reply +{ + struct reply_header __header; + int enable; + int delay; + int period; + char __pad_20[4]; +}; + + enum request { REQ_new_process, @@ -5924,6 +5942,7 @@ enum request REQ_suspend_process, REQ_resume_process, REQ_get_next_thread, + REQ_set_keyboard_repeat, REQ_NB_REQUESTS }; @@ -6215,6 +6234,7 @@ union generic_request struct suspend_process_request suspend_process_request; struct resume_process_request resume_process_request; struct get_next_thread_request get_next_thread_request; + struct set_keyboard_repeat_request set_keyboard_repeat_request; }; union generic_reply { @@ -6504,11 +6524,12 @@ union generic_reply struct suspend_process_reply suspend_process_reply; struct resume_process_reply resume_process_reply; struct get_next_thread_reply get_next_thread_reply; + struct set_keyboard_repeat_reply set_keyboard_repeat_reply; }; /* ### protocol_version begin ### */ -#define SERVER_PROTOCOL_VERSION 803 +#define SERVER_PROTOCOL_VERSION 804 /* ### protocol_version end ### */ diff --git a/server/protocol.def b/server/protocol.def index ac103156fe4..c84a93a5d69 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3877,3 +3877,15 @@ struct handle_info @REPLY obj_handle_t handle; /* next thread handle */ @END + + +/* Setup keyboard auto-repeat */ +(a)REQ(set_keyboard_repeat) + int enable; /* whether to enable auto-repeat */ + int delay; /* auto-repeat delay in ms */ + int period; /* auto-repeat period in ms */ +(a)REPLY + int enable; /* previous state of auto-repeat enable */ + int delay; /* previous auto-repeat delay in ms */ + int period; /* previous auto-repeat period in ms */ +(a)END diff --git a/server/queue.c b/server/queue.c index 9d9bea706c1..c9c38584ed6 100644 --- a/server/queue.c +++ b/server/queue.c @@ -2095,9 +2095,20 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons return wait; } +static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, const hw_input_t *input, + unsigned int origin, struct msg_queue *sender, int repeat ); + +static void key_repeat_timeout( void *private ) +{ + struct desktop *desktop = private; + + desktop->key_repeat.timeout = NULL; + queue_keyboard_message( desktop, desktop->key_repeat.win, &desktop->key_repeat.input, IMO_HARDWARE, NULL, 1 ); +} + /* queue a hardware message for a keyboard event */ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, const hw_input_t *input, - unsigned int origin, struct msg_queue *sender ) + unsigned int origin, struct msg_queue *sender, int repeat ) { struct hw_msg_source source = { IMDT_KEYBOARD, origin }; struct hardware_msg_data *msg_data; @@ -2200,6 +2211,29 @@ static int queue_keyboard_message( struct desktop *desktop, user_handle_t win, c } } + if (origin == IMO_HARDWARE) + { + /* if the repeat key is released, stop auto-repeating */ + if (((input->kbd.flags & KEYEVENTF_KEYUP) && + (input->kbd.scan == desktop->key_repeat.input.kbd.scan))) + { + if (desktop->key_repeat.timeout) remove_timeout_user( desktop->key_repeat.timeout ); + desktop->key_repeat.timeout = NULL; + desktop->key_repeat.win = 0; + memset( &desktop->key_repeat.input, 0, sizeof(desktop->key_repeat.input) ); + } + /* if a key is down, start or continue auto-repeating */ + if (!(input->kbd.flags & KEYEVENTF_KEYUP) && desktop->key_repeat.enable) + { + timeout_t timeout = repeat ? desktop->key_repeat.period : desktop->key_repeat.delay; + desktop->key_repeat.input = *input; + desktop->key_repeat.input.kbd.time = 0; + desktop->key_repeat.win = win; + if (desktop->key_repeat.timeout) remove_timeout_user( desktop->key_repeat.timeout ); + desktop->key_repeat.timeout = add_timeout_user( timeout, key_repeat_timeout, desktop ); + } + } + if (!unicode && (foreground = get_foreground_thread( desktop, win ))) { struct rawinput_message raw_msg = {0}; @@ -2961,7 +2995,7 @@ DECL_HANDLER(send_hardware_message) wait = queue_mouse_message( desktop, req->win, &req->input, origin, sender ); break; case INPUT_KEYBOARD: - wait = queue_keyboard_message( desktop, req->win, &req->input, origin, sender ); + wait = queue_keyboard_message( desktop, req->win, &req->input, origin, sender, 0 ); break; case INPUT_HARDWARE: queue_custom_hardware_message( desktop, req->win, origin, &req->input ); @@ -3835,3 +3869,23 @@ DECL_HANDLER(update_rawinput_devices) release_object( desktop ); } } + +DECL_HANDLER(set_keyboard_repeat) +{ + struct desktop *desktop; + + if (!(desktop = get_hardware_input_desktop( 0 ))) return; + + /* report previous values */ + reply->enable = desktop->key_repeat.enable; + reply->delay = -desktop->key_repeat.delay / 10000; + reply->period = -desktop->key_repeat.period / 10000; + + /* ignore negative values to allow partial updates */ + if (req->enable >= 0) desktop->key_repeat.enable = req->enable; + if (req->delay >= 0) desktop->key_repeat.delay = -req->delay * 10000; + if (req->period >= 0) desktop->key_repeat.period = -req->period * 10000; + + if (desktop->key_repeat.timeout && !desktop->key_repeat.enable) + remove_timeout_user( desktop->key_repeat.timeout ); +} diff --git a/server/request.h b/server/request.h index 327c6168241..07cae3188fb 100644 --- a/server/request.h +++ b/server/request.h @@ -403,6 +403,7 @@ DECL_HANDLER(terminate_job); DECL_HANDLER(suspend_process); DECL_HANDLER(resume_process); DECL_HANDLER(get_next_thread); +DECL_HANDLER(set_keyboard_repeat); #ifdef WANT_REQUEST_HANDLERS @@ -693,6 +694,7 @@ static const req_handler req_handlers[REQ_NB_REQUESTS] = (req_handler)req_suspend_process, (req_handler)req_resume_process, (req_handler)req_get_next_thread, + (req_handler)req_set_keyboard_repeat, }; C_ASSERT( sizeof(abstime_t) == 8 ); @@ -2335,6 +2337,14 @@ C_ASSERT( FIELD_OFFSET(struct get_next_thread_request, flags) == 28 ); C_ASSERT( sizeof(struct get_next_thread_request) == 32 ); C_ASSERT( FIELD_OFFSET(struct get_next_thread_reply, handle) == 8 ); C_ASSERT( sizeof(struct get_next_thread_reply) == 16 ); +C_ASSERT( FIELD_OFFSET(struct set_keyboard_repeat_request, enable) == 12 ); +C_ASSERT( FIELD_OFFSET(struct set_keyboard_repeat_request, delay) == 16 ); +C_ASSERT( FIELD_OFFSET(struct set_keyboard_repeat_request, period) == 20 ); +C_ASSERT( sizeof(struct set_keyboard_repeat_request) == 24 ); +C_ASSERT( FIELD_OFFSET(struct set_keyboard_repeat_reply, enable) == 8 ); +C_ASSERT( FIELD_OFFSET(struct set_keyboard_repeat_reply, delay) == 12 ); +C_ASSERT( FIELD_OFFSET(struct set_keyboard_repeat_reply, period) == 16 ); +C_ASSERT( sizeof(struct set_keyboard_repeat_reply) == 24 ); #endif /* WANT_REQUEST_HANDLERS */ diff --git a/server/trace.c b/server/trace.c index a68577763e7..1dc3ac31b27 100644 --- a/server/trace.c +++ b/server/trace.c @@ -4590,6 +4590,20 @@ static void dump_get_next_thread_reply( const struct get_next_thread_reply *req fprintf( stderr, " handle=%04x", req->handle ); } +static void dump_set_keyboard_repeat_request( const struct set_keyboard_repeat_request *req ) +{ + fprintf( stderr, " enable=%d", req->enable ); + fprintf( stderr, ", delay=%d", req->delay ); + fprintf( stderr, ", period=%d", req->period ); +} + +static void dump_set_keyboard_repeat_reply( const struct set_keyboard_repeat_reply *req ) +{ + fprintf( stderr, " enable=%d", req->enable ); + fprintf( stderr, ", delay=%d", req->delay ); + fprintf( stderr, ", period=%d", req->period ); +} + static const dump_func req_dumpers[REQ_NB_REQUESTS] = { (dump_func)dump_new_process_request, (dump_func)dump_get_new_process_info_request, @@ -4875,6 +4889,7 @@ static const dump_func req_dumpers[REQ_NB_REQUESTS] = { (dump_func)dump_suspend_process_request, (dump_func)dump_resume_process_request, (dump_func)dump_get_next_thread_request, + (dump_func)dump_set_keyboard_repeat_request, }; static const dump_func reply_dumpers[REQ_NB_REQUESTS] = { @@ -5162,6 +5177,7 @@ static const dump_func reply_dumpers[REQ_NB_REQUESTS] = { NULL, NULL, (dump_func)dump_get_next_thread_reply, + (dump_func)dump_set_keyboard_repeat_reply, }; static const char * const req_names[REQ_NB_REQUESTS] = { @@ -5449,6 +5465,7 @@ static const char * const req_names[REQ_NB_REQUESTS] = { "suspend_process", "resume_process", "get_next_thread", + "set_keyboard_repeat", }; static const struct diff --git a/server/user.h b/server/user.h index e3a14466c9d..0a79897916d 100644 --- a/server/user.h +++ b/server/user.h @@ -64,6 +64,16 @@ struct global_cursor user_handle_t win; /* window that contains the cursor */ }; +struct key_repeat +{ + int enable; /* enable auto-repeat */ + timeout_t delay; /* auto-repeat delay */ + timeout_t period; /* auto-repeat period */ + hw_input_t input; /* the input to repeat */ + user_handle_t win; /* target window for input event */ + struct timeout_user *timeout; /* timeout for repeat */ +}; + struct desktop { struct object obj; /* object header */ @@ -82,6 +92,7 @@ struct desktop unsigned int users; /* processes and threads using this desktop */ struct global_cursor cursor; /* global cursor information */ unsigned char keystate[256]; /* asynchronous key state */ + struct key_repeat key_repeat; /* key auto-repeat */ }; /* user handles functions */ diff --git a/server/winstation.c b/server/winstation.c index 80126ad5d60..66fb6ad1e35 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -294,6 +294,7 @@ static struct desktop *create_desktop( const struct unicode_str *name, unsigned list_init( &desktop->threads ); memset( &desktop->cursor, 0, sizeof(desktop->cursor) ); memset( desktop->keystate, 0, sizeof(desktop->keystate) ); + memset( &desktop->key_repeat, 0, sizeof(desktop->key_repeat) ); list_add_tail( &winstation->desktops, &desktop->entry ); list_init( &desktop->hotkeys ); list_init( &desktop->pointers ); @@ -365,6 +366,7 @@ static void desktop_destroy( struct object *obj ) if (desktop->msg_window) free_window_handle( desktop->msg_window ); if (desktop->global_hooks) release_object( desktop->global_hooks ); if (desktop->close_timeout) remove_timeout_user( desktop->close_timeout ); + if (desktop->key_repeat.timeout) remove_timeout_user( desktop->key_repeat.timeout ); release_object( desktop->winstation ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5741
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> --- dlls/win32u/message.c | 58 ------------------------------------ dlls/win32u/ntuser_private.h | 5 ---- dlls/win32u/sysparams.c | 34 +++++++++++++++++++-- 3 files changed, 32 insertions(+), 65 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 877be545810..6b89b873f37 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -288,7 +288,6 @@ struct send_message_info }; static const INPUT_MESSAGE_SOURCE msg_source_unavailable = { IMDT_UNAVAILABLE, IMO_UNAVAILABLE }; -static BOOL keyboard_auto_repeat_enabled; /* flag for messages that contain pointers */ /* 32 messages per entry, messages 0..31 map to bits 0..31 */ @@ -516,13 +515,6 @@ static inline BOOL check_hwnd_filter( const MSG *msg, HWND hwnd_filter ) return (msg->hwnd == hwnd_filter || is_child( hwnd_filter, msg->hwnd )); } -BOOL set_keyboard_auto_repeat( BOOL enable ) -{ - BOOL enabled = keyboard_auto_repeat_enabled; - keyboard_auto_repeat_enabled = enable; - return enabled; -} - /*********************************************************************** * unpack_message * @@ -2309,21 +2301,6 @@ static void send_parent_notify( HWND hwnd, WORD event, WORD idChild, POINT pt ) } } - -static void handle_keyboard_repeat_message( HWND hwnd ) -{ - struct user_thread_info *thread_info = get_user_thread_info(); - MSG *msg = &thread_info->key_repeat_msg; - UINT speed; - - msg->lParam = (msg->lParam & ~(LPARAM)0xffff) + ((msg->lParam + 1) & 0xffff); - - if (NtUserSystemParametersInfo( SPI_GETKEYBOARDSPEED, 0, &speed, 0 )) - NtUserSetSystemTimer( hwnd, SYSTEM_TIMER_KEY_REPEAT, 400 / (speed + 1) ); - - NtUserPostMessage( hwnd, msg->message, msg->wParam, msg->lParam ); -} - /*********************************************************************** * process_pointer_message * @@ -2416,37 +2393,6 @@ static BOOL process_keyboard_message( MSG *msg, UINT hw_id, HWND hwnd_filter, if (ImmProcessKey( msg->hwnd, NtUserGetKeyboardLayout(0), msg->wParam, msg->lParam, 0 )) msg->wParam = VK_PROCESSKEY; - /* set/kill timers for key auto-repeat */ - if (remove && keyboard_auto_repeat_enabled) - { - struct user_thread_info *thread_info = get_user_thread_info(); - - switch (msg->message) - { - case WM_KEYDOWN: - case WM_SYSKEYDOWN: - { - UINT delay; - - if (msg->wParam == VK_PROCESSKEY) break; - - if (thread_info->key_repeat_msg.hwnd != msg->hwnd) - kill_system_timer( thread_info->key_repeat_msg.hwnd, SYSTEM_TIMER_KEY_REPEAT ); - thread_info->key_repeat_msg = *msg; - if (NtUserSystemParametersInfo( SPI_GETKEYBOARDDELAY, 0, &delay, 0 )) - NtUserSetSystemTimer( msg->hwnd, SYSTEM_TIMER_KEY_REPEAT, (delay + 1) * 250 ); - break; - } - - case WM_KEYUP: - case WM_SYSKEYUP: - /* Only stop repeat if the scan codes match. */ - if ((thread_info->key_repeat_msg.lParam & 0x01ff0000) == (msg->lParam & 0x01ff0000)) - kill_system_timer( thread_info->key_repeat_msg.hwnd, SYSTEM_TIMER_KEY_REPEAT ); - break; - } - } - return TRUE; } @@ -3655,10 +3601,6 @@ LRESULT WINAPI NtUserDispatchMessage( const MSG *msg ) case SYSTEM_TIMER_TRACK_MOUSE: update_mouse_tracking_info( msg->hwnd ); return 0; - - case SYSTEM_TIMER_KEY_REPEAT: - handle_keyboard_repeat_message( msg->hwnd ); - return 0; } } diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index f0bd14179d4..020c98005df 100644 --- a/dlls/win32u/ntuser_private.h +++ b/dlls/win32u/ntuser_private.h @@ -33,9 +33,6 @@ enum system_timer_id { SYSTEM_TIMER_TRACK_MOUSE = 0xfffa, SYSTEM_TIMER_CARET = 0xffff, - - /* not compatible with native */ - SYSTEM_TIMER_KEY_REPEAT = 0xfff0, }; struct user_object @@ -120,7 +117,6 @@ struct user_thread_info struct received_message_info *receive_info; /* Message being currently received */ struct user_key_state_info *key_state; /* Cache of global key state */ struct imm_thread_data *imm_thread_data; /* IMM thread data */ - MSG key_repeat_msg; /* Last WM_KEYDOWN message to repeat */ HKL kbd_layout; /* Current keyboard layout */ UINT kbd_layout_id; /* Current keyboard layout ID */ struct hardware_msg_data *rawinput; /* Current rawinput message data */ @@ -249,7 +245,6 @@ struct peek_message_filter }; extern int peek_message( MSG *msg, const struct peek_message_filter *filter ); -extern BOOL set_keyboard_auto_repeat( BOOL enable ); /* systray.c */ extern LRESULT system_tray_call( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam, void *data ); diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 49bf26214e7..686982dc3e5 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -3744,6 +3744,25 @@ BOOL WINAPI NtUserPerMonitorDPIPhysicalToLogicalPoint( HWND hwnd, POINT *pt ) return ret; } +/* Set server auto-repeat properties. delay and speed are expressed in terms of + * SPI_KEYBOARDDELAY and SPI_KEYBOARDSPEED values. Returns whether auto-repeat + * was enabled before this request. */ +static BOOL set_server_keyboard_repeat( int enable, int delay, int speed ) +{ + BOOL enabled = FALSE; + + SERVER_START_REQ( set_keyboard_repeat ) + { + req->enable = enable >= 0 ? (enable > 0) : -1; + req->delay = delay >= 0 ? (delay + 1) * 250 : -1; + req->period = speed >= 0 ? 400 / (speed + 1) : -1; + if (!wine_server_call( req )) enabled = reply->enable; + } + SERVER_END_REQ; + + return enabled; +} + /* retrieve the cached base keys for a given entry */ static BOOL get_base_keys( enum parameter_key index, HKEY *base_key, HKEY *volatile_key ) { @@ -4974,7 +4993,8 @@ BOOL WINAPI NtUserSystemParametersInfo( UINT action, UINT val, void *ptr, UINT w break; case SPI_SETKEYBOARDSPEED: if (val > 31) val = 31; - ret = set_entry( &entry_KEYBOARDSPEED, val, ptr, winini ); + if ((ret = set_entry( &entry_KEYBOARDSPEED, val, ptr, winini ))) + set_server_keyboard_repeat( -1, -1, val ); break; WINE_SPI_WARN(SPI_LANGDRIVER); /* not implemented in Windows */ @@ -5018,7 +5038,8 @@ BOOL WINAPI NtUserSystemParametersInfo( UINT action, UINT val, void *ptr, UINT w ret = get_entry( &entry_KEYBOARDDELAY, val, ptr ); break; case SPI_SETKEYBOARDDELAY: - ret = set_entry( &entry_KEYBOARDDELAY, val, ptr, winini ); + if ((ret = set_entry( &entry_KEYBOARDDELAY, val, ptr, winini ))) + set_server_keyboard_repeat( -1, val, -1 ); break; case SPI_ICONVERTICALSPACING: if (ptr != NULL) @@ -6272,6 +6293,15 @@ static void thread_detach(void) exiting_thread_id = 0; } +static BOOL set_keyboard_auto_repeat( BOOL enable ) +{ + UINT delay, speed; + + get_entry( &entry_KEYBOARDDELAY, 0, &delay ); + get_entry( &entry_KEYBOARDSPEED, 0, &speed ); + return set_server_keyboard_repeat( enable, delay, speed ); +} + /*********************************************************************** * NtUserCallNoParam (win32u.@) */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5741
On Wed Jun 5 13:32:04 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/5741/diffs?diff_id=116686&start_sha=a6b446f47be6842195cd81bd48e2ec9159482f18#b4e51a57fda93554e5c008a209f92ef67bd4d777_3240_3168) Removed the tests in v3.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5741#note_72463
On Wed Jun 5 13:32:06 2024 +0000, Alexandros Frantzis wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/5741/diffs?diff_id=116686&start_sha=a6b446f47be6842195cd81bd48e2ec9159482f18#61204a3bfaf089b3588b75ee6440092f482a6098_883_886) Done in v3. Now the reply of the server request (returning previous values) is less useful, but have kept it for completeness.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5741#note_72462
v3: * Move RIDEV_NOLEGACY check (for keyboard) to `queue_hardware_input` * Removed cross-process RIDEV_NOLEGACY tests. * Implement win32u auto-repeat functionality using new server request. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5741#note_72465
A manual test shows that hotkeys messages are supposed to be received even with that flag, so basically moving the NOLEGACY check right after find_hardware_message_window looks like a simple fix and goes in the right direction.
Done in v3. I considered adding tests for hotkey/rawinput interactions, but found that we emit WM_HOTKEY and WM_INPUT messages in the opposite order from Windows, so skipped them for now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5741#note_72464
Rémi Bernon (@rbernon) commented about server/queue.c:
free_message( msg ); return; } + + if (thread->process->rawinput_kbd && + (thread->process->rawinput_kbd->flags & RIDEV_NOLEGACY) && + get_hardware_msg_bit( msg->msg ) == QS_KEY) + { + free_message( msg ); + return; + }
I think the tests were showing that NOLEGACY should cause the thread keystate to not reflect the key presses (because there's no window message), but here we still update it if for some reason we couldn't find a window. What about something like that instead? You could also probably remove the same check from queue_mouse_message while at it. ```suggestion:-14+0 static unsigned int get_rawinput_device_flags( struct process *process, struct message *msg ) { switch (get_hardware_msg_bit( msg->msg )) { case QS_KEY: return process->rawinput_kdb ? process->rawinput_kdb->flags : 0; case QS_MOUSEMOVE: case QS_MOUSEBUTTON: return process->rawinput_mouse ? process->rawinput_mouse->flags : 0; } return 0; } /* ... */ win = find_hardware_message_window( desktop, input, msg, &msg_code, &thread ); flags = thread ? get_rawinput_device_flags( thread->process, msg ) : 0; if (!win || !thread || (flags & RIDEV_NOLEGACY)) { if (input && !(flags & RIDEV_NOLEGACY)) update_input_key_state( input->desktop, input->keystate, msg->msg, msg->wparam ); free_message( msg ); return; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5741#note_72477
Rémi Bernon (@rbernon) commented about server/queue.c:
+ struct desktop *desktop; + + if (!(desktop = get_hardware_input_desktop( 0 ))) return; + + /* report previous values */ + reply->enable = desktop->key_repeat.enable; + reply->delay = -desktop->key_repeat.delay / 10000; + reply->period = -desktop->key_repeat.period / 10000; + + /* ignore negative values to allow partial updates */ + if (req->enable >= 0) desktop->key_repeat.enable = req->enable; + if (req->delay >= 0) desktop->key_repeat.delay = -req->delay * 10000; + if (req->period >= 0) desktop->key_repeat.period = -req->period * 10000; + + if (desktop->key_repeat.timeout && !desktop->key_repeat.enable) + remove_timeout_user( desktop->key_repeat.timeout ); You're leaking the desktop here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5741#note_72479
Rémi Bernon (@rbernon) commented about server/protocol.def:
@REPLY obj_handle_t handle; /* next thread handle */ @END + + +/* Setup keyboard auto-repeat */ +(a)REQ(set_keyboard_repeat) + int enable; /* whether to enable auto-repeat */ + int delay; /* auto-repeat delay in ms */ + int period; /* auto-repeat period in ms */ +(a)REPLY + int enable; /* previous state of auto-repeat enable */ + int delay; /* previous auto-repeat delay in ms */ + int period; /* previous auto-repeat period in ms */ If it's not going to be useful in the short term (and as this MR don't use them I would say they aren't) I think it's better to remove the return values and only add them when it will be.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5741#note_72478
Rémi Bernon (@rbernon) commented about server/queue.c:
release_object( desktop ); } } + +DECL_HANDLER(set_keyboard_repeat) +{ + struct desktop *desktop; + + if (!(desktop = get_hardware_input_desktop( 0 ))) return;
This should probably instead be the desktop associated with the caller thread / process. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5741#note_72480
participants (3)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Rémi Bernon