[PATCH v5 0/5] MR7812: win32u: Add more CompAttr, CompClause implementation using cursor_begin, cursor_end concept.
If CompAttr and CompClause are properly configured, Japanese input will be more comfortable. Inspired by cursor_begin and cursor_end from Wayland zwp_text_input_v3:: preedit_string, I extended the cursor_pos concept as follows: cursor_pos = MAKELONG( cursor_begin, cursor_end ); ime_to_tascii_ex() uses this to construct CompAttr, CompClause. MS Windows native CompAttr, CompClause is a bit more complicated than this, but the concept is useful enough. It requires additional implementation in the Wine ime_ui_window proc and richedit control. However, it is useful for applications that inline ime composition string. This can be tested with MS Office Word, Excel. LANG=ja_JP.UTF-8 wine EXCEL.EXE Test key sequences: - “n-i-h-o-n-g-o-n-o-m-o-j-i-d-e-s-u-.-SPACE”. - And, RIGHT, LEFT, Shift+LEFT, Shift+RIGHT, ESC, SPACE, etc. -- v5: winex11: Update only when caret pos changed in xic_preedit_caret. winex11: Extend cursor_pos using cursor_begin, cursor_end. winemac: Extend cursor_pos using cursor_begin, cursor_end. winewayland: Extend cursor_pos using cursor_begin, cursor_end. win32u: Add more CompAttr, CompClause implementation using cursor_begin, cursor_end concept. https://gitlab.winehq.org/wine/wine/-/merge_requests/7812
From: Byeongsik Jeon <bsjeon(a)hanmail.net> If CompAttr and CompClause are properly configured, Japanese input will be more comfortable. Inspired by cursor_begin and cursor_end from Wayland zwp_text_input_v3:: preedit_string, I extended the cursor_pos concept as follows: cursor_pos = MAKELONG( cursor_begin, cursor_end ); ime_to_tascii_ex() uses this to construct Compttr, CompClause. MS Windows native CompStrAttr, CompStrClause is a bit more complicated than this, but the concept is useful enough. It requires additional implementation in the Wine ime_ui_window proc and richedit control. However, it is useful for applications that inline ime composition string. This can be tested with MS Office Word, Excel. LANG=ja_JP.UTF-8 wine EXCEL.EXE Test key sequences: - 'n-i-h-o-n-g-o-n-o-m-o-j-i-d-e-s-u-.-SPACE'. - And, RIGHT, LEFT, Shift+LEFT, Shift+RIGHT, ESC, SPACE, UP, DOWN, etc. --- dlls/win32u/imm.c | 69 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/dlls/win32u/imm.c b/dlls/win32u/imm.c index c0ed670e779..1c677772e0c 100644 --- a/dlls/win32u/imm.c +++ b/dlls/win32u/imm.c @@ -445,8 +445,8 @@ static void post_ime_update( HWND hwnd, UINT cursor_pos, WCHAR *comp_str, WCHAR WCHAR *prev_result_str, *tmp; struct ime_update *update; - TRACE( "hwnd %p, cursor_pos %u, comp_str %s, result_str %s\n", hwnd, cursor_pos, - debugstr_w(comp_str), debugstr_w(result_str) ); + TRACE( "hwnd %p, cursor_pos %u - %u, comp_str %s, result_str %s\n", hwnd, LOWORD(cursor_pos), + HIWORD(cursor_pos), debugstr_w(comp_str), debugstr_w(result_str) ); comp_len = comp_str ? wcslen( comp_str ) + 1 : 0; result_len = result_str ? wcslen( result_str ) + 1 : 0; @@ -495,6 +495,48 @@ static void post_ime_update( HWND hwnd, UINT cursor_pos, WCHAR *comp_str, WCHAR free( tmp ); } +static UINT get_comp_clause_count( UINT comp_len, UINT cursor_begin, UINT cursor_end ) +{ + if (cursor_begin == cursor_end || (cursor_begin == 0 && cursor_end == comp_len)) + return 2; + else if (cursor_begin == 0 || cursor_end == comp_len) + return 3; + else + return 4; +} + +static void set_comp_clause( DWORD *comp_clause, UINT comp_clause_count, UINT comp_len, + UINT cursor_begin, UINT cursor_end ) +{ + comp_clause[0] = 0; + switch (comp_clause_count) + { + case 2: + comp_clause[1] = comp_len; + break; + case 3: + comp_clause[1] = cursor_begin == 0 ? cursor_end : cursor_begin; + comp_clause[2] = comp_len; + break; + case 4: + comp_clause[1] = cursor_begin; + comp_clause[2] = cursor_end; + comp_clause[3] = comp_len; + break; + } +} + +static void set_comp_attr( BYTE *comp_attr, UINT comp_attr_len, UINT cursor_begin, UINT cursor_end ) +{ + if (cursor_begin == cursor_end) + memset( comp_attr, ATTR_INPUT, comp_attr_len ); + else + { + memset( comp_attr, ATTR_CONVERTED, comp_attr_len ); + memset( comp_attr + cursor_begin, ATTR_TARGET_CONVERTED, cursor_end - cursor_begin ); + } +} + static struct ime_update *find_ime_update( WORD vkey, WORD scan ) { struct ime_update *update; @@ -508,7 +550,8 @@ static struct ime_update *find_ime_update( WORD vkey, WORD scan ) static UINT ime_to_tascii_ex( UINT vkey, UINT lparam, const BYTE *state, COMPOSITIONSTRING *compstr, BOOL *key_consumed, HIMC himc ) { - UINT needed = sizeof(COMPOSITIONSTRING), comp_len, result_len; + UINT needed = sizeof(COMPOSITIONSTRING), comp_len, result_len, comp_clause_count = 0; + UINT cursor_begin = 0, cursor_end = 0; struct ime_update *update; void *dst; @@ -528,9 +571,18 @@ static UINT ime_to_tascii_ex( UINT vkey, UINT lparam, const BYTE *state, COMPOSI else { comp_len = wcslen( update->comp_str ); + cursor_begin = LOWORD(update->cursor_pos); + cursor_end = HIWORD(update->cursor_pos); + + if (cursor_begin > comp_len) cursor_begin = comp_len; + if (cursor_end > comp_len) cursor_end = comp_len; + if (cursor_end < cursor_begin) cursor_end = cursor_begin; + + comp_clause_count = get_comp_clause_count( comp_len, cursor_begin, cursor_end ); + needed += comp_len * sizeof(WCHAR); /* GCS_COMPSTR */ needed += comp_len; /* GCS_COMPATTR */ - needed += 2 * sizeof(DWORD); /* GCS_COMPCLAUSE */ + needed += comp_clause_count * sizeof(DWORD); /* GCS_COMPCLAUSE */ } if (!update->result_str) result_len = 0; @@ -556,7 +608,7 @@ static UINT ime_to_tascii_ex( UINT vkey, UINT lparam, const BYTE *state, COMPOSI if (update->comp_str) { - compstr->dwCursorPos = update->cursor_pos; + compstr->dwCursorPos = cursor_begin; compstr->dwCompStrLen = comp_len; compstr->dwCompStrOffset = compstr->dwSize; @@ -564,17 +616,16 @@ static UINT ime_to_tascii_ex( UINT vkey, UINT lparam, const BYTE *state, COMPOSI memcpy( dst, update->comp_str, compstr->dwCompStrLen * sizeof(WCHAR) ); compstr->dwSize += compstr->dwCompStrLen * sizeof(WCHAR); - compstr->dwCompClauseLen = 2 * sizeof(DWORD); + compstr->dwCompClauseLen = comp_clause_count * sizeof(DWORD); compstr->dwCompClauseOffset = compstr->dwSize; dst = (BYTE *)compstr + compstr->dwCompClauseOffset; - *((DWORD *)dst + 0) = 0; - *((DWORD *)dst + 1) = compstr->dwCompStrLen; + set_comp_clause( dst, comp_clause_count, comp_len, cursor_begin, cursor_end ); compstr->dwSize += compstr->dwCompClauseLen; compstr->dwCompAttrLen = compstr->dwCompStrLen; compstr->dwCompAttrOffset = compstr->dwSize; dst = (BYTE *)compstr + compstr->dwCompAttrOffset; - memset( dst, ATTR_INPUT, compstr->dwCompAttrLen ); + set_comp_attr( dst, compstr->dwCompAttrLen, cursor_begin, cursor_end ); compstr->dwSize += compstr->dwCompAttrLen; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7812
From: Byeongsik Jeon <bsjeon(a)hanmail.net> --- dlls/winewayland.drv/wayland_text_input.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/dlls/winewayland.drv/wayland_text_input.c b/dlls/winewayland.drv/wayland_text_input.c index f8df5fe09cb..d27d83d62c4 100644 --- a/dlls/winewayland.drv/wayland_text_input.c +++ b/dlls/winewayland.drv/wayland_text_input.c @@ -100,14 +100,17 @@ 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; - TRACE("data %p, text_input %p, text %s, cursor_begin %d.\n", data, zwp_text_input_v3, - debugstr_a(text), cursor_begin); + 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)) && cursor_begin > 0) + if ((text_input->preedit_string = strdupUtoW(text))) { - RtlUTF8ToUnicodeN(NULL, 0, &text_input->preedit_cursor_pos, text, cursor_begin); - text_input->preedit_cursor_pos /= sizeof(WCHAR); + 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_unlock(&text_input->mutex); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7812
From: Byeongsik Jeon <bsjeon(a)hanmail.net> --- dlls/winemac.drv/cocoa_window.m | 3 ++- dlls/winemac.drv/event.c | 4 +++- dlls/winemac.drv/macdrv_cocoa.h | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 4ef9c305b61..71d5c96aa1a 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -867,7 +867,8 @@ - (void) setMarkedText:(id)string selectedRange:(NSRange)selectedRange replaceme event->im_set_text.himc = [window himc]; event->im_set_text.text = (CFStringRef)[[markedText string] copy]; event->im_set_text.complete = FALSE; - event->im_set_text.cursor_pos = markedTextSelection.location + markedTextSelection.length; + event->im_set_text.cursor_begin = markedTextSelection.location; + event->im_set_text.cursor_end = markedTextSelection.location + markedTextSelection.length; [[window queue] postEvent:event]; diff --git a/dlls/winemac.drv/event.c b/dlls/winemac.drv/event.c index 20a791ac48a..82bf829d1a0 100644 --- a/dlls/winemac.drv/event.c +++ b/dlls/winemac.drv/event.c @@ -176,7 +176,9 @@ static void macdrv_im_set_text(const macdrv_event *event) } if (event->im_set_text.complete) post_ime_update(hwnd, -1, NULL, text); - else post_ime_update(hwnd, event->im_set_text.cursor_pos, text, NULL); + else post_ime_update(hwnd, + MAKELONG(event->im_set_text.cursor_begin, event->im_set_text.cursor_end), + text, NULL); free(text); } diff --git a/dlls/winemac.drv/macdrv_cocoa.h b/dlls/winemac.drv/macdrv_cocoa.h index 917c8103d41..9807d4e53fc 100644 --- a/dlls/winemac.drv/macdrv_cocoa.h +++ b/dlls/winemac.drv/macdrv_cocoa.h @@ -365,7 +365,8 @@ extern int macdrv_set_display_mode(const struct macdrv_display* display, struct { void *himc; CFStringRef text; /* new text or NULL if just completing existing text */ - unsigned int cursor_pos; + unsigned int cursor_begin; + unsigned int cursor_end; unsigned int complete; /* is completing text? */ } im_set_text; struct { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7812
From: Byeongsik Jeon <bsjeon(a)hanmail.net> When tested on ibus, fcitx5, and uim, the applicable bit is XIMReverse. Since these always have chg_first == 0, I referenced the libX11 documentation for applying chg_first. --- dlls/winex11.drv/xim.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/xim.c b/dlls/winex11.drv/xim.c index acbe1c3aac1..1b2abb8e92b 100644 --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -168,6 +168,33 @@ static int xic_preedit_done( XIC xic, XPointer user, XPointer arg ) return 0; } +static DWORD get_comp_cursor_pos( XIMPreeditDrawCallbackStruct *params ) +{ + int i, cursor_begin = -1, cursor_end = -1; + XIMText *text = params->text; + + if (text && text->feedback) + { + for (i = 0; i < text->length; i++) + { + if (text->feedback[i] & XIMReverse) + { + if (cursor_begin == -1) cursor_begin = i; + cursor_end = i + 1; + } + } + if (cursor_begin != -1) cursor_begin += params->chg_first; + if (cursor_end != -1) cursor_end += params->chg_first; + } + + if (cursor_begin == cursor_end) + cursor_begin = cursor_end = params->caret; /* ATTR_INPUT */ + + TRACE( "caret %d, cursor_begin %d, cursor_end %d\n", params->caret, cursor_begin, cursor_end ); + + return MAKELONG( cursor_begin, cursor_end ); +} + static int xic_preedit_draw( XIC xic, XPointer user, XPointer arg ) { XIMPreeditDrawCallbackStruct *params = (void *)arg; @@ -202,7 +229,7 @@ static int xic_preedit_draw( XIC xic, XPointer user, XPointer arg ) if (text && str != text->string.multi_byte) free( str ); - post_ime_update( hwnd, params->caret, ime_comp_buf, NULL ); + post_ime_update( hwnd, get_comp_cursor_pos( params ), ime_comp_buf, NULL ); return 0; } @@ -248,7 +275,7 @@ static int xic_preedit_caret( XIC xic, XPointer user, XPointer arg ) } params->position = xim_caret_pos = pos; - post_ime_update( hwnd, pos, ime_comp_buf, NULL ); + post_ime_update( hwnd, MAKELONG( pos, pos ), ime_comp_buf, NULL ); return 0; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7812
From: Byeongsik Jeon <bsjeon(a)hanmail.net> --- dlls/winex11.drv/xim.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/dlls/winex11.drv/xim.c b/dlls/winex11.drv/xim.c index 1b2abb8e92b..ef09356fbb0 100644 --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -45,6 +45,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(xim); #endif static WCHAR *ime_comp_buf; +static DWORD ime_comp_cursor_pos = 0; static XIMStyle input_style = 0; static XIMStyle input_style_req = XIMPreeditCallbacks | XIMStatusCallbacks; @@ -229,14 +230,14 @@ static int xic_preedit_draw( XIC xic, XPointer user, XPointer arg ) if (text && str != text->string.multi_byte) free( str ); - post_ime_update( hwnd, get_comp_cursor_pos( params ), ime_comp_buf, NULL ); + ime_comp_cursor_pos = get_comp_cursor_pos( params ); + post_ime_update( hwnd, ime_comp_cursor_pos, ime_comp_buf, NULL ); return 0; } static int xic_preedit_caret( XIC xic, XPointer user, XPointer arg ) { - static int xim_caret_pos; XIMPreeditCaretCallbackStruct *params = (void *)arg; HWND hwnd = (HWND)user; int pos; @@ -245,7 +246,7 @@ static int xic_preedit_caret( XIC xic, XPointer user, XPointer arg ) if (!params) return 0; - pos = xim_caret_pos; + pos = LOWORD( ime_comp_cursor_pos ); switch (params->direction) { case XIMForwardChar: @@ -273,9 +274,17 @@ static int xic_preedit_caret( XIC xic, XPointer user, XPointer arg ) FIXME( "Not implemented\n" ); break; } - params->position = xim_caret_pos = pos; + params->position = pos; - post_ime_update( hwnd, MAKELONG( pos, pos ), ime_comp_buf, NULL ); + /* uim implements the preedit_caret callback. This callback is only + valid when the xim is in non-converted state (ATTR_INPUT). + */ + if (LOWORD( ime_comp_cursor_pos ) == HIWORD( ime_comp_cursor_pos ) && + LOWORD( ime_comp_cursor_pos ) != pos) + { + ime_comp_cursor_pos = MAKELONG( pos, pos ); + post_ime_update( hwnd, ime_comp_cursor_pos, ime_comp_buf, NULL ); + } return 0; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7812
v3: drop negative cursor value code. I've tested fixing the comp_clause length to 4 and didn't find any problems. Imagining the application code, I think it should be fine. Nevertheless, I can't be sure that there are no applications written in an unusual way. The actual MS clause concept is more complex and may have comp_clause larger than 4. If we can extend the concept further in the future, I think we'll need something like get_comp_clause_count, set_comp_clause, set_comp_attr. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7812#note_101226
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7812
participants (3)
-
Byeongsik Jeon -
Byeongsik Jeon (＠bsjeon)
-
Rémi Bernon