[PATCH v9 0/4] MR10007: Support enabling/disabling IME on winewayland
Currently, IME is always enabled for all applications. This can cause some very annoying issues on winewayland. For example, fcitx takes over the character repeat functionality when enabled, and does it via repeatedly sending sequences of keyup/keydown, which leads to applications not being able to track hold duration of a button (can be reproduced on any game which requires holding a letter key). The spec states that the enable request should only be sent when a text edit is focused, and a disable request should be sent when it is unfocused. Windows provides a "similar-ish" mechanism: applications can use ImmAssociateContext/ImmAssociateContextEx to associate an IMC when IME should be enabled, and set associated IMC to null when it should be disabled. From my rather limited testing, it is implemented this way in Chromium/CEF, QT and Unity. Setting it to null internally results in deactivation of the currently associated context. This MR makes the ImeSetActiveContext function of the builtin IME utilize a new driver function to communicate activation status. It does not take into account which specific context was activated, thus semi-stub. This fixes behavior of applications using the above-mentioned components. I only implemented it for winewayland, it probably should also be implemented for winex11 too, but I'm not sure how. ~~Unfortunately this breaks IME input in Wine comctl32/user32 components. I am not sure how it is supposed to behave on Windows or how to test it at the current point, will look into it later. Having IME on by default appears to "fix" it, but that is very likely not the Windows behavior; moreover, at least Unity does not disable IME on start, which causes the above-mentioned issue to occur until a textbox is manually focused and unfocused.~~ Fixed -- v9: imm32/tests: Test which IME receives ImeSetActiveContext deactivation call after layout change https://gitlab.winehq.org/wine/wine/-/merge_requests/10007
From: Ziia Shi <mkrsym1@gmail.com> --- dlls/win32u/driver.c | 12 ++++++++++++ dlls/win32u/input.c | 5 +++++ dlls/win32u/sysparams.c | 3 +++ dlls/win32u/win32u_private.h | 1 + include/ntuser.h | 1 + include/wine/gdi_driver.h | 3 ++- 6 files changed, 24 insertions(+), 1 deletion(-) diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index 60aba702d1a..42f4a11175d 100644 --- a/dlls/win32u/driver.c +++ b/dlls/win32u/driver.c @@ -685,6 +685,11 @@ static BOOL nulldrv_SetIMECompositionRect( HWND hwnd, RECT rect ) return FALSE; } +static BOOL nulldrv_SetIMEEnabled( HWND hwnd, BOOL enabled ) +{ + return FALSE; +} + static LRESULT nulldrv_DesktopWindowProc( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam ) { return default_window_proc( hwnd, msg, wparam, lparam, FALSE ); @@ -1113,6 +1118,11 @@ static BOOL loaderdrv_SetIMECompositionRect( HWND hwnd, RECT rect ) return load_driver()->pSetIMECompositionRect( hwnd, rect ); } +static BOOL loaderdrv_SetIMEEnabled( HWND hwnd, BOOL enabled ) +{ + return load_driver()->pSetIMEEnabled( hwnd, enabled ); +} + static LONG loaderdrv_ChangeDisplaySettings( LPDEVMODEW displays, LPCWSTR primary_name, HWND hwnd, DWORD flags, LPVOID lparam ) { @@ -1258,6 +1268,7 @@ static const struct user_driver_funcs lazy_load_driver = loaderdrv_ImeProcessKey, loaderdrv_NotifyIMEStatus, loaderdrv_SetIMECompositionRect, + loaderdrv_SetIMEEnabled, /* cursor/icon functions */ nulldrv_DestroyCursorIcon, loaderdrv_SetCursor, @@ -1362,6 +1373,7 @@ void __wine_set_user_driver( const struct user_driver_funcs *funcs, UINT version SET_USER_FUNC(ImeProcessKey); SET_USER_FUNC(NotifyIMEStatus); SET_USER_FUNC(SetIMECompositionRect); + SET_USER_FUNC(SetIMEEnabled); SET_USER_FUNC(DestroyCursorIcon); SET_USER_FUNC(SetCursor); SET_USER_FUNC(GetCursorPos); diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index c86a357ab14..f3fa1d757ff 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -2467,6 +2467,11 @@ BOOL set_ime_composition_rect( HWND hwnd, RECT rect ) return user_driver->pSetIMECompositionRect( NtUserGetAncestor( hwnd, GA_ROOT ), rect ); } +BOOL set_ime_enabled( HWND hwnd, BOOL enabled ) +{ + return user_driver->pSetIMEEnabled( hwnd, enabled ); +} + /***************************************************************** * NtUserSetCaretPos (win32u.@) */ diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index 945b03c0b15..0a957802076 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -7607,6 +7607,9 @@ ULONG_PTR WINAPI NtUserCallTwoParam( ULONG_PTR arg1, ULONG_PTR arg2, ULONG code case NtUserCallTwoParam_SetIMECompositionRect: return set_ime_composition_rect( UlongToHandle(arg1), *(const RECT *)arg2 ); + case NtUserCallTwoParam_SetIMEEnabled: + return set_ime_enabled( UlongToHandle(arg1), arg2 ); + case NtUserCallTwoParam_AdjustWindowRect: { struct adjust_window_rect_params *params = (void *)arg2; diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index 1024fb71c26..85c2592b575 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -103,6 +103,7 @@ extern BOOL set_capture_window( HWND hwnd, UINT gui_flags, HWND *prev_ret ); extern BOOL set_foreground_window( HWND hwnd, BOOL mouse, BOOL force ); extern BOOL set_active_window( HWND hwnd, HWND *prev, BOOL mouse, BOOL focus, DWORD new_active_thread_id ); extern BOOL set_ime_composition_rect( HWND hwnd, RECT rect ); +extern BOOL set_ime_enabled( HWND hwnd, BOOL enabled ); extern void toggle_caret( HWND hwnd ); extern void update_mouse_tracking_info( HWND hwnd ); extern void update_current_mouse_window( HWND hwnd, INT hittest, POINT pos ); diff --git a/include/ntuser.h b/include/ntuser.h index ce1b18fdfd9..84caec08efb 100644 --- a/include/ntuser.h +++ b/include/ntuser.h @@ -1217,6 +1217,7 @@ enum NtUserCallTwoParam_MonitorFromRect, NtUserCallTwoParam_SetIconParam, NtUserCallTwoParam_SetIMECompositionRect, + NtUserCallTwoParam_SetIMEEnabled, NtUserCallTwoParam_AdjustWindowRect, NtUserCallTwoParam_GetVirtualScreenRect, /* temporary exports */ diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h index f6390bce878..07e8d160691 100644 --- a/include/wine/gdi_driver.h +++ b/include/wine/gdi_driver.h @@ -218,7 +218,7 @@ struct gdi_dc_funcs }; /* increment this when you change the DC function table */ -#define WINE_GDI_DRIVER_VERSION 108 +#define WINE_GDI_DRIVER_VERSION 109 #define GDI_PRIORITY_NULL_DRV 0 /* null driver */ #define GDI_PRIORITY_FONT_DRV 100 /* any font driver */ @@ -375,6 +375,7 @@ struct user_driver_funcs UINT (*pImeProcessKey)(HIMC,UINT,UINT,const BYTE*); void (*pNotifyIMEStatus)(HWND,UINT); BOOL (*pSetIMECompositionRect)(HWND,RECT); + BOOL (*pSetIMEEnabled)(HWND,BOOL); /* cursor/icon functions */ void (*pDestroyCursorIcon)(HCURSOR); void (*pSetCursor)(HWND,HCURSOR); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10007
From: Ziia Shi <mkrsym1@gmail.com> --- dlls/imm32/ime.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dlls/imm32/ime.c b/dlls/imm32/ime.c index 65f46f20550..e7ac5ad8661 100644 --- a/dlls/imm32/ime.c +++ b/dlls/imm32/ime.c @@ -505,8 +505,16 @@ BOOL WINAPI ImeSelect( HIMC himc, BOOL select ) BOOL WINAPI ImeSetActiveContext( HIMC himc, BOOL flag ) { - TRACE( "himc %p, flag %#x stub!\n", himc, flag ); - return TRUE; + INPUTCONTEXT *ctx; + ULONG_PTR ret; + + TRACE( "himc %p, flag %#x semi-stub!\n", himc, flag ); + + if (!(ctx = ImmLockIMC( himc ))) return FALSE; + ret = NtUserCallTwoParam( (ULONG_PTR)ctx->hWnd, (ULONG_PTR)flag, NtUserCallTwoParam_SetIMEEnabled ); + ImmUnlockIMC( himc ); + + return ret; } BOOL WINAPI ImeProcessKey( HIMC himc, UINT vkey, LPARAM lparam, BYTE *state ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10007
From: Ziia Shi <mkrsym1@gmail.com> --- dlls/winewayland.drv/wayland_text_input.c | 93 ++++++++++++++++++++--- dlls/winewayland.drv/waylanddrv.h | 4 + dlls/winewayland.drv/waylanddrv_main.c | 1 + dlls/winewayland.drv/window.c | 2 + 4 files changed, 89 insertions(+), 11 deletions(-) diff --git a/dlls/winewayland.drv/wayland_text_input.c b/dlls/winewayland.drv/wayland_text_input.c index 52cfef27e50..0bbab4e75c4 100644 --- a/dlls/winewayland.drv/wayland_text_input.c +++ b/dlls/winewayland.drv/wayland_text_input.c @@ -77,10 +77,40 @@ static void wayland_text_input_reset_all_state(struct wayland_text_input *text_i wayland_text_input_reset_pending_state(text_input); } +static void wayland_text_input_ime_enable(struct wayland_text_input *text_input) +{ + TRACE("text_input->enabled %u\n", text_input->enabled); + + if (!text_input->enabled) + { + zwp_text_input_v3_enable(text_input->zwp_text_input_v3); + zwp_text_input_v3_set_content_type(text_input->zwp_text_input_v3, + ZWP_TEXT_INPUT_V3_CONTENT_HINT_NONE, + ZWP_TEXT_INPUT_V3_CONTENT_PURPOSE_NORMAL); + zwp_text_input_v3_set_cursor_rectangle(text_input->zwp_text_input_v3, 0, 0, 0, 0); + zwp_text_input_v3_commit(text_input->zwp_text_input_v3); + text_input->enabled = TRUE; + } +} + +static void wayland_text_input_ime_disable(struct wayland_text_input *text_input) +{ + TRACE("text_input->enabled %u\n", text_input->enabled); + + if (text_input->enabled) + { + zwp_text_input_v3_disable(text_input->zwp_text_input_v3); + zwp_text_input_v3_commit(text_input->zwp_text_input_v3); + wayland_text_input_reset_all_state(text_input); + text_input->enabled = FALSE; + } +} + static void text_input_enter(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, struct wl_surface *surface) { struct wayland_text_input *text_input = data; + struct wayland_win_data *win; HWND hwnd; if (!surface) return; @@ -88,15 +118,14 @@ static void text_input_enter(void *data, struct zwp_text_input_v3 *zwp_text_inpu hwnd = wl_surface_get_user_data(surface); TRACE("data %p, text_input %p, hwnd %p.\n", data, zwp_text_input_v3, hwnd); + if (!(win = wayland_win_data_get(hwnd))) return; + pthread_mutex_lock(&text_input->mutex); text_input->focused_hwnd = hwnd; - zwp_text_input_v3_enable(text_input->zwp_text_input_v3); - zwp_text_input_v3_set_content_type(text_input->zwp_text_input_v3, - ZWP_TEXT_INPUT_V3_CONTENT_HINT_NONE, - ZWP_TEXT_INPUT_V3_CONTENT_PURPOSE_NORMAL); - zwp_text_input_v3_set_cursor_rectangle(text_input->zwp_text_input_v3, 0, 0, 0, 0); - zwp_text_input_v3_commit(text_input->zwp_text_input_v3); + if (win->num_ime_children) wayland_text_input_ime_enable(text_input); pthread_mutex_unlock(&text_input->mutex); + + wayland_win_data_release(win); } static void text_input_leave(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, @@ -106,14 +135,12 @@ static void text_input_leave(void *data, struct zwp_text_input_v3 *zwp_text_inpu TRACE("data %p, text_input %p.\n", data, zwp_text_input_v3); pthread_mutex_lock(&text_input->mutex); - zwp_text_input_v3_disable(text_input->zwp_text_input_v3); - zwp_text_input_v3_commit(text_input->zwp_text_input_v3); + wayland_text_input_ime_disable(text_input); if (text_input->focused_hwnd) { post_ime_update(text_input->focused_hwnd, 0, NULL, NULL); text_input->focused_hwnd = NULL; } - wayland_text_input_reset_all_state(text_input); pthread_mutex_unlock(&text_input->mutex); } @@ -203,6 +230,7 @@ void wayland_text_input_init(void) text_input->zwp_text_input_v3 = zwp_text_input_manager_v3_get_text_input( process_wayland.zwp_text_input_manager_v3, process_wayland.seat.wl_seat); zwp_text_input_v3_add_listener(text_input->zwp_text_input_v3, &text_input_listener, text_input); + text_input->enabled = FALSE; pthread_mutex_unlock(&text_input->mutex); }; @@ -214,6 +242,7 @@ void wayland_text_input_deinit(void) zwp_text_input_v3_destroy(text_input->zwp_text_input_v3); text_input->zwp_text_input_v3 = NULL; text_input->focused_hwnd = NULL; + text_input->enabled = FALSE; wayland_text_input_reset_all_state(text_input); pthread_mutex_unlock(&text_input->mutex); }; @@ -231,13 +260,13 @@ BOOL WAYLAND_SetIMECompositionRect(HWND hwnd, RECT rect) pthread_mutex_lock(&text_input->mutex); - if (!text_input->zwp_text_input_v3 || hwnd != text_input->focused_hwnd) + if (!text_input->zwp_text_input_v3 || !text_input->enabled || hwnd != text_input->focused_hwnd) goto err; if (!(data = wayland_win_data_get(hwnd))) goto err; - if (!(surface = data->wayland_surface)) + if (!(surface = data->wayland_surface) || !data->num_ime_children) { wayland_win_data_release(data); goto err; @@ -264,3 +293,45 @@ err: pthread_mutex_unlock(&text_input->mutex); return FALSE; } + +BOOL WAYLAND_SetIMEEnabled(HWND hwnd, BOOL enabled) +{ + struct wayland_text_input *text_input = &process_wayland.text_input; + struct wayland_win_data *hwnd_data, *toplevel_data; + HWND toplevel; + + toplevel = NtUserGetAncestor(hwnd, GA_ROOT); + TRACE("hwnd %p, toplevel hwnd %p, enabled %u.\n", hwnd, toplevel, enabled); + + pthread_mutex_lock(&text_input->mutex); + + if (!text_input->zwp_text_input_v3) + goto err; + + if (!(hwnd_data = wayland_win_data_get(hwnd))) + goto err; + + if (!(toplevel_data = wayland_win_data_get_nolock(toplevel))) + { + wayland_win_data_release(hwnd_data); + goto err; + } + + if (!hwnd_data->ime_enabled && enabled) toplevel_data->num_ime_children++; + else if (hwnd_data->ime_enabled && !enabled) toplevel_data->num_ime_children--; + hwnd_data->ime_enabled = enabled; + + if (text_input->focused_hwnd == toplevel) + { + if (toplevel_data->num_ime_children) wayland_text_input_ime_enable(text_input); + else wayland_text_input_ime_disable(text_input); + } + + wayland_win_data_release(hwnd_data); + pthread_mutex_unlock(&text_input->mutex); + return TRUE; + +err: + pthread_mutex_unlock(&text_input->mutex); + return FALSE; +} diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 9452d237fa3..701e48a2059 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -129,6 +129,7 @@ struct wayland_text_input } preedit, current_preedit; WCHAR *commit_string; HWND focused_hwnd; + BOOL enabled; pthread_mutex_t mutex; }; @@ -372,6 +373,8 @@ struct wayland_win_data BOOL is_fullscreen; BOOL managed; BOOL layered_attribs_set; + BOOL ime_enabled; + int num_ime_children; }; struct wayland_win_data *wayland_win_data_get(HWND hwnd); @@ -443,6 +446,7 @@ BOOL WAYLAND_ClipCursor(const RECT *clip, BOOL reset); LRESULT WAYLAND_DesktopWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp); void WAYLAND_DestroyWindow(HWND hwnd); BOOL WAYLAND_SetIMECompositionRect(HWND hwnd, RECT rect); +BOOL WAYLAND_SetIMEEnabled(HWND hwnd, BOOL enable); void WAYLAND_SetCursor(HWND hwnd, HCURSOR hcursor); BOOL WAYLAND_SetCursorPos(INT x, INT y); void WAYLAND_SetLayeredWindowAttributes(HWND hwnd, COLORREF key, BYTE alpha, DWORD flags); diff --git a/dlls/winewayland.drv/waylanddrv_main.c b/dlls/winewayland.drv/waylanddrv_main.c index cdb5dd8a956..c8c801d63b8 100644 --- a/dlls/winewayland.drv/waylanddrv_main.c +++ b/dlls/winewayland.drv/waylanddrv_main.c @@ -40,6 +40,7 @@ static const struct user_driver_funcs waylanddrv_funcs = .pDesktopWindowProc = WAYLAND_DesktopWindowProc, .pDestroyWindow = WAYLAND_DestroyWindow, .pSetIMECompositionRect = WAYLAND_SetIMECompositionRect, + .pSetIMEEnabled = WAYLAND_SetIMEEnabled, .pKbdLayerDescriptor = WAYLAND_KbdLayerDescriptor, .pReleaseKbdTables = WAYLAND_ReleaseKbdTables, .pSetCursor = WAYLAND_SetCursor, diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 07e0858fb39..2d96d457b52 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -72,6 +72,8 @@ static struct wayland_win_data *wayland_win_data_create(HWND hwnd, const struct data->hwnd = hwnd; data->rects = *rects; + data->ime_enabled = FALSE; + data->num_ime_children = 0; pthread_mutex_lock(&win_data_mutex); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10007
From: Ziia Shi <mkrsym1@gmail.com> --- dlls/imm32/tests/imm32.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dlls/imm32/tests/imm32.c b/dlls/imm32/tests/imm32.c index fed49315d3b..92201dcfeb9 100644 --- a/dlls/imm32/tests/imm32.c +++ b/dlls/imm32/tests/imm32.c @@ -5584,6 +5584,27 @@ static void test_ImmSetActiveContext(void) ok_ret( 1, ImmActivateLayout( hkl ) ); ok_ret( 1, ImmLoadIME( hkl ) ); + ok_ret( 1, ImmSetActiveContext( hwnd, default_himc, TRUE ) ); + ok_ret( 1, ImmActivateLayout( default_hkl ) ); + ok_ret( 1, ImmLoadIME( default_hkl ) ); + process_messages(); + memset( ime_calls, 0, sizeof(ime_calls) ); + ime_call_count = 0; + ok_ret( 1, ImmSetActiveContext( hwnd, default_himc, FALSE ) ); + ok_seq( empty_sequence ); + + ok_ret( 1, ImmFreeLayout( hkl ) ); + + ok_ret( 1, ImmSetActiveContext( hwnd, default_himc, TRUE ) ); + ok_ret( 1, ImmActivateLayout( hkl ) ); + ok_ret( 1, ImmLoadIME( hkl ) ); + process_messages(); + memset( ime_calls, 0, sizeof(ime_calls) ); + ime_call_count = 0; + ok_ret( 1, ImmSetActiveContext( hwnd, default_himc, FALSE ) ); + ok_seq( deactivate_0_seq ); + + ok_ret( 1, ImmFreeLayout( default_hkl ) ); process_messages(); memset( ime_calls, 0, sizeof(ime_calls) ); ime_call_count = 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10007
Unfortunately it is indeed the new IME which receives the deactivation. I suppose `NtUserAssociateInputContext` could check if the IME in ctx/win-\>imc is builtin, and do the driver call instead? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10007#note_128680
IMC doesn't really holds that information, it's more a matter of HKL, but I'm not sure we do that properly yet. In any case I expect it should not make much difference to notify the host IME about activation / deactivation even if it's not supposed to be used, compared to current code where it's always enabled? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10007#note_128690
Well it is indeed not worse than the current code. I'll do it that way then. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10007#note_128693
participants (3)
-
Rémi Bernon (＠rbernon) -
Ziia Shi
-
Ziia Shi (＠mkrsym1)