[PATCH v2 0/2] MR9246: winemac: Use event handle to detect macdrv ime event processing done.
Since !9128, macdrv ime processing has been broken. In my opinion, this seems to be an issue with the macdrv ime code rather than a problem with !9128. In macdrv_ImeProcessKey: ``` macdrv_send_keydown_to_input_source(flags, repeat, keyc, himc, &done); while (!done) NtUserMsgWaitForMultipleObjectsEx(0, NULL, INFINITE, QS_POSTMESSAGE | QS_SENDMESSAGE, 0); ``` Within the macdrv WINE_IME_POST_UPDATE code path, there are no message post/send operations, causing it to wait infinitely here. I haven't analyzed in detail why it worked before, but I guess it escaped the wait due to unrelated messages. This code likely represents a remnant of the old macdrv ime code. I recall the old macdrv ime code used SendMessage(). In ime processing, some key inputs reach macdrv_sent_text_input without calling macdrv_ime_set_text. Therefore, I modified the code to clearly detect when macdrv_sent_text_input is reached. I left a working fallback code in case NtCreateEvent() fails. But that might be superfluous. -- v2: win32u: Fix typo. winemac: Use event handle to detect macdrv ime processing done. https://gitlab.winehq.org/wine/wine/-/merge_requests/9246
From: Byeongsik Jeon <bsjeon(a)hanmail.net> --- dlls/winemac.drv/cocoa_window.m | 6 ++++-- dlls/winemac.drv/event.c | 4 ++++ dlls/winemac.drv/keyboard.c | 13 +++++++++++-- dlls/winemac.drv/macdrv.h | 1 + dlls/winemac.drv/macdrv_cocoa.h | 5 +++-- dlls/winemac.drv/macdrv_main.c | 4 ++++ 6 files changed, 27 insertions(+), 6 deletions(-) diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 326b5925c5f..2dd7d180c24 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -4034,13 +4034,14 @@ uint32_t macdrv_window_background_color(void) } /*********************************************************************** - * macdrv_send_keydown_to_input_source + * macdrv_ime_process_key * * Sends a key down event to the active window's inputContext so that it can be * processed by input sources (AKA IMEs). This is only called when there is an * active non-keyboard input source. */ -void macdrv_send_keydown_to_input_source(unsigned int flags, int repeat, int keyc, void* himc, int* done) +void macdrv_ime_process_key(int keyc, unsigned int flags, int repeat, void *himc, + int *done, void *ime_done_event) { OnMainThreadAsync(^{ BOOL ret; @@ -4080,6 +4081,7 @@ void macdrv_send_keydown_to_input_source(unsigned int flags, int repeat, int key event = macdrv_create_event(SENT_TEXT_INPUT, window); event->sent_text_input.handled = ret; event->sent_text_input.done = done; + event->sent_text_input.ime_done_event = ime_done_event; [[window queue] postEvent:event]; macdrv_release_event(event); }); diff --git a/dlls/winemac.drv/event.c b/dlls/winemac.drv/event.c index b97b876cb70..6841fd3603e 100644 --- a/dlls/winemac.drv/event.c +++ b/dlls/winemac.drv/event.c @@ -190,8 +190,12 @@ static void macdrv_im_set_text(const macdrv_event *event) */ static void macdrv_sent_text_input(const macdrv_event *event) { + HANDLE ime_done_event = event->sent_text_input.ime_done_event; + TRACE_(imm)("handled: %s\n", event->sent_text_input.handled ? "TRUE" : "FALSE"); + *event->sent_text_input.done = event->sent_text_input.handled ? 1 : -1; + if (ime_done_event) NtSetEvent(ime_done_event, NULL); } diff --git a/dlls/winemac.drv/keyboard.c b/dlls/winemac.drv/keyboard.c index 645d082b166..a9c9e2d8f30 100644 --- a/dlls/winemac.drv/keyboard.c +++ b/dlls/winemac.drv/keyboard.c @@ -1139,8 +1139,17 @@ UINT macdrv_ImeProcessKey(HIMC himc, UINT wparam, UINT lparam, const BYTE *key_s TRACE("flags 0x%08x keyc 0x%04x\n", flags, keyc); - macdrv_send_keydown_to_input_source(flags, repeat, keyc, himc, &done); - while (!done) NtUserMsgWaitForMultipleObjectsEx(0, NULL, INFINITE, QS_POSTMESSAGE | QS_SENDMESSAGE, 0); + if (!thread_data->ime_done_event) + { + NTSTATUS status; + status = NtCreateEvent(&thread_data->ime_done_event, EVENT_ALL_ACCESS, NULL, + SynchronizationEvent, FALSE); + if (status != STATUS_SUCCESS) WARN("NtCreateEvent call failed.\n"); + } + + macdrv_ime_process_key(keyc, flags, repeat, himc, &done, thread_data->ime_done_event); + while (!done) + NtUserMsgWaitForMultipleObjectsEx(1, &thread_data->ime_done_event, INFINITE, 0, 0); return done > 0; } diff --git a/dlls/winemac.drv/macdrv.h b/dlls/winemac.drv/macdrv.h index 22e96fa1129..5b9515f086f 100644 --- a/dlls/winemac.drv/macdrv.h +++ b/dlls/winemac.drv/macdrv.h @@ -112,6 +112,7 @@ static inline RECT rect_from_cgrect(CGRect cgrect) HKL active_keyboard_layout; WORD keyc2vkey[128]; WORD keyc2scan[128]; + HANDLE ime_done_event; }; extern struct macdrv_thread_data *macdrv_init_thread_data(void); diff --git a/dlls/winemac.drv/macdrv_cocoa.h b/dlls/winemac.drv/macdrv_cocoa.h index f91e9f9f169..bd6af9ed25e 100644 --- a/dlls/winemac.drv/macdrv_cocoa.h +++ b/dlls/winemac.drv/macdrv_cocoa.h @@ -381,6 +381,7 @@ extern int macdrv_set_display_mode(const struct macdrv_display* display, struct { int handled; int *done; + void *ime_done_event; } sent_text_input; struct { macdrv_status_item item; @@ -545,8 +546,8 @@ extern void macdrv_order_cocoa_window(macdrv_window w, macdrv_window prev, extern int macdrv_get_view_backing_size(macdrv_view v, int backing_size[2]); extern void macdrv_set_view_backing_size(macdrv_view v, const int backing_size[2]); extern uint32_t macdrv_window_background_color(void); -extern void macdrv_send_keydown_to_input_source(unsigned int flags, int repeat, int keyc, - void* data, int* done); +extern void macdrv_ime_process_key(int keyc, unsigned int flags, int repeat, void *data, + int *done, void *ime_done_event); extern int macdrv_is_any_wine_window_visible(void); diff --git a/dlls/winemac.drv/macdrv_main.c b/dlls/winemac.drv/macdrv_main.c index 20295d1c6ee..f5e2b5e08d2 100644 --- a/dlls/winemac.drv/macdrv_main.c +++ b/dlls/winemac.drv/macdrv_main.c @@ -464,6 +464,8 @@ void macdrv_ThreadDetach(void) macdrv_destroy_event_queue(data->queue); if (data->keyboard_layout_uchr) CFRelease(data->keyboard_layout_uchr); + if (data->ime_done_event) + NtClose(data->ime_done_event); free(data); /* clear data in case we get re-entered from user32 before the thread is truly dead */ NtUserGetThreadInfo()->driver_data = 0; @@ -523,6 +525,8 @@ struct macdrv_thread_data *macdrv_init_thread_data(void) NtTerminateProcess(0, 1); } + data->ime_done_event = NULL; + macdrv_get_input_source_info(&data->keyboard_layout_uchr, &data->keyboard_type, &data->iso_keyboard, &input_source); data->active_keyboard_layout = macdrv_get_hkl_from_source(input_source); CFRelease(input_source); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9246
From: Byeongsik Jeon <bsjeon(a)hanmail.net> --- dlls/win32u/imm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/win32u/imm.c b/dlls/win32u/imm.c index 8b416a654d9..3337f66e291 100644 --- a/dlls/win32u/imm.c +++ b/dlls/win32u/imm.c @@ -683,7 +683,7 @@ LRESULT ime_driver_call( HWND hwnd, enum wine_ime_call call, WPARAM wparam, LPAR res = TRUE; } - TRACE( "processing scan %#x, vkey %#x -> %lu\n", LOWORD(wparam), HIWORD(lparam), res ); + TRACE( "processing vkey %#x, scan %#x -> %lu\n", LOWORD(wparam), HIWORD(lparam), res ); return res; } case WINE_IME_TO_ASCII_EX: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9246
On Wed Oct 22 06:10:36 2025 +0000, Rémi Bernon wrote:
I don't think we really need to worry about the event failing to be created, the wait won't work otherwise anyway. ```suggestion:-4+0 NtUserMsgWaitForMultipleObjectsEx(1, &thread_data->ime_done_event, INFINITE, 0, 0); ``` Thank you. I've modified it to check the done value when event creation fails.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9246#note_119281
On Wed Oct 22 07:23:40 2025 +0000, Byeongsik Jeon wrote:
Thank you. I've modified it to check the done value when event creation fails. I don't think we need the while anymore, do we?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9246#note_119284
On Wed Oct 22 07:35:54 2025 +0000, Rémi Bernon wrote:
I don't think we need the while anymore, do we? If there is no 'while(!done)' loop when event creation fails, macdrv_ImeProcessKey may terminate before ime_sent_text_input is called. I simulated the failure, but it malfunctioned.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9246#note_119285
On Wed Oct 22 07:49:39 2025 +0000, Byeongsik Jeon wrote:
If there is no 'while(!done)' loop when event creation fails, macdrv_ImeProcessKey may terminate before ime_sent_text_input is called. I simulated the failure, but it malfunctioned. If the event has been created, it exits the while loop with a single call to NtUserMsgWaitForMultipleObjectsEx.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9246#note_119286
On Wed Oct 22 08:00:13 2025 +0000, Byeongsik Jeon wrote:
If the event has been created, it exits the while loop with a single call to NtUserMsgWaitForMultipleObjectsEx. Yes but I don't think we need to care about the case where the event fails to be created. The warning message is fine, just in case it happens (it could even be an ERR to be more visible), but if it happens it means something very wrong is going on and the key processing failure is the least of our problems.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9246#note_119287
On Wed Oct 22 08:07:35 2025 +0000, Rémi Bernon wrote:
Yes but I don't think we need to care about the case where the event fails to be created. The warning message is fine, just in case it happens (it could even be an ERR to be more visible), but if it happens it means something very wrong is going on and the key processing failure is the least of our problems. Understood. I'll update it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9246#note_119288
participants (3)
-
Byeongsik Jeon -
Byeongsik Jeon (＠bsjeon)
-
Rémi Bernon