[PATCH 0/2] MR8196: winewayland: Ignore text-input "done" events that don't modify state.
Compositors send a "done" event after every text-input commit, even if the reported state (preedit etc) hasn't changed. Acting on such events is at best wasteful, but can additionally lead to incorrect IME related effects (e.g., deleting the currently selected text), so ignore them, similarly to what Qt and GTK do. The first commit performs some state related cleanups. -- This solves an IME problem I have been experiencing with compositors that support text-input-v3 (e.g., kwin). 1. Run notepad 2. Write some text 3. Try to select text Expected result: text is selected Actual result: text is deleted What happens is that while the user is selecting text the driver gets a series of `SetIMECompositionRect` callback which cause a series of text-input-v3 `set_cursor_rectangle`-`commit` requests. These `commit`s elicit `done` events from the compositor with no state changes, which we interpret as "clear the composition string" (and by extension the active selection), because that's what such states normally means, and forward them to windows IME. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8196
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Ensure state resources are cleaned up when not needed, and also avoid memory leaks in case we get multiple events for the same state before a "done" event. --- dlls/winewayland.drv/wayland_text_input.c | 33 +++++++++++++++-------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/dlls/winewayland.drv/wayland_text_input.c b/dlls/winewayland.drv/wayland_text_input.c index 18f64797131..ddf30abe12a 100644 --- a/dlls/winewayland.drv/wayland_text_input.c +++ b/dlls/winewayland.drv/wayland_text_input.c @@ -60,6 +60,15 @@ static WCHAR *strdupUtoW(const char *str) return ret; } +static void wayland_text_input_reset_pending_state(struct wayland_text_input *text_input) +{ + free(text_input->preedit_string); + text_input->preedit_string = NULL; + text_input->preedit_cursor_pos = 0; + free(text_input->commit_string); + text_input->commit_string = NULL; +} + static void text_input_enter(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, struct wl_surface *surface) { @@ -96,6 +105,7 @@ static void text_input_leave(void *data, struct zwp_text_input_v3 *zwp_text_inpu post_ime_update(text_input->focused_hwnd, 0, NULL, NULL); text_input->focused_hwnd = NULL; } + wayland_text_input_reset_pending_state(text_input); pthread_mutex_unlock(&text_input->mutex); } @@ -103,18 +113,22 @@ static void text_input_preedit_string(void *data, struct zwp_text_input_v3 *zwp_ const char *text, int32_t cursor_begin, int32_t cursor_end) { struct wayland_text_input *text_input = data; + DWORD begin = 0, end = 0; + WCHAR *textW; + TRACE("data %p, text_input %p, text %s, cursor %d - %d.\n", data, zwp_text_input_v3, debugstr_a(text), cursor_begin, cursor_end); - pthread_mutex_lock(&text_input->mutex); - if ((text_input->preedit_string = strdupUtoW(text))) + if ((textW = strdupUtoW(text))) { - DWORD begin = 0, end = 0; - if (cursor_begin > 0) RtlUTF8ToUnicodeN(NULL, 0, &begin, text, cursor_begin); if (cursor_end > 0) RtlUTF8ToUnicodeN(NULL, 0, &end, text, cursor_end); - text_input->preedit_cursor_pos = MAKELONG(begin / sizeof(WCHAR), end / sizeof(WCHAR)); } + + pthread_mutex_lock(&text_input->mutex); + free(text_input->preedit_string); + text_input->preedit_string = textW; + text_input->preedit_cursor_pos = MAKELONG(begin / sizeof(WCHAR), end / sizeof(WCHAR)); pthread_mutex_unlock(&text_input->mutex); } @@ -125,6 +139,7 @@ static void text_input_commit_string(void *data, struct zwp_text_input_v3 *zwp_t TRACE("data %p, text_input %p, text %s.\n", data, zwp_text_input_v3, debugstr_a(text)); pthread_mutex_lock(&text_input->mutex); + free(text_input->commit_string); text_input->commit_string = strdupUtoW(text); pthread_mutex_unlock(&text_input->mutex); } @@ -149,12 +164,7 @@ static void text_input_done(void *data, struct zwp_text_input_v3 *zwp_text_input post_ime_update(text_input->focused_hwnd, text_input->preedit_cursor_pos, text_input->preedit_string, text_input->commit_string); } - - free(text_input->preedit_string); - text_input->preedit_string = NULL; - text_input->preedit_cursor_pos = 0; - free(text_input->commit_string); - text_input->commit_string = NULL; + wayland_text_input_reset_pending_state(text_input); pthread_mutex_unlock(&text_input->mutex); } @@ -187,6 +197,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; + wayland_text_input_reset_pending_state(text_input); pthread_mutex_unlock(&text_input->mutex); }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8196
From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com> Compositors send a "done" event after every text-input commit, even if the reported state (preedit etc) hasn't changed. Acting on such events is at best wasteful, but can additionally lead to incorrect IME related effects (e.g., deleting the currently selected text), so ignore them, similarly to what Qt and GTK do. --- dlls/winewayland.drv/wayland_text_input.c | 41 ++++++++++++++++------- dlls/winewayland.drv/waylanddrv.h | 7 ++-- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/dlls/winewayland.drv/wayland_text_input.c b/dlls/winewayland.drv/wayland_text_input.c index ddf30abe12a..52cfef27e50 100644 --- a/dlls/winewayland.drv/wayland_text_input.c +++ b/dlls/winewayland.drv/wayland_text_input.c @@ -62,13 +62,21 @@ static WCHAR *strdupUtoW(const char *str) static void wayland_text_input_reset_pending_state(struct wayland_text_input *text_input) { - free(text_input->preedit_string); - text_input->preedit_string = NULL; - text_input->preedit_cursor_pos = 0; + free(text_input->preedit.string); + text_input->preedit.string = NULL; + text_input->preedit.cursor_pos = 0; free(text_input->commit_string); text_input->commit_string = NULL; } +static void wayland_text_input_reset_all_state(struct wayland_text_input *text_input) +{ + free(text_input->current_preedit.string); + text_input->current_preedit.string = NULL; + text_input->current_preedit.cursor_pos = 0; + wayland_text_input_reset_pending_state(text_input); +} + static void text_input_enter(void *data, struct zwp_text_input_v3 *zwp_text_input_v3, struct wl_surface *surface) { @@ -105,7 +113,7 @@ static void text_input_leave(void *data, struct zwp_text_input_v3 *zwp_text_inpu post_ime_update(text_input->focused_hwnd, 0, NULL, NULL); text_input->focused_hwnd = NULL; } - wayland_text_input_reset_pending_state(text_input); + wayland_text_input_reset_all_state(text_input); pthread_mutex_unlock(&text_input->mutex); } @@ -126,9 +134,9 @@ static void text_input_preedit_string(void *data, struct zwp_text_input_v3 *zwp_ } pthread_mutex_lock(&text_input->mutex); - free(text_input->preedit_string); - text_input->preedit_string = textW; - text_input->preedit_cursor_pos = MAKELONG(begin / sizeof(WCHAR), end / sizeof(WCHAR)); + free(text_input->preedit.string); + text_input->preedit.string = textW; + text_input->preedit.cursor_pos = MAKELONG(begin / sizeof(WCHAR), end / sizeof(WCHAR)); pthread_mutex_unlock(&text_input->mutex); } @@ -158,12 +166,21 @@ static void text_input_done(void *data, struct zwp_text_input_v3 *zwp_text_input pthread_mutex_lock(&text_input->mutex); /* Some compositors will send a done event for every commit, regardless of * the focus state of the text input. This behavior is arguably out of spec, - * but otherwise harmless, so just ignore the new state in such cases. */ - if (text_input->focused_hwnd) + * but otherwise harmless, so just ignore the new state in such cases. + * Additionally ignore done events that don't actually modify the state. */ + if (text_input->focused_hwnd && + (text_input->commit_string || + text_input->preedit.cursor_pos != text_input->current_preedit.cursor_pos || + !!text_input->preedit.string != !!text_input->current_preedit.string || + (text_input->preedit.string && text_input->current_preedit.string && + wcscmp(text_input->preedit.string, text_input->current_preedit.string)))) { - post_ime_update(text_input->focused_hwnd, text_input->preedit_cursor_pos, - text_input->preedit_string, text_input->commit_string); + post_ime_update(text_input->focused_hwnd, text_input->preedit.cursor_pos, + text_input->preedit.string, text_input->commit_string); } + free(text_input->current_preedit.string); + text_input->current_preedit = text_input->preedit; + text_input->preedit.string = NULL; wayland_text_input_reset_pending_state(text_input); pthread_mutex_unlock(&text_input->mutex); } @@ -197,7 +214,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; - wayland_text_input_reset_pending_state(text_input); + wayland_text_input_reset_all_state(text_input); pthread_mutex_unlock(&text_input->mutex); }; diff --git a/dlls/winewayland.drv/waylanddrv.h b/dlls/winewayland.drv/waylanddrv.h index 251a23a4032..001cb29e05e 100644 --- a/dlls/winewayland.drv/waylanddrv.h +++ b/dlls/winewayland.drv/waylanddrv.h @@ -120,8 +120,11 @@ struct wayland_pointer struct wayland_text_input { struct zwp_text_input_v3 *zwp_text_input_v3; - WCHAR *preedit_string; - DWORD preedit_cursor_pos; + struct + { + WCHAR *string; + DWORD cursor_pos; + } preedit, current_preedit; WCHAR *commit_string; HWND focused_hwnd; pthread_mutex_t mutex; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8196
This is a problem caused by !7827. I was preparing a patch to fix it with a different approach, but this looks good too. Also, there is an issue with the wayland driver entering an infinite IME message generation loop under certain conditions, which this MR fixes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8196#note_105307
This merge request was approved by Attila Fidan. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8196
participants (4)
-
Alexandros Frantzis -
Alexandros Frantzis (＠afrantzis)
-
Attila Fidan (＠atticf)
-
Byeongsik Jeon (＠bsjeon)