From e4aa758d7054933ecaa06ed26ba86374831d42e4 Mon Sep 17 00:00:00 2001 From: Sebastian Parborg Date: Fri, 16 May 2025 14:21:06 +0200 Subject: [PATCH] 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 | 7 ++- intern/ghost/intern/GHOST_ImeWin32.cc | 14 ++--- intern/ghost/intern/GHOST_SystemWayland.cc | 51 +++++++++---------- intern/ghost/intern/GHOST_SystemWin32.cc | 2 +- intern/ghost/intern/GHOST_WindowViewCocoa.hh | 19 +++---- source/blender/blenkernel/BKE_wm_runtime.hh | 12 +++++ source/blender/blenkernel/CMakeLists.txt | 4 ++ .../blender/blenkernel/intern/wm_runtime.cc | 3 ++ .../editors/interface/interface_handlers.cc | 16 +++--- .../editors/interface/interface_widgets.cc | 10 ++-- .../makesdna/DNA_windowmanager_types.h | 12 ++--- source/blender/windowmanager/WM_types.hh | 6 +-- source/blender/windowmanager/intern/wm.cc | 5 -- .../windowmanager/intern/wm_event_system.cc | 16 ++++-- .../blender/windowmanager/intern/wm_window.cc | 7 +-- 15 files changed, 92 insertions(+), 92 deletions(-) diff --git a/intern/ghost/GHOST_Types.h b/intern/ghost/GHOST_Types.h index 2900bf1a6cf..c627ddb871c 100644 --- a/intern/ghost/GHOST_Types.h +++ b/intern/ghost/GHOST_Types.h @@ -9,6 +9,7 @@ #pragma once #include +#include #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 */ diff --git a/intern/ghost/intern/GHOST_ImeWin32.cc b/intern/ghost/intern/GHOST_ImeWin32.cc index 5a0b44a8877..0b6d1533dff 100644 --- a/intern/ghost/intern/GHOST_ImeWin32.cc +++ b/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; diff --git a/intern/ghost/intern/GHOST_SystemWayland.cc b/intern/ghost/intern/GHOST_SystemWayland.cc index e9f7d5edd7e..fe028ffc4b2 100644 --- a/intern/ghost/intern/GHOST_SystemWayland.cc +++ b/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 : ""); GWL_Seat *seat = static_cast(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; } diff --git a/intern/ghost/intern/GHOST_SystemWin32.cc b/intern/ghost/intern/GHOST_SystemWin32.cc index c4d7c77f747..61838df6ee4 100644 --- a/intern/ghost/intern/GHOST_SystemWin32.cc +++ b/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); } diff --git a/intern/ghost/intern/GHOST_WindowViewCocoa.hh b/intern/ghost/intern/GHOST_WindowViewCocoa.hh index 6f3b0499005..a03a2171007 100644 --- a/intern/ghost/intern/GHOST_WindowViewCocoa.hh +++ b/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; diff --git a/source/blender/blenkernel/BKE_wm_runtime.hh b/source/blender/blenkernel/BKE_wm_runtime.hh index cb0505bdd02..40c26cedf80 100644 --- a/source/blender/blenkernel/BKE_wm_runtime.hh +++ b/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(); }; diff --git a/source/blender/blenkernel/CMakeLists.txt b/source/blender/blenkernel/CMakeLists.txt index 8df7d0fe1c3..838f0201cec 100644 --- a/source/blender/blenkernel/CMakeLists.txt +++ b/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") diff --git a/source/blender/blenkernel/intern/wm_runtime.cc b/source/blender/blenkernel/intern/wm_runtime.cc index 9da479ad660..546016d2fa3 100644 --- a/source/blender/blenkernel/intern/wm_runtime.cc +++ b/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)); } diff --git a/source/blender/editors/interface/interface_handlers.cc b/source/blender/editors/interface/interface_handlers.cc index 9878cf58f36..a26c9659375 100644 --- a/source/blender/editors/interface/interface_handlers.cc +++ b/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()); } } } diff --git a/source/blender/editors/interface/interface_widgets.cc b/source/blender/editors/interface/interface_widgets.cc index 36c311deef3..7e1ecceab9b 100644 --- a/source/blender/editors/interface/interface_widgets.cc +++ b/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); } diff --git a/source/blender/makesdna/DNA_windowmanager_types.h b/source/blender/makesdna/DNA_windowmanager_types.h index 49f27a8e988..c8a48dd7aa2 100644 --- a/source/blender/makesdna/DNA_windowmanager_types.h +++ b/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 diff --git a/source/blender/windowmanager/WM_types.hh b/source/blender/windowmanager/WM_types.hh index a0ccce37de7..d11c83af714 100644 --- a/source/blender/windowmanager/WM_types.hh +++ b/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; diff --git a/source/blender/windowmanager/intern/wm.cc b/source/blender/windowmanager/intern/wm.cc index f96fde2abc7..b1b19bece3f 100644 --- a/source/blender/windowmanager/intern/wm.cc +++ b/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); diff --git a/source/blender/windowmanager/intern/wm_event_system.cc b/source/blender/windowmanager/intern/wm_event_system.cc index 38ea7fde6d9..1e053bbb77a 100644 --- a/source/blender/windowmanager/intern/wm_event_system.cc +++ b/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(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(customdata); + win->runtime->ime_data = MEM_new(__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; diff --git a/source/blender/windowmanager/intern/wm_window.cc b/source/blender/windowmanager/intern/wm_window.cc index a497e69c292..1c929b67415 100644 --- a/source/blender/windowmanager/intern/wm_window.cc +++ b/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(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 */