Fix #137346: IME input getting lost when using Wayland

Our IME input system relied on passing around pointers to global variables.
However this will not work as the Wayland input handling is multithreaded so the content of the global variable would change while the event loop were reading the data.

Pull Request: https://projects.blender.org/blender/blender/pulls/138871

intern/ghost/GHOST_Types.h

@@ -9,6 +9,7 @@
 #pragma once
 
 #include <stdint.h>
+#include <string>
 
 #ifdef WITH_VULKAN_BACKEND
 #  ifdef __APPLE__
@@ -638,10 +639,8 @@ typedef struct {
  * All members must remain aligned and the struct size match!
  */
 typedef struct {
-  /** size_t */
-  GHOST_TUserDataPtr result_len, composite_len;
-  /** char * utf8 encoding */
-  GHOST_TUserDataPtr result, composite;
+  /** utf8 encoded strings */
+  std::string result, composite;
   /** Cursor position in the IME composition. */
   int cursor_position;
   /** Represents the position of the beginning of the selection */

intern/ghost/intern/GHOST_ImeWin32.cc

@@ -422,7 +422,7 @@ void GHOST_ImeWin32::EndIME(HWND window_handle)
   is_enable = false;
   CleanupComposition(window_handle);
   ::ImmAssociateContextEx(window_handle, nullptr, 0);
-  eventImeData.composite_len = 0;
+  eventImeData.composite.clear();
 }
 
 void GHOST_ImeWin32::BeginIME(HWND window_handle, const GHOST_Rect &caret_rect, bool complete)
@@ -489,23 +489,19 @@ void GHOST_ImeWin32::UpdateInfo(HWND window_handle)
   int comp = this->GetComposition(window_handle, GCS_COMPSTR | GCS_COMPATTR, &compInfo);
   /* convert wchar to utf8 */
   if (res) {
-    eventImeData.result_len = (GHOST_TUserDataPtr)updateUtf8Buf(resultInfo);
-    eventImeData.result = &resultInfo.utf8_buf[0];
+    eventImeData.result = std::string(&resultInfo.utf8_buf[0]);
   }
   else {
-    eventImeData.result = 0;
-    eventImeData.result_len = 0;
+    eventImeData.result = "";
   }
   if (comp) {
-    eventImeData.composite_len = (GHOST_TUserDataPtr)updateUtf8Buf(compInfo);
-    eventImeData.composite = &compInfo.utf8_buf[0];
+    eventImeData.composite = std::string(&compInfo.utf8_buf[0]);
     eventImeData.cursor_position = compInfo.cursor_position;
     eventImeData.target_start = compInfo.target_start;
     eventImeData.target_end = compInfo.target_end;
   }
   else {
-    eventImeData.composite = 0;
-    eventImeData.composite_len = 0;
+    eventImeData.composite = "";
     eventImeData.cursor_position = -1;
     eventImeData.target_start = -1;
     eventImeData.target_end = -1;

intern/ghost/intern/GHOST_SystemWayland.cc

@@ -1013,10 +1013,11 @@ struct GWL_SeatIME {
    */
   wl_surface *surface_window = nullptr;
   GHOST_TEventImeData event_ime_data = {
-      /*result_len*/ nullptr,
-      /*composite_len*/ nullptr,
-      /*result*/ nullptr,
-      /*composite*/ nullptr,
+      /** Storage for #GHOST_TEventImeData::result (the result of the `commit_string` callback). */
+      /*result*/ "",
+      /** Storage for #GHOST_TEventImeData::composite (the result of the `preedit_string`
+         callback). */
+      /*composite*/ "",
       /*cursor_position*/ -1,
       /*target_start*/ -1,
       /*target_end*/ -1,
@@ -1029,11 +1030,6 @@ struct GWL_SeatIME {
    */
   bool has_preedit = false;
 
-  /** Storage for #GHOST_TEventImeData::result (the result of the `commit_string` callback). */
-  std::string result;
-  /** Storage for #GHOST_TEventImeData::composite (the result of the `preedit_string` callback). */
-  std::string composite;
-
   /** #zwp_text_input_v3_listener::commit_string was called with a null text argument. */
   bool result_is_null = false;
   /** #zwp_text_input_v3_listener::preedit_string was called with a null text argument. */
@@ -1373,22 +1369,17 @@ static void gwl_seat_ime_full_reset(GWL_Seat *seat)
 
 static void gwl_seat_ime_result_reset(GWL_Seat *seat)
 {
-  seat->ime.result.clear();
-  seat->ime.result_is_null = false;
-
   GHOST_TEventImeData &event_ime_data = seat->ime.event_ime_data;
-  event_ime_data.result_len = nullptr;
-  event_ime_data.result = nullptr;
+  event_ime_data.result.clear();
+  seat->ime.result_is_null = false;
 }
 
 static void gwl_seat_ime_preedit_reset(GWL_Seat *seat)
 {
-  seat->ime.composite.clear();
-  seat->ime.composite_is_null = false;
 
   GHOST_TEventImeData &event_ime_data = seat->ime.event_ime_data;
-  event_ime_data.composite_len = nullptr;
-  event_ime_data.composite = nullptr;
+  event_ime_data.composite.clear();
+  seat->ime.composite_is_null = false;
 
   event_ime_data.cursor_position = -1;
   event_ime_data.target_start = -1;
@@ -5754,6 +5745,9 @@ static const zwp_primary_selection_source_v1_listener primary_selection_source_l
 #ifdef WITH_INPUT_IME
 
 class GHOST_EventIME : public GHOST_Event {
+ protected:
+  GHOST_TEventImeData event_ime_data;
+
  public:
   /**
    * Constructor.
@@ -5761,10 +5755,17 @@ class GHOST_EventIME : public GHOST_Event {
    * \param type: The type of key event.
    * \param key: The key code of the key.
    */
-  GHOST_EventIME(uint64_t msec, GHOST_TEventType type, GHOST_IWindow *window, void *customdata)
+  GHOST_EventIME(uint64_t msec,
+                 GHOST_TEventType type,
+                 GHOST_IWindow *window,
+                 GHOST_TEventImeData *customdata)
       : GHOST_Event(msec, type, window)
   {
-    this->m_data = customdata;
+    /* Make sure that we kee a copy of the ime input. Otherwise it might get lost
+     * because we overwrite it before it can be read in Blender. (See #137346)
+     */
+    this->event_ime_data = *customdata;
+    this->m_data = &this->event_ime_data;
   }
 };
 
@@ -5828,9 +5829,7 @@ static void text_input_handle_preedit_string(void *data,
 
   seat->ime.composite_is_null = (text == nullptr);
   if (!seat->ime.composite_is_null) {
-    seat->ime.composite = text;
-    seat->ime.event_ime_data.composite = (void *)seat->ime.composite.c_str();
-    seat->ime.event_ime_data.composite_len = (void *)seat->ime.composite.size();
+    seat->ime.event_ime_data.composite = text;
 
     seat->ime.event_ime_data.cursor_position = cursor_begin;
     seat->ime.event_ime_data.target_start = cursor_begin;
@@ -5847,11 +5846,9 @@ static void text_input_handle_commit_string(void *data,
   CLOG_INFO(LOG, 2, "commit_string (text=\"%s\")", text ? text : "<null>");
 
   GWL_Seat *seat = static_cast<GWL_Seat *>(data);
-  seat->ime.result = text ? text : "";
   seat->ime.result_is_null = (text == nullptr);
-  seat->ime.event_ime_data.result = (void *)seat->ime.result.c_str();
-  seat->ime.event_ime_data.result_len = (void *)seat->ime.result.size();
-  seat->ime.event_ime_data.cursor_position = seat->ime.result.size();
+  seat->ime.event_ime_data.result = text ? text : "";
+  seat->ime.event_ime_data.cursor_position = seat->ime.event_ime_data.result.size();
 
   seat->ime.has_commit_string_callback = true;
 }

intern/ghost/intern/GHOST_SystemWin32.cc

@@ -1694,7 +1694,7 @@ LRESULT WINAPI GHOST_SystemWin32::s_wndProc(HWND hwnd, uint msg, WPARAM wParam,
           eventHandled = true;
           ime->UpdateImeWindow(hwnd);
           ime->UpdateInfo(hwnd);
-          if (ime->eventImeData.result_len) {
+          if (ime->eventImeData.result.size()) {
             /* remove redundant IME event */
             eventManager->removeTypeEvents(GHOST_kEventImeComposition, window);
           }

intern/ghost/intern/GHOST_WindowViewCocoa.hh

@@ -27,8 +27,6 @@
 
     /* Event data. */
     GHOST_TEventImeData event;
-    std::string result;
-    std::string composite;
     std::string combined_result;
   } ime;
 #endif
@@ -407,8 +405,8 @@ HANDLE_TABLET_EVENT(tabletProximity)
 - (void)endIME
 {
   ime.state_flag = 0;
-  ime.result.clear();
-  ime.composite.clear();
+  ime.event.result.clear();
+  ime.event.composite.clear();
 
   [self unmarkText];
   @autoreleasepool {
@@ -418,11 +416,6 @@ HANDLE_TABLET_EVENT(tabletProximity)
 
 - (void)processImeEvent:(GHOST_TEventType)imeEventType
 {
-  ime.event.result_len = (GHOST_TUserDataPtr)ime.result.size();
-  ime.event.result = (GHOST_TUserDataPtr)ime.result.c_str();
-  ime.event.composite_len = (GHOST_TUserDataPtr)ime.composite.size();
-  ime.event.composite = (GHOST_TUserDataPtr)ime.composite.c_str();
-
   GHOST_Event *event = new GHOST_EventIME(
       m_systemCocoa->getMilliSeconds(), imeEventType, m_windowCocoa, &ime.event);
   m_systemCocoa->pushEvent(event);
@@ -438,12 +431,12 @@ HANDLE_TABLET_EVENT(tabletProximity)
 
 - (void)setImeComposition:(NSString *)inString selectedRange:(NSRange)range
 {
-  ime.composite = [self convertNSString:inString];
+  ime.event.composite = [self convertNSString:inString];
 
   /* For Korean input, both "Result Event" and "Composition Event" can occur in a single keyDown.
    */
   if (!(ime.state_flag & GHOST_IME_RESULT_EVENT)) {
-    ime.result.clear();
+    ime.event.result.clear();
   }
 
   /* The target string is equivalent to the string in selectedRange of setMarkedText.
@@ -460,8 +453,8 @@ HANDLE_TABLET_EVENT(tabletProximity)
 
 - (void)setImeResult:(std::string)result
 {
-  ime.result = result;
-  ime.composite.clear();
+  ime.event.result = result;
+  ime.event.composite.clear();
   ime.event.cursor_position = -1;
   ime.event.target_start = -1;
   ime.event.target_end = -1;

source/blender/blenkernel/BKE_wm_runtime.hh

@@ -9,6 +9,9 @@
 #pragma once
 
 struct GSet;
+#ifdef WITH_INPUT_IME
+struct wmIMEData;
+#endif
 
 #include "DNA_windowmanager_types.h"
 
@@ -45,6 +48,15 @@ struct WindowRuntime {
   /** All events #wmEvent (ghost level events were handled). */
   ListBase event_queue = {nullptr, nullptr};
 
+#ifdef WITH_INPUT_IME
+  /**
+   * Input Method Editor data - complex character input (especially for Asian character input)
+   * Only used when `WITH_INPUT_IME` is defined.
+   */
+  wmIMEData *ime_data = nullptr;
+  char ime_data_is_composing = false;
+#endif
+
   WindowRuntime() = default;
   ~WindowRuntime();
 };

source/blender/blenkernel/CMakeLists.txt

@@ -808,6 +808,10 @@ if(WITH_XR_OPENXR)
   add_definitions(-DWITH_XR_OPENXR)
 endif()
 
+if(WITH_INPUT_IME)
+  add_definitions(-DWITH_INPUT_IME)
+endif()
+
 # # Warnings as errors, this is too strict!
 # if(MSVC)
 #    string(APPEND CMAKE_C_FLAGS " /WX")

source/blender/blenkernel/intern/wm_runtime.cc

@@ -31,6 +31,9 @@ WindowManagerRuntime::~WindowManagerRuntime()
 
 WindowRuntime::~WindowRuntime()
 {
+#ifdef WITH_INPUT_IME
+  BLI_assert(this->ime_data == nullptr);
+#endif
   /** The event_queue should be freed when the window is freed. */
   BLI_assert(BLI_listbase_is_empty(&this->event_queue));
 }

source/blender/editors/interface/interface_handlers.cc

@@ -3447,7 +3447,7 @@ static void ui_textedit_ime_begin(wmWindow *win, uiBut * /*but*/)
   /* XXX Is this really needed? */
   int x, y;
 
-  BLI_assert(win->ime_data == nullptr);
+  BLI_assert(win->runtime->ime_data == nullptr);
 
   /* enable IME and position to cursor, it's a trick */
   x = win->eventstate->xy[0];
@@ -3477,7 +3477,7 @@ const wmIMEData *ui_but_ime_data_get(uiBut *but)
   uiHandleButtonData *data = but->semi_modal_state ? but->semi_modal_state : but->active;
 
   if (data && data->window) {
-    return data->window->ime_data;
+    return data->window->runtime->ime_data;
   }
   return nullptr;
 }
@@ -3692,7 +3692,7 @@ static void ui_textedit_end(bContext *C, uiBut *but, uiHandleButtonData *data)
 #ifdef WITH_INPUT_IME
   /* See #wm_window_IME_end code-comments for details. */
 #  if defined(WIN32) || defined(__APPLE__)
-  if (win->ime_data)
+  if (win->runtime->ime_data)
 #  endif
   {
     ui_textedit_ime_end(win, but);
@@ -3804,8 +3804,8 @@ static int ui_do_but_textedit(
 
 #ifdef WITH_INPUT_IME
   wmWindow *win = CTX_wm_window(C);
-  const wmIMEData *ime_data = win->ime_data;
-  const bool is_ime_composing = ime_data && win->ime_data_is_composing;
+  const wmIMEData *ime_data = win->runtime->ime_data;
+  const bool is_ime_composing = ime_data && win->runtime->ime_data_is_composing;
 #else
   const bool is_ime_composing = false;
 #endif
@@ -4136,15 +4136,15 @@ static int ui_do_but_textedit(
   }
   else if (event->type == WM_IME_COMPOSITE_EVENT) {
     changed = true;
-    if (ime_data->result_len) {
+    if (ime_data->result.size()) {
       if (ELEM(but->type, UI_BTYPE_NUM, UI_BTYPE_NUM_SLIDER) &&
-          STREQ(ime_data->str_result, "\xE3\x80\x82"))
+          STREQ(ime_data->result.c_str(), "\xE3\x80\x82"))
       {
         /* Convert Ideographic Full Stop (U+3002) to decimal point when entering numbers. */
         ui_textedit_insert_ascii(but, data, '.');
       }
       else {
-        ui_textedit_insert_buf(but, text_edit, ime_data->str_result, ime_data->result_len);
+        ui_textedit_insert_buf(but, text_edit, ime_data->result.c_str(), ime_data->result.size());
       }
     }
   }

source/blender/editors/interface/interface_widgets.cc

@@ -1928,7 +1928,7 @@ static void widget_draw_text_ime_underline(const uiFontStyle *fstyle,
     }
 
     width = BLF_width(
-        fstyle->uifont_id, drawstr + but->ofs, ime_data->composite_len + but->pos - but->ofs);
+        fstyle->uifont_id, drawstr + but->ofs, ime_data->composite.size() + but->pos - but->ofs);
 
     rgba_uchar_to_float(fcol, wcol->text);
     UI_draw_text_underline(rect->xmin + ofs_x,
@@ -2008,7 +2008,7 @@ static void widget_draw_text(const uiFontStyle *fstyle,
       /* FIXME: IME is modifying `const char *drawstr`! */
       ime_data = ui_but_ime_data_get(but);
 
-      if (ime_data && ime_data->composite_len) {
+      if (ime_data && ime_data->composite.size()) {
         /* insert composite string into cursor pos */
         char tmp_drawstr[UI_MAX_DRAW_STR];
         STRNCPY(tmp_drawstr, drawstr);
@@ -2017,7 +2017,7 @@ static void widget_draw_text(const uiFontStyle *fstyle,
                      "%.*s%s%s",
                      but->pos,
                      but->editstr,
-                     ime_data->str_composite,
+                     ime_data->composite.c_str(),
                      but->editstr + but->pos);
         but->drawstr = tmp_drawstr;
         drawstr = but->drawstr.c_str();
@@ -2093,7 +2093,7 @@ static void widget_draw_text(const uiFontStyle *fstyle,
 
 #ifdef WITH_INPUT_IME
     /* If is IME compositing, move the cursor. */
-    if (ime_data && ime_data->composite_len && ime_data->cursor_pos != -1) {
+    if (ime_data && ime_data->composite.size() && ime_data->cursor_pos != -1) {
       but_pos_ofs += ime_data->cursor_pos;
     }
 #endif
@@ -2142,7 +2142,7 @@ static void widget_draw_text(const uiFontStyle *fstyle,
     if (ime_reposition_window) {
       ui_but_ime_reposition(but, ime_win_x, ime_win_y, false);
     }
-    if (ime_data && ime_data->composite_len) {
+    if (ime_data && ime_data->composite.size()) {
       /* Composite underline. */
       widget_draw_text_ime_underline(fstyle, wcol, but, rect, ime_data, drawstr);
     }

source/blender/makesdna/DNA_windowmanager_types.h

@@ -385,19 +385,12 @@ typedef struct wmWindow {
    */
   struct wmEvent *event_last_handled;
 
-  /**
-   * Input Method Editor data - complex character input (especially for Asian character input)
-   * Only used when `WITH_INPUT_IME` is defined, runtime-only data.
-   */
-  const struct wmIMEData *ime_data;
-  char ime_data_is_composing;
-  char _pad1[6];
-
   /**
    * Internal: tag this for extra mouse-move event,
    * makes cursors/buttons active on UI switching.
    */
   char addmousemove;
+  char _pad1[7];
 
   /** Window+screen handlers, handled last. */
   ListBase handlers;
@@ -420,10 +413,11 @@ typedef struct wmWindow {
    * The time when the key is pressed in milliseconds (see #GHOST_GetEventTime).
    * Used to detect double-click events.
    */
+  void *_pad2;
   uint64_t eventstate_prev_press_time_ms;
 
-  void *_pad2;
   WindowRuntimeHandle *runtime;
+  void *_pad3;
 } wmWindow;
 
 #ifdef ime_data

source/blender/windowmanager/WM_types.hh

@@ -1171,12 +1171,10 @@ struct wmOperatorCallParams {
  * All members must remain aligned and the struct size match!
  */
 struct wmIMEData {
-  size_t result_len, composite_len;
-
   /** UTF8 encoding. */
-  char *str_result;
+  std::string result;
   /** UTF8 encoding. */
-  char *str_composite;
+  std::string composite;
 
   /** Cursor position in the IME composition. */
   int cursor_pos;

source/blender/windowmanager/intern/wm.cc

@@ -172,11 +172,6 @@ static void window_manager_blend_read_data(BlendDataReader *reader, ID *id)
     win->event_last_handled = nullptr;
     win->cursor_keymap_status = nullptr;
 
-    /* Some files could be saved with ime_data still present.
-     * See https://projects.blender.org/blender/blender/issues/136829 */
-    win->ime_data = nullptr;
-    win->ime_data_is_composing = false;
-
     BLI_listbase_clear(&win->handlers);
     BLI_listbase_clear(&win->modalhandlers);
     BLI_listbase_clear(&win->gesture);

source/blender/windowmanager/intern/wm_event_system.cc

@@ -6360,9 +6360,17 @@ void wm_event_add_ghostevent(wmWindowManager *wm,
 #ifdef WITH_INPUT_IME
     case GHOST_kEventImeCompositionStart: {
       event.val = KM_PRESS;
-      win->ime_data = static_cast<const wmIMEData *>(customdata);
-      BLI_assert(win->ime_data != nullptr);
-      win->ime_data_is_composing = true;
+      BLI_assert(customdata != nullptr);
+      /* We need to free the previously allocated data (if any). */
+      MEM_delete(win->runtime->ime_data);
+
+      /* We make a copy of the ghost custom data as it is not certain that the pointer
+       * will be valid after the event itself gets freed.
+       */
+      const wmIMEData *ghost_event_data = static_cast<const wmIMEData *>(customdata);
+      win->runtime->ime_data = MEM_new<wmIMEData>(__func__, *ghost_event_data);
+
+      win->runtime->ime_data_is_composing = true;
       event.type = WM_IME_COMPOSITE_START;
       wm_event_add_intern(win, &event);
       break;
@@ -6375,7 +6383,7 @@ void wm_event_add_ghostevent(wmWindowManager *wm,
     }
     case GHOST_kEventImeCompositionEnd: {
       event.val = KM_PRESS;
-      win->ime_data_is_composing = false;
+      win->runtime->ime_data_is_composing = false;
       event.type = WM_IME_COMPOSITE_END;
       wm_event_add_intern(win, &event);
       break;

source/blender/windowmanager/intern/wm_window.cc

@@ -3153,11 +3153,12 @@ void wm_window_IME_end(wmWindow *win)
    * Even if no IME events were generated (which assigned `ime_data`).
    * TODO: check if #GHOST_EndIME can run on WIN32 & APPLE without causing problems. */
 #  if defined(WIN32) || defined(__APPLE__)
-  BLI_assert(win->ime_data);
+  BLI_assert(win->runtime->ime_data);
 #  endif
   GHOST_EndIME(static_cast<GHOST_WindowHandle>(win->ghostwin));
-  win->ime_data = nullptr;
-  win->ime_data_is_composing = false;
+  MEM_delete(win->runtime->ime_data);
+  win->runtime->ime_data = nullptr;
+  win->runtime->ime_data_is_composing = false;
 }
 #endif /* WITH_INPUT_IME */