[PATCH 0/3] MR6286: win32u: Support setting host IME composition window position for ImmSetCompositionWindow().
Based on the wine-staging patch winex11-CandidateWindowPos from Felix Yan with works from Muneyuki Noguchi and Sebastian Lackner. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6286
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Based on the wine-staging patch winex11-CandidateWindowPos from Felix Yan with works from Muneyuki Noguchi and Sebastian Lackner. --- dlls/imm32/imm.c | 17 ++++++++++++++ dlls/win32u/driver.c | 8 +++++++ dlls/win32u/input.c | 8 +++++++ dlls/win32u/sysparams.c | 3 +++ dlls/win32u/win32u_private.h | 1 + dlls/winex11.drv/init.c | 1 + dlls/winex11.drv/x11drv.h | 1 + dlls/winex11.drv/xim.c | 45 ++++++++++++++++++++++++++++++++++++ include/ntuser.h | 1 + include/wine/gdi_driver.h | 2 ++ 10 files changed, 87 insertions(+) diff --git a/dlls/imm32/imm.c b/dlls/imm32/imm.c index cab531e4342..42d171864cc 100644 --- a/dlls/imm32/imm.c +++ b/dlls/imm32/imm.c @@ -2637,6 +2637,7 @@ BOOL WINAPI ImmSetCompositionStringW( BOOL WINAPI ImmSetCompositionWindow( HIMC himc, COMPOSITIONFORM *composition ) { INPUTCONTEXT *ctx; + POINT point; TRACE( "himc %p, composition %s\n", himc, debugstr_composition( composition ) ); @@ -2649,6 +2650,22 @@ BOOL WINAPI ImmSetCompositionWindow( HIMC himc, COMPOSITIONFORM *composition ) ImmNotifyIME( himc, NI_CONTEXTUPDATED, 0, IMC_SETCOMPOSITIONWINDOW ); SendMessageW( ctx->hWnd, WM_IME_NOTIFY, IMN_SETCOMPOSITIONWINDOW, 0 ); + if (composition->dwStyle & (CFS_RECT | CFS_POINT | CFS_FORCE_POSITION)) + { + if (composition->dwStyle & CFS_RECT) + { + point.x = composition->rcArea.left; + point.y = composition->rcArea.top; + } + else + { + point = composition->ptCurrentPos; + } + + NtUserLogicalToPerMonitorDPIPhysicalPoint( ctx->hWnd, &point ); + NtUserCallTwoParam( (ULONG_PTR)ctx->hWnd, (ULONG_PTR)&point, NtUserCallTwoParam_SetIMECompositionWindowPos ); + } + ImmUnlockIMC( himc ); return TRUE; diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index 362f32b93d7..a13afb6d20f 100644 --- a/dlls/win32u/driver.c +++ b/dlls/win32u/driver.c @@ -913,6 +913,11 @@ static void nulldrv_ThreadDetach( void ) { } +static BOOL nulldrv_SetIMECompositionWindowPos( HWND hwnd, const POINT *point ) +{ + return FALSE; +} + static const WCHAR guid_key_prefixW[] = { '\\','R','e','g','i','s','t','r','y', @@ -1293,6 +1298,8 @@ static const struct user_driver_funcs lazy_load_driver = nulldrv_wine_get_wgl_driver, /* thread management */ nulldrv_ThreadDetach, + /* IME support */ + nulldrv_SetIMECompositionWindowPos, }; const struct user_driver_funcs *user_driver = &lazy_load_driver; @@ -1376,6 +1383,7 @@ void __wine_set_user_driver( const struct user_driver_funcs *funcs, UINT version SET_USER_FUNC(VulkanInit); SET_USER_FUNC(wine_get_wgl_driver); SET_USER_FUNC(ThreadDetach); + SET_USER_FUNC(SetIMECompositionWindowPos); #undef SET_USER_FUNC prev = InterlockedCompareExchangePointer( (void **)&user_driver, driver, (void *)&lazy_load_driver ); diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index 1cf174760da..e4a4ffe516f 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -2351,6 +2351,14 @@ BOOL WINAPI NtUserGetCaretPos( POINT *pt ) return ret; } +BOOL set_ime_composition_window_pos( HWND hwnd, const POINT *point ) +{ + if (!NtUserIsWindow( hwnd )) + return FALSE; + + return user_driver->pSetIMECompositionWindowPos( hwnd, point ); +} + /******************************************************************* * set_caret_pos */ diff --git a/dlls/win32u/sysparams.c b/dlls/win32u/sysparams.c index afa75928c94..1b830722128 100644 --- a/dlls/win32u/sysparams.c +++ b/dlls/win32u/sysparams.c @@ -6541,6 +6541,9 @@ ULONG_PTR WINAPI NtUserCallTwoParam( ULONG_PTR arg1, ULONG_PTR arg2, ULONG code case NtUserCallTwoParam_SetIconParam: return set_icon_param( UlongToHandle(arg1), arg2 ); + case NtUserCallTwoParam_SetIMECompositionWindowPos: + return set_ime_composition_window_pos( UlongToHandle(arg1), (const POINT *)arg2 ); + case NtUserCallTwoParam_UnhookWindowsHook: return unhook_windows_hook( arg1, (HOOKPROC)arg2 ); diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index fc0f0c62c93..a1f11215294 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -102,6 +102,7 @@ extern BOOL set_capture_window( HWND hwnd, UINT gui_flags, HWND *prev_ret ); extern BOOL set_caret_blink_time( unsigned int time ); extern BOOL set_caret_pos( int x, int y ); extern BOOL set_foreground_window( HWND hwnd, BOOL mouse ); +extern BOOL set_ime_composition_window_pos( HWND hwnd, const POINT *point ); extern void toggle_caret( HWND hwnd ); extern void update_mouse_tracking_info( HWND hwnd ); extern BOOL get_clip_cursor( RECT *rect, UINT dpi ); diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c index 4a78073cb9b..4a01aca7e28 100644 --- a/dlls/winex11.drv/init.c +++ b/dlls/winex11.drv/init.c @@ -429,6 +429,7 @@ static const struct user_driver_funcs x11drv_funcs = .pVulkanInit = X11DRV_VulkanInit, .pwine_get_wgl_driver = X11DRV_wine_get_wgl_driver, .pThreadDetach = X11DRV_ThreadDetach, + .pSetIMECompositionWindowPos = X11DRV_SetIMECompositionWindowPos, }; diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index eddbc74a726..97d379c6341 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -253,6 +253,7 @@ extern void X11DRV_WindowPosChanged( HWND hwnd, HWND insert_after, UINT swp_flag extern BOOL X11DRV_SystemParametersInfo( UINT action, UINT int_param, void *ptr_param, UINT flags ); extern void X11DRV_ThreadDetach(void); +extern BOOL X11DRV_SetIMECompositionWindowPos( HWND hwnd, const POINT *point ); /* X11 driver internal functions */ diff --git a/dlls/winex11.drv/xim.c b/dlls/winex11.drv/xim.c index c6a93eb5e16..9c271df5d00 100644 --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -488,3 +488,48 @@ void xim_set_focus( HWND hwnd, BOOL focus ) if (focus) XSetICFocus( xic ); else XUnsetICFocus( xic ); } + +BOOL X11DRV_SetIMECompositionWindowPos( HWND hwnd, const POINT *point ) +{ + struct x11drv_win_data *data; + XVaNestedList attr; + XPoint xpoint; + HWND parent; + POINT pt; + + if (!(input_style & XIMPreeditPosition)) + return FALSE; + + for (parent = hwnd; parent && parent != NtUserGetDesktopWindow(); + parent = NtUserGetAncestor( parent, GA_PARENT )) + { + if (!(data = get_win_data( parent ))) + continue; + if (!data->xic) + { + release_win_data( data ); + continue; + } + + pt = *point; + + if (hwnd != data->hwnd) + NtUserMapWindowPoints( hwnd, data->hwnd, &pt, 1, 0 /* per-monitor DPI */ ); + + if (NtUserGetWindowLongW( data->hwnd, GWL_EXSTYLE ) & WS_EX_LAYOUTRTL) + pt.x = data->client_rect.right - data->client_rect.left - 1 - pt.x; + + xpoint.x = pt.x + data->client_rect.left - data->whole_rect.left; + xpoint.y = pt.y + data->client_rect.top - data->whole_rect.top; + attr = XVaCreateNestedList( 0, XNSpotLocation, &xpoint, NULL ); + if (attr) + { + XSetICValues( data->xic, XNPreeditAttributes, attr, NULL ); + XFree( attr ); + } + + release_win_data( data ); + } + + return TRUE; +} diff --git a/include/ntuser.h b/include/ntuser.h index 4148c8cf0b6..70efa44f85c 100644 --- a/include/ntuser.h +++ b/include/ntuser.h @@ -1053,6 +1053,7 @@ enum NtUserCallTwoParam_MonitorFromRect, NtUserCallTwoParam_SetCaretPos, NtUserCallTwoParam_SetIconParam, + NtUserCallTwoParam_SetIMECompositionWindowPos, NtUserCallTwoParam_UnhookWindowsHook, NtUserCallTwoParam_AdjustWindowRect, NtUserCallTwoParam_IsWindowRectFullScreen, diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h index 8eec0ada4ca..28c64afed86 100644 --- a/include/wine/gdi_driver.h +++ b/include/wine/gdi_driver.h @@ -356,6 +356,8 @@ struct user_driver_funcs struct opengl_funcs * (*pwine_get_wgl_driver)(UINT); /* thread management */ void (*pThreadDetach)(void); + /* IME support */ + BOOL (*pSetIMECompositionWindowPos)(HWND, const POINT *); }; W32KAPI void __wine_set_user_driver( const struct user_driver_funcs *funcs, UINT version ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6286
From: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/win32u/input.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index e4a4ffe516f..cfe0d9ddc46 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -2367,6 +2367,7 @@ BOOL set_caret_pos( int x, int y ) int old_state = 0; int hidden = 0; HWND hwnd = 0; + POINT pt; BOOL ret; RECT r; @@ -2396,7 +2397,10 @@ BOOL set_caret_pos( int x, int y ) r.bottom += y - r.top; r.left = x; r.top = y; + pt.x = x; + pt.y = y; display_caret( hwnd, &r ); + set_ime_composition_window_pos( hwnd, &pt ); NtUserSetSystemTimer( hwnd, SYSTEM_TIMER_CARET, caret.timeout ); } return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6286
From: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/win32u/input.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index cfe0d9ddc46..f773c099114 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -2412,6 +2412,7 @@ BOOL set_caret_pos( int x, int y ) BOOL WINAPI NtUserShowCaret( HWND hwnd ) { int hidden = 0; + POINT pt; BOOL ret; RECT r; @@ -2434,7 +2435,10 @@ BOOL WINAPI NtUserShowCaret( HWND hwnd ) if (ret && hidden == 1) /* hidden was 1 so it's now 0 */ { + pt.x = r.left; + pt.y = r.top; display_caret( hwnd, &r ); + set_ime_composition_window_pos( hwnd, &pt ); NtUserSetSystemTimer( hwnd, SYSTEM_TIMER_CARET, caret.timeout ); } return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6286
Have you tried different IM engines with it? I've had bad experience in the past with XNSpotLocation. My conclusion was that instead of relying on bogus and inconsistent host IM engines we should be improving our own composition window. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6286#note_78932
On Fri Aug 16 08:17:38 2024 +0000, Rémi Bernon wrote:
Have you tried different IM engines with it? I've had bad experience in the past with XNSpotLocation. My conclusion was that instead of relying on bogus and inconsistent host IM engines we should be improving our own composition window. I tried iBus and Fcitx 5. With iBus, before and after this MR, the composition window of iBus is below the window and Wine's composition window is at the caret position. For Fcitx 5, the Fcitx composition window is at the caret position and no wine composition window. Wine already sets (0,0) XNSpotLocation in xic_create(). I think this MR should be an improvement over what we have now.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6286#note_78934
On Fri Aug 16 08:17:38 2024 +0000, Zhiyi Zhang wrote:
I tried iBus and Fcitx 5. With iBus, before and after this MR, the composition window of iBus is below the window and Wine's composition window is at the caret position. For Fcitx 5, the Fcitx composition window is at the caret position and no wine composition window. Wine already sets (0,0) XNSpotLocation in xic_create(). I think this MR should be an improvement over what we have now. Maybe, but it adds more complexity and some backend-specific helpers without improving the common case.
Wine already sets (0,0) XNSpotLocation in xic_create().
Yes, but I had issues when calling it later to move the window, like you're doing here. I don't remember precisely which IME was involved though, I'll need to check again. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6286#note_78938
Rémi Bernon (@rbernon) commented about dlls/winex11.drv/xim.c:
+ if (!data->xic) + { + release_win_data( data ); + continue; + } + + pt = *point; + + if (hwnd != data->hwnd) + NtUserMapWindowPoints( hwnd, data->hwnd, &pt, 1, 0 /* per-monitor DPI */ ); + + if (NtUserGetWindowLongW( data->hwnd, GWL_EXSTYLE ) & WS_EX_LAYOUTRTL) + pt.x = data->client_rect.right - data->client_rect.left - 1 - pt.x; + + xpoint.x = pt.x + data->client_rect.left - data->whole_rect.left; + xpoint.y = pt.y + data->client_rect.top - data->whole_rect.top; XIC are only created on top-level windows, it would be better lookup the top-level window and do that point mapping in win32u.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6286#note_79086
Rémi Bernon (@rbernon) commented about dlls/imm32/imm.c:
ImmNotifyIME( himc, NI_CONTEXTUPDATED, 0, IMC_SETCOMPOSITIONWINDOW ); SendMessageW( ctx->hWnd, WM_IME_NOTIFY, IMN_SETCOMPOSITIONWINDOW, 0 );
+ if (composition->dwStyle & (CFS_RECT | CFS_POINT | CFS_FORCE_POSITION)) + { + if (composition->dwStyle & CFS_RECT) + { + point.x = composition->rcArea.left; + point.y = composition->rcArea.top; + } + else + { + point = composition->ptCurrentPos; + } + + NtUserLogicalToPerMonitorDPIPhysicalPoint( ctx->hWnd, &point );
I think there's some DPI inconsistencies for the point parameter. Here you map it to window monitor DPI coordinates, while I believe SetCaretPos and ShowCaret coordinates are in caret window logical coordinates. It would be better to always pass the point in hwnd logical coordinates and map DPI in win32u before calling into the drivers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6286#note_79087
Rémi Bernon (@rbernon) commented about include/wine/gdi_driver.h:
struct opengl_funcs * (*pwine_get_wgl_driver)(UINT); /* thread management */ void (*pThreadDetach)(void); + /* IME support */ + BOOL (*pSetIMECompositionWindowPos)(HWND, const POINT *);
You need to bump `WINE_GDI_DRIVER_VERSION` too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6286#note_79088
On Mon Aug 19 09:28:33 2024 +0000, Rémi Bernon wrote:
Maybe, but it adds more complexity and some backend-specific calls without improving the common case.
Wine already sets (0,0) XNSpotLocation in xic_create(). Yes, but I had issues when calling it later to move the window, like you're doing here. I don't remember precisely which IME was involved though, I'll need to check again. Fwiw I tried a couple of IME, although it's always a struggle and I couldn't make SCIM work, and wasn't able to reproduce the issue I was seeing. Maybe it's gone or maybe is was something I was doing incorrectly.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6286#note_79089
participants (3)
-
Rémi Bernon -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)