[PATCH 0/2] MR8196: winewayland: Ignore text-input "done" events that don't modify state.

[Alexandros Frantzis (＠afrantzis)]

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.

[PATCH 1/2] winewayland: Improve cleanup of text-input pending state.

[Alexandros Frantzis]

From: Alexandros Frantzis

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);
 };

[PATCH 2/2] winewayland: Ignore text-input "done" events that don't modify state.

[Alexandros Frantzis]

From: Alexandros Frantzis

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;

[Byeongsik Jeon (＠bsjeon)]

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.