[PATCH] winex11.drv: Fix IME_SetResultString for Korean(hangul) IME.
In a special case for Hangul(Korean), when Composition is completed and this function is called, the new Composition disappears. so, calling IME_SetCompositionString again for makes composition will be starting. Signed-off-by: Alex Kwak <take-me-home(a)kakao.com> --- dlls/winex11.drv/ime.c | 4 ++-- dlls/winex11.drv/x11drv.h | 27 +++++++++++++++++++++++++++ dlls/winex11.drv/xim.c | 13 +++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c index c1584930861..f7827b31635 100644 --- a/dlls/winex11.drv/ime.c +++ b/dlls/winex11.drv/ime.c @@ -1050,8 +1050,8 @@ void IME_SetResultString(LPWSTR lpResult, DWORD dwResultLen) GenerateIMEMessage(imc, WM_IME_COMPOSITION, lpResult[0], GCS_RESULTSTR|GCS_RESULTCLAUSE); GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION, 0, 0); - if (!inComp) - ImmSetOpenStatus(imc, FALSE); + myPrivate->bInComposition = FALSE; + ImmSetOpenStatus(imc, FALSE); ImmUnlockIMC(imc); } diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index f389f3e0836..8ce3a7e8528 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -841,4 +841,31 @@ static inline BOOL is_window_rect_mapped( const RECT *rect ) max( rect->bottom, rect->top + 1 ) > virtual_rect.top); } +#define HANGUL_BASE 0xac00 +#define HANGUL_LAST 0xd7a3 +#define HANGUL_JAMO_BASE 0x1100 +#define HANGUL_JAMO_LAST 0x11F9 +#define HANGUL_COMPAT_JAMO_BASE 0x3130 +#define HANGUL_COMPAT_JAMO_LAST 0x318E +#define HANGUL_LCOUNT 19 +#define HANGUL_VCOUNT 21 +#define HANGUL_TCOUNT 28 +#define HANGUL_NCOUNT (HANGUL_VCOUNT * HANGUL_TCOUNT) +#define HANGUL_SCOUNT (HANGUL_LCOUNT * HANGUL_NCOUNT) + +/* + * This method exists to handle special cases related to Korean(Hangul). + */ +static inline BOOL IsHangul(WPARAM wParam) +{ + if (!wParam) + { + return FALSE; + } + + return (wParam >= HANGUL_BASE && wParam < HANGUL_LAST) || + (wParam >= HANGUL_COMPAT_JAMO_BASE && wParam < HANGUL_COMPAT_JAMO_LAST) || + (wParam >= HANGUL_JAMO_BASE && wParam < HANGUL_JAMO_LAST); +} + #endif /* __WINE_X11DRV_H */ diff --git a/dlls/winex11.drv/xim.c b/dlls/winex11.drv/xim.c index 3994c2106cc..884db67c59c 100644 --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -117,6 +117,19 @@ void X11DRV_XIMLookupChars( const char *str, DWORD count ) IME_SetResultString(wcOutput, dwOutput); HeapFree(GetProcessHeap(), 0, wcOutput); + + /* + * In a special case for Hangul(Korean), when Composition is completed and + * this function is called, the new Composition disappears. so, calling + * IME_SetCompositionString again for makes composition will be + * starting. + */ + if (CompositionString && dwCompStringLength >= sizeof(WCHAR) && + IsHangul(((const WCHAR*) CompositionString)[0])) + { + IME_SetCompositionString(SCS_SETSTR, CompositionString, + dwCompStringLength, NULL, 0); + } } static BOOL XIMPreEditStateNotifyCallback(XIC xic, XPointer p, XPointer data) -- 2.32.0
On Sun, 20 Mar 2022 21:17:27 +0900, Alex Kwak wrote:
In a special case for Hangul(Korean), when Composition is completed and this function is called, the new Composition disappears. so, calling IME_SetCompositionString again for makes composition will be starting.
Please add Wine-Bug header before Signed-off-by line.
Signed-off-by: Alex Kwak <take-me-home(a)kakao.com> --- dlls/winex11.drv/ime.c | 4 ++-- dlls/winex11.drv/x11drv.h | 27 +++++++++++++++++++++++++++ dlls/winex11.drv/xim.c | 13 +++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c index c1584930861..f7827b31635 100644 --- a/dlls/winex11.drv/ime.c +++ b/dlls/winex11.drv/ime.c @@ -1050,8 +1050,8 @@ void IME_SetResultString(LPWSTR lpResult, DWORD dwResultLen) GenerateIMEMessage(imc, WM_IME_COMPOSITION, lpResult[0], GCS_RESULTSTR|GCS_RESULTCLAUSE); GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION, 0, 0);
- if (!inComp) - ImmSetOpenStatus(imc, FALSE); + myPrivate->bInComposition = FALSE; + ImmSetOpenStatus(imc, FALSE);
ImmUnlockIMC(imc); } I'm not sure about this change. It always turns off IME open status. Since composition might be continuous at this moment, it can't be set unconditionally. If we should do so for some reasons, could you explain in detail?
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index f389f3e0836..8ce3a7e8528 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h [..] --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -117,6 +117,19 @@ void X11DRV_XIMLookupChars( const char *str, DWORD count )
IME_SetResultString(wcOutput, dwOutput); HeapFree(GetProcessHeap(), 0, wcOutput); + + /* + * In a special case for Hangul(Korean), when Composition is completed and + * this function is called, the new Composition disappears. so, calling + * IME_SetCompositionString again for makes composition will be + * starting. + */ + if (CompositionString && dwCompStringLength >= sizeof(WCHAR) && + IsHangul(((const WCHAR*) CompositionString)[0])) + { + IME_SetCompositionString(SCS_SETSTR, CompositionString, + dwCompStringLength, NULL, 0); + } }
static BOOL XIMPreEditStateNotifyCallback(XIC xic, XPointer p, XPointer data) Could you revert Hangul(Korean) specific changes? This issue seems not to be Korean specific because I can occasionally reproduce it with Japanese input method.
From my point of view, the main cause of this issue is that the order of PreeditCallbacks and XmbLookupString() aren't stable. The current wine's implementation seems to rely on getting the result of XmbLookupString() before PreeditCallback for the next clause. However, when one or more clauses are left after commiting a clause, PreeditCallback for the left clauses sometimes (or always in Hangul?) called before getting the result of XmbLookupString. What do you think about these XIM situations? Does your patch works well for both cases?
Regards, Akihiro Sagawa -- Best Regards, Akihiro Sagawa
I'm not sure about this change. It always turns off IME open status. Since composition might be continuous at this moment, it can't be set unconditionally. If we should do so for some reasons, could you explain in detail? If WM_IME_ENDCOMPOSITION comes, I think WM_IME_STARTCOMPOSITION should come next time. but it was not. If continue to expect composition, I don't think WM_IME_ENDCOMPOSITION should not come.
From my point of view, the main cause of this issue is that the order of PreeditCallbacks and XmbLookupString() aren't stable. The current wine's implementation seems to rely on getting the result of XmbLookupString() before PreeditCallback for the next clause. However, when one or more clauses are left after commiting a clause, PreeditCallback for the left clauses sometimes (or always in Hangul?) called before getting the result of XmbLookupString. What do you think about these XIM situations? Does your patch works well for both cases? I agree with unstable on CJK inputs.
Unlike Windows, it is implemented without considering what language you are typing and what characteristics each language is input to, so I thought it would not be possible to apply it to all CJK collectively. ( also, XIM doesn't support ) The same situation as the case you mentioned cannot happen in Hangul. because in Hangul, when a word is completed, it must be one clause or the composition must be exited with non-composition character(or event). so, I'm worried about causing problems in Chinese and Japanese. 22. 3. 26. 19:44에 Akihiro Sagawa 이(가) 쓴 글:
On Sun, 20 Mar 2022 21:17:27 +0900, Alex Kwak wrote:
In a special case for Hangul(Korean), when Composition is completed and this function is called, the new Composition disappears. so, calling IME_SetCompositionString again for makes composition will be starting.
Please add Wine-Bug header before Signed-off-by line.
Signed-off-by: Alex Kwak <take-me-home(a)kakao.com> --- dlls/winex11.drv/ime.c | 4 ++-- dlls/winex11.drv/x11drv.h | 27 +++++++++++++++++++++++++++ dlls/winex11.drv/xim.c | 13 +++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c index c1584930861..f7827b31635 100644 --- a/dlls/winex11.drv/ime.c +++ b/dlls/winex11.drv/ime.c @@ -1050,8 +1050,8 @@ void IME_SetResultString(LPWSTR lpResult, DWORD dwResultLen) GenerateIMEMessage(imc, WM_IME_COMPOSITION, lpResult[0], GCS_RESULTSTR|GCS_RESULTCLAUSE); GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION, 0, 0);
- if (!inComp) - ImmSetOpenStatus(imc, FALSE); + myPrivate->bInComposition = FALSE; + ImmSetOpenStatus(imc, FALSE);
ImmUnlockIMC(imc); } I'm not sure about this change. It always turns off IME open status. Since composition might be continuous at this moment, it can't be set unconditionally. If we should do so for some reasons, could you explain in detail?
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index f389f3e0836..8ce3a7e8528 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h [..] --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -117,6 +117,19 @@ void X11DRV_XIMLookupChars( const char *str, DWORD count )
IME_SetResultString(wcOutput, dwOutput); HeapFree(GetProcessHeap(), 0, wcOutput); + + /* + * In a special case for Hangul(Korean), when Composition is completed and + * this function is called, the new Composition disappears. so, calling + * IME_SetCompositionString again for makes composition will be + * starting. + */ + if (CompositionString && dwCompStringLength >= sizeof(WCHAR) && + IsHangul(((const WCHAR*) CompositionString)[0])) + { + IME_SetCompositionString(SCS_SETSTR, CompositionString, + dwCompStringLength, NULL, 0); + } }
static BOOL XIMPreEditStateNotifyCallback(XIC xic, XPointer p, XPointer data) Could you revert Hangul(Korean) specific changes? This issue seems not to be Korean specific because I can occasionally reproduce it with Japanese input method.
From my point of view, the main cause of this issue is that the order of PreeditCallbacks and XmbLookupString() aren't stable. The current wine's implementation seems to rely on getting the result of XmbLookupString() before PreeditCallback for the next clause. However, when one or more clauses are left after commiting a clause, PreeditCallback for the left clauses sometimes (or always in Hangul?) called before getting the result of XmbLookupString. What do you think about these XIM situations? Does your patch works well for both cases?
Regards, Akihiro Sagawa
Hi I resubmitted the problems related to this by dividing them into 4 patches. All CJK have tested it on their own, but in my imagination, the problem seems to have been solved. I tried to create a test, but ImSetCompositionStringW seems to be set regardless of dwIndex, so I can't implement it well. I'll have to take a look when I have time. [PATCH v2] winex11.drv: call XIMReset on CPS_COMPLETE https://source.winehq.org/patches/data/231256 [PATCH v2] riched20: Send CPS_COMPLETE from mouse click event. https://source.winehq.org/patches/data/231251 [PATCH v2] winex11.drv: Call ImeSetCompositionString after IME_SetResultString https://source.winehq.org/patches/data/231236 [PATCH v2] riched20: Handle GCS_COMPSTR, GCS_CURSORPOS on WM_IME_COMPOSITION. https://source.winehq.org/patches/data/231055 Regards, Alex Kwak 22. 3. 26. 19:44에 Akihiro Sagawa 이(가) 쓴 글:
On Sun, 20 Mar 2022 21:17:27 +0900, Alex Kwak wrote:
In a special case for Hangul(Korean), when Composition is completed and this function is called, the new Composition disappears. so, calling IME_SetCompositionString again for makes composition will be starting.
Please add Wine-Bug header before Signed-off-by line.
Signed-off-by: Alex Kwak<take-me-home(a)kakao.com> --- dlls/winex11.drv/ime.c | 4 ++-- dlls/winex11.drv/x11drv.h | 27 +++++++++++++++++++++++++++ dlls/winex11.drv/xim.c | 13 +++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c index c1584930861..f7827b31635 100644 --- a/dlls/winex11.drv/ime.c +++ b/dlls/winex11.drv/ime.c @@ -1050,8 +1050,8 @@ void IME_SetResultString(LPWSTR lpResult, DWORD dwResultLen) GenerateIMEMessage(imc, WM_IME_COMPOSITION, lpResult[0], GCS_RESULTSTR|GCS_RESULTCLAUSE); GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION, 0, 0);
- if (!inComp) - ImmSetOpenStatus(imc, FALSE); + myPrivate->bInComposition = FALSE; + ImmSetOpenStatus(imc, FALSE);
ImmUnlockIMC(imc); } I'm not sure about this change. It always turns off IME open status. Since composition might be continuous at this moment, it can't be set unconditionally. If we should do so for some reasons, could you explain in detail?
diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index f389f3e0836..8ce3a7e8528 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h [..] --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -117,6 +117,19 @@ void X11DRV_XIMLookupChars( const char *str, DWORD count )
IME_SetResultString(wcOutput, dwOutput); HeapFree(GetProcessHeap(), 0, wcOutput); + + /* + * In a special case for Hangul(Korean), when Composition is completed and + * this function is called, the new Composition disappears. so, calling + * IME_SetCompositionString again for makes composition will be + * starting. + */ + if (CompositionString && dwCompStringLength >= sizeof(WCHAR) && + IsHangul(((const WCHAR*) CompositionString)[0])) + { + IME_SetCompositionString(SCS_SETSTR, CompositionString, + dwCompStringLength, NULL, 0); + } }
static BOOL XIMPreEditStateNotifyCallback(XIC xic, XPointer p, XPointer data) Could you revert Hangul(Korean) specific changes? This issue seems not to be Korean specific because I can occasionally reproduce it with Japanese input method.
From my point of view, the main cause of this issue is that the order of PreeditCallbacks and XmbLookupString() aren't stable. The current wine's implementation seems to rely on getting the result of XmbLookupString() before PreeditCallback for the next clause. However, when one or more clauses are left after commiting a clause, PreeditCallback for the left clauses sometimes (or always in Hangul?) called before getting the result of XmbLookupString. What do you think about these XIM situations? Does your patch works well for both cases?
Regards, Akihiro Sagawa
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=111635 Your paranoid android. === debian11 (build log) === Task: Patch failed to apply === debian11 (build log) === Task: Patch failed to apply
A new patch has been submitted and will no longer be used. Regards, Alex 2022-03-20 오후 9:17에 Alex Kwak 이(가) 쓴 글:
In a special case for Hangul(Korean), when Composition is completed and this function is called, the new Composition disappears. so, calling IME_SetCompositionString again for makes composition will be starting.
Signed-off-by: Alex Kwak <take-me-home(a)kakao.com> --- dlls/winex11.drv/ime.c | 4 ++-- dlls/winex11.drv/x11drv.h | 27 +++++++++++++++++++++++++++ dlls/winex11.drv/xim.c | 13 +++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/dlls/winex11.drv/ime.c b/dlls/winex11.drv/ime.c index c1584930861..f7827b31635 100644 --- a/dlls/winex11.drv/ime.c +++ b/dlls/winex11.drv/ime.c @@ -1050,8 +1050,8 @@ void IME_SetResultString(LPWSTR lpResult, DWORD dwResultLen) GenerateIMEMessage(imc, WM_IME_COMPOSITION, lpResult[0], GCS_RESULTSTR|GCS_RESULTCLAUSE); GenerateIMEMessage(imc, WM_IME_ENDCOMPOSITION, 0, 0);
- if (!inComp) - ImmSetOpenStatus(imc, FALSE); + myPrivate->bInComposition = FALSE; + ImmSetOpenStatus(imc, FALSE);
ImmUnlockIMC(imc); } diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index f389f3e0836..8ce3a7e8528 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -841,4 +841,31 @@ static inline BOOL is_window_rect_mapped( const RECT *rect ) max( rect->bottom, rect->top + 1 ) > virtual_rect.top); }
+#define HANGUL_BASE 0xac00 +#define HANGUL_LAST 0xd7a3 +#define HANGUL_JAMO_BASE 0x1100 +#define HANGUL_JAMO_LAST 0x11F9 +#define HANGUL_COMPAT_JAMO_BASE 0x3130 +#define HANGUL_COMPAT_JAMO_LAST 0x318E +#define HANGUL_LCOUNT 19 +#define HANGUL_VCOUNT 21 +#define HANGUL_TCOUNT 28 +#define HANGUL_NCOUNT (HANGUL_VCOUNT * HANGUL_TCOUNT) +#define HANGUL_SCOUNT (HANGUL_LCOUNT * HANGUL_NCOUNT) + +/* + * This method exists to handle special cases related to Korean(Hangul). + */ +static inline BOOL IsHangul(WPARAM wParam) +{ + if (!wParam) + { + return FALSE; + } + + return (wParam >= HANGUL_BASE && wParam < HANGUL_LAST) || + (wParam >= HANGUL_COMPAT_JAMO_BASE && wParam < HANGUL_COMPAT_JAMO_LAST) || + (wParam >= HANGUL_JAMO_BASE && wParam < HANGUL_JAMO_LAST); +} + #endif /* __WINE_X11DRV_H */ diff --git a/dlls/winex11.drv/xim.c b/dlls/winex11.drv/xim.c index 3994c2106cc..884db67c59c 100644 --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -117,6 +117,19 @@ void X11DRV_XIMLookupChars( const char *str, DWORD count )
IME_SetResultString(wcOutput, dwOutput); HeapFree(GetProcessHeap(), 0, wcOutput); + + /* + * In a special case for Hangul(Korean), when Composition is completed and + * this function is called, the new Composition disappears. so, calling + * IME_SetCompositionString again for makes composition will be + * starting. + */ + if (CompositionString && dwCompStringLength >= sizeof(WCHAR) && + IsHangul(((const WCHAR*) CompositionString)[0])) + { + IME_SetCompositionString(SCS_SETSTR, CompositionString, + dwCompStringLength, NULL, 0); + } }
static BOOL XIMPreEditStateNotifyCallback(XIC xic, XPointer p, XPointer data)
participants (4)
-
Akihiro Sagawa -
Alex Kwak -
Alex Kwak -
Marvin