[PATCH v5 0/6] MR2698: winex11: Avoid breaking XIM XFilterEvent / XmbLookupString sequence.
-- v5: winex11: Report empty preedit string when result string is committed. winex11: Avoid breaking XIM sequence when merging mouse motion events. winex11: Avoid breaking XIM sequence when merging ConfigureNotify events. winex11: Introduce new handle_delayed_event helper. winex11: Move call_event_handler function around. winex11: Remove unnecessary process_events helper. https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 55709a0bc5f..56e4c1d5115 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -399,17 +399,23 @@ static inline BOOL call_event_handler( Display *display, XEvent *event ) /*********************************************************************** - * process_events + * ProcessEvents (X11DRV.@) */ -static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,XPointer), ULONG_PTR arg ) +BOOL X11DRV_ProcessEvents( DWORD mask ) { XEvent event, prev_event; + struct x11drv_thread_data *thread_data; + Display *display; int count = 0; BOOL queued = FALSE; enum event_merge_action action = MERGE_DISCARD; + if (!(thread_data = x11drv_thread_data())) return FALSE; + if (thread_data->current_event) mask = 0; /* don't process nested events */ + display = thread_data->display; + prev_event.type = 0; - while (XCheckIfEvent( display, &event, filter, (char *)arg )) + while (XCheckIfEvent( display, &event, filter_event, (XPointer)(UINT_PTR)mask )) { count++; if (XFilterEvent( &event, None )) @@ -469,19 +475,6 @@ static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,X } -/*********************************************************************** - * ProcessEvents (X11DRV.@) - */ -BOOL X11DRV_ProcessEvents( DWORD mask ) -{ - struct x11drv_thread_data *data = x11drv_thread_data(); - - if (!data) return FALSE; - if (data->current_event) mask = 0; /* don't process nested events */ - - return process_events( data->display, filter_event, mask ); -} - /*********************************************************************** * EVENT_x11_time_to_win32_time * -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 68 ++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 56e4c1d5115..35bb0460287 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -256,6 +256,40 @@ static Bool filter_event( Display *display, XEvent *event, char *arg ) } +/*********************************************************************** + * call_event_handler + */ +static inline BOOL call_event_handler( Display *display, XEvent *event ) +{ + HWND hwnd; + XEvent *prev; + struct x11drv_thread_data *thread_data; + BOOL ret; + + if (!handlers[event->type]) + { + TRACE( "%s for win %lx, ignoring\n", dbgstr_event( event->type ), event->xany.window ); + return FALSE; /* no handler, ignore it */ + } + +#ifdef GenericEvent + if (event->type == GenericEvent) hwnd = 0; else +#endif + if (XFindContext( display, event->xany.window, winContext, (char **)&hwnd ) != 0) + hwnd = 0; /* not for a registered window */ + if (!hwnd && event->xany.window == root_window) hwnd = NtUserGetDesktopWindow(); + + TRACE( "%lu %s for hwnd/window %p/%lx\n", + event->xany.serial, dbgstr_event( event->type ), hwnd, event->xany.window ); + thread_data = x11drv_thread_data(); + prev = thread_data->current_event; + thread_data->current_event = event; + ret = handlers[event->type]( hwnd, event ); + thread_data->current_event = prev; + return ret; +} + + enum event_merge_action { MERGE_DISCARD, /* discard the old event */ @@ -364,40 +398,6 @@ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) } -/*********************************************************************** - * call_event_handler - */ -static inline BOOL call_event_handler( Display *display, XEvent *event ) -{ - HWND hwnd; - XEvent *prev; - struct x11drv_thread_data *thread_data; - BOOL ret; - - if (!handlers[event->type]) - { - TRACE( "%s for win %lx, ignoring\n", dbgstr_event( event->type ), event->xany.window ); - return FALSE; /* no handler, ignore it */ - } - -#ifdef GenericEvent - if (event->type == GenericEvent) hwnd = 0; else -#endif - if (XFindContext( display, event->xany.window, winContext, (char **)&hwnd ) != 0) - hwnd = 0; /* not for a registered window */ - if (!hwnd && event->xany.window == root_window) hwnd = NtUserGetDesktopWindow(); - - TRACE( "%lu %s for hwnd/window %p/%lx\n", - event->xany.serial, dbgstr_event( event->type ), hwnd, event->xany.window ); - thread_data = x11drv_thread_data(); - prev = thread_data->current_event; - thread_data->current_event = event; - ret = handlers[event->type]( hwnd, event ); - thread_data->current_event = prev; - return ret; -} - - /*********************************************************************** * ProcessEvents (X11DRV.@) */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 35bb0460287..5851a8d65b9 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -289,6 +289,16 @@ static inline BOOL call_event_handler( Display *display, XEvent *event ) return ret; } +static BOOL handle_delayed_event( Display *display, XEvent *event ) +{ + BOOL queued = FALSE; + if (!event->type) return FALSE; + TRACE( "Handling delayed %s event for window %lx\n", dbgstr_event(event->type), event->xany.window ); + queued |= call_event_handler( display, event ); + free_event_data( event ); + event->type = 0; + return queued; +} enum event_merge_action { @@ -453,8 +463,9 @@ BOOL X11DRV_ProcessEvents( DWORD mask ) switch( action ) { case MERGE_HANDLE: /* handle prev, keep new */ - queued |= call_event_handler( display, &prev_event ); - /* fall through */ + queued |= handle_delayed_event( display, &prev_event ); + prev_event = event; + break; case MERGE_DISCARD: /* discard prev, keep new */ free_event_data( &prev_event ); prev_event = event; @@ -467,8 +478,7 @@ BOOL X11DRV_ProcessEvents( DWORD mask ) break; } } - if (prev_event.type) queued |= call_event_handler( display, &prev_event ); - free_event_data( &prev_event ); + queued |= handle_delayed_event( display, &prev_event ); XFlush( gdi_display ); if (count) TRACE( "processed %d events, returning %d\n", count, queued ); return queued; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 5851a8d65b9..8f59a921d74 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -348,6 +348,24 @@ static enum event_merge_action merge_raw_motion_events( XIRawEvent *prev, XIRawE } #endif +/* merge ConfigureNotify events to reduce the chances of window resize feedback loops */ +static BOOL merge_configure_events( Display *display, XEvent *prev, XEvent *next ) +{ + BOOL queued = FALSE; + + /* allow merging ConfigureNotify across Expose and PropertyNotify events */ + if (next->type == Expose || next->type == PropertyNotify) return FALSE; + if (next->type != ConfigureNotify) return handle_delayed_event( display, prev ); + + if (!prev->type || prev->xany.window != next->xany.window) queued |= handle_delayed_event( display, prev ); + else WARN( "discarding duplicate ConfigureNotify for window %lx\n", next->xany.window ); + + TRACE( "delaying ConfigureNotify for window %lx for merging\n", next->xany.window ); + *prev = *next; + next->type = 0; + return queued; +} + /*********************************************************************** * merge_events * @@ -357,21 +375,6 @@ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) { switch (prev->type) { - case ConfigureNotify: - switch (next->type) - { - case ConfigureNotify: - if (prev->xany.window == next->xany.window) - { - TRACE( "discarding duplicate ConfigureNotify for window %lx\n", prev->xany.window ); - return MERGE_DISCARD; - } - break; - case Expose: - case PropertyNotify: - return MERGE_KEEP; - } - break; case MotionNotify: switch (next->type) { @@ -413,7 +416,7 @@ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) */ BOOL X11DRV_ProcessEvents( DWORD mask ) { - XEvent event, prev_event; + XEvent event, prev_configure = {0}, prev_event; struct x11drv_thread_data *thread_data; Display *display; int count = 0; @@ -459,7 +462,9 @@ BOOL X11DRV_ProcessEvents( DWORD mask ) continue; /* filtered, ignore it */ } get_event_data( &event ); - if (prev_event.type) action = merge_events( &prev_event, &event ); + queued |= merge_configure_events( display, &prev_configure, &event ); + if (!event.type) action = MERGE_IGNORE; + else if (prev_event.type) action = merge_events( &prev_event, &event ); switch( action ) { case MERGE_HANDLE: /* handle prev, keep new */ @@ -478,6 +483,7 @@ BOOL X11DRV_ProcessEvents( DWORD mask ) break; } } + queued |= handle_delayed_event( display, &prev_configure ); queued |= handle_delayed_event( display, &prev_event ); XFlush( gdi_display ); if (count) TRACE( "processed %d events, returning %d\n", count, queued ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 183 ++++++++++++++++++--------------------- 1 file changed, 84 insertions(+), 99 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 8f59a921d74..a252e5137d6 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -300,54 +300,100 @@ static BOOL handle_delayed_event( Display *display, XEvent *event ) return queued; } -enum event_merge_action +static BOOL is_mergeable_raw_motion( XEvent *event ) { - MERGE_DISCARD, /* discard the old event */ - MERGE_HANDLE, /* handle the old event */ - MERGE_KEEP, /* keep the old event for future merging */ - MERGE_IGNORE /* ignore the new event, keep the old one */ -}; +#ifdef HAVE_X11_EXTENSIONS_XINPUT2_H + if (event->type != GenericEvent) return FALSE; + if (event->xcookie.extension != xinput2_opcode) return FALSE; + if (event->xcookie.evtype != XI_RawMotion) return FALSE; + if (x11drv_thread_data()->warp_serial) return FALSE; + return TRUE; +#endif + return FALSE; +} -/*********************************************************************** - * merge_raw_motion_events - */ #ifdef HAVE_X11_EXTENSIONS_XINPUT2_H -static enum event_merge_action merge_raw_motion_events( XIRawEvent *prev, XIRawEvent *next ) +static void merge_xi_valuator_states( XIValuatorState *dst, const XIValuatorState *src ) { int i, j, k; + for (i = j = k = 0; i < 8; i++) + { + if (XIMaskIsSet( src->mask, i )) dst->values[j] += src->values[k++]; + if (XIMaskIsSet( dst->mask, i )) j++; + } +} + +static int merge_xi_raw_events( XIRawEvent *prev, XIRawEvent *next ) +{ unsigned char mask; - if (!prev->valuators.mask_len) return MERGE_HANDLE; - if (!next->valuators.mask_len) return MERGE_HANDLE; + if (!prev->valuators.mask_len) return 1; /* keep next */ + if (!next->valuators.mask_len) return -1; /* keep prev */ mask = prev->valuators.mask[0] | next->valuators.mask[0]; - if (mask == next->valuators.mask[0]) /* keep next */ + if (mask == next->valuators.mask[0]) { - for (i = j = k = 0; i < 8; i++) - { - if (XIMaskIsSet( prev->valuators.mask, i )) - next->valuators.values[j] += prev->valuators.values[k++]; - if (XIMaskIsSet( next->valuators.mask, i )) j++; - } - TRACE( "merging duplicate GenericEvent\n" ); - return MERGE_DISCARD; + merge_xi_valuator_states( &next->valuators, &prev->valuators ); + return 1; /* keep next */ } - if (mask == prev->valuators.mask[0]) /* keep prev */ + if (mask == prev->valuators.mask[0]) { - for (i = j = k = 0; i < 8; i++) - { - if (XIMaskIsSet( next->valuators.mask, i )) - prev->valuators.values[j] += next->valuators.values[k++]; - if (XIMaskIsSet( prev->valuators.mask, i )) j++; - } - TRACE( "merging duplicate GenericEvent\n" ); - return MERGE_IGNORE; + merge_xi_valuator_states( &prev->valuators, &next->valuators ); + return -1; /* keep prev */ } /* can't merge events with disjoint masks */ - return MERGE_HANDLE; + return 0; } #endif +/* merge XIRawEvent events when process or wineserver don't keep up */ +static BOOL merge_raw_motion_events( Display *display, XEvent *prev, XEvent *next ) +{ +#ifdef HAVE_X11_EXTENSIONS_XINPUT2_H + BOOL queued = FALSE; + int ret; + + /* allow merging XI_RawMotion across MotionNotify events */ + if (next->type == MotionNotify) return FALSE; + if (!is_mergeable_raw_motion( next )) return handle_delayed_event( display, prev ); + + if (!is_mergeable_raw_motion( prev )) ret = 0; + else ret = merge_xi_raw_events( prev->xcookie.data, next->xcookie.data ); + + if (!ret) queued |= handle_delayed_event( display, prev ); + else + { + WARN( "discarding duplicate XIRawEvent for window %lx\n", next->xany.window ); + if (ret != -1) free_event_data( prev ); + else free_event_data( next ); + } + + TRACE( "delaying XIRawEvent for window %lx for merging\n", next->xany.window ); + if (ret != -1) *prev = *next; + next->type = 0; + return queued; +#endif /* HAVE_X11_EXTENSIONS_XINPUT2_H */ + return FALSE; +} + +/* merge MotionNotify events when process or wineserver don't keep up */ +static BOOL merge_motion_events( Display *display, XEvent *prev, XEvent *next ) +{ + BOOL queued = FALSE; + + /* allow merging MotionNotify across XI_RawMotion events */ + if (is_mergeable_raw_motion( next )) return FALSE; + if (next->type != MotionNotify) return handle_delayed_event( display, prev ); + + if (!prev->type || prev->xany.window != next->xany.window) queued |= handle_delayed_event( display, prev ); + else WARN( "discarding duplicate MotionNotify for window %lx\n", next->xany.window ); + + TRACE( "delaying MotionNotify for window %lx for merging\n", next->xany.window ); + *prev = *next; + next->type = 0; + return queued; +} + /* merge ConfigureNotify events to reduce the chances of window resize feedback loops */ static BOOL merge_configure_events( Display *display, XEvent *prev, XEvent *next ) { @@ -366,68 +412,21 @@ static BOOL merge_configure_events( Display *display, XEvent *prev, XEvent *next return queued; } -/*********************************************************************** - * merge_events - * - * Try to merge 2 consecutive events. - */ -static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) -{ - switch (prev->type) - { - case MotionNotify: - switch (next->type) - { - case MotionNotify: - if (prev->xany.window == next->xany.window) - { - TRACE( "discarding duplicate MotionNotify for window %lx\n", prev->xany.window ); - return MERGE_DISCARD; - } - break; -#ifdef HAVE_X11_EXTENSIONS_XINPUT2_H - case GenericEvent: - if (next->xcookie.extension != xinput2_opcode) break; - if (next->xcookie.evtype != XI_RawMotion) break; - if (x11drv_thread_data()->warp_serial) break; - return MERGE_KEEP; - } - break; - case GenericEvent: - if (prev->xcookie.extension != xinput2_opcode) break; - if (prev->xcookie.evtype != XI_RawMotion) break; - switch (next->type) - { - case GenericEvent: - if (next->xcookie.extension != xinput2_opcode) break; - if (next->xcookie.evtype != XI_RawMotion) break; - if (x11drv_thread_data()->warp_serial) break; - return merge_raw_motion_events( prev->xcookie.data, next->xcookie.data ); -#endif - } - break; - } - return MERGE_HANDLE; -} - - /*********************************************************************** * ProcessEvents (X11DRV.@) */ BOOL X11DRV_ProcessEvents( DWORD mask ) { - XEvent event, prev_configure = {0}, prev_event; + XEvent event, prev_configure = {0}, prev_motion = {0}, prev_raw_motion = {0}; struct x11drv_thread_data *thread_data; Display *display; int count = 0; BOOL queued = FALSE; - enum event_merge_action action = MERGE_DISCARD; if (!(thread_data = x11drv_thread_data())) return FALSE; if (thread_data->current_event) mask = 0; /* don't process nested events */ display = thread_data->display; - prev_event.type = 0; while (XCheckIfEvent( display, &event, filter_event, (XPointer)(UINT_PTR)mask )) { count++; @@ -463,28 +462,14 @@ BOOL X11DRV_ProcessEvents( DWORD mask ) } get_event_data( &event ); queued |= merge_configure_events( display, &prev_configure, &event ); - if (!event.type) action = MERGE_IGNORE; - else if (prev_event.type) action = merge_events( &prev_event, &event ); - switch( action ) - { - case MERGE_HANDLE: /* handle prev, keep new */ - queued |= handle_delayed_event( display, &prev_event ); - prev_event = event; - break; - case MERGE_DISCARD: /* discard prev, keep new */ - free_event_data( &prev_event ); - prev_event = event; - break; - case MERGE_KEEP: /* handle new, keep prev for future merging */ - queued |= call_event_handler( display, &event ); - /* fall through */ - case MERGE_IGNORE: /* ignore new, keep prev for future merging */ - free_event_data( &event ); - break; - } + if (event.type) queued |= merge_raw_motion_events( display, &prev_raw_motion, &event ); + if (event.type) queued |= merge_motion_events( display, &prev_motion, &event ); + if (event.type) queued |= call_event_handler( display, &event ); + free_event_data( &event ); } queued |= handle_delayed_event( display, &prev_configure ); - queued |= handle_delayed_event( display, &prev_event ); + queued |= handle_delayed_event( display, &prev_raw_motion ); + queued |= handle_delayed_event( display, &prev_motion ); XFlush( gdi_display ); if (count) TRACE( "processed %d events, returning %d\n", count, queued ); return queued; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
From: Rémi Bernon <rbernon(a)codeweavers.com> Based on a patch from Byeong-Sik Jeon <bsjeon(a)hanmail.net>. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55027 --- dlls/winex11.drv/xim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/winex11.drv/xim.c b/dlls/winex11.drv/xim.c index 1c3d2dd9875..1b063cfe886 100644 --- a/dlls/winex11.drv/xim.c +++ b/dlls/winex11.drv/xim.c @@ -140,7 +140,7 @@ void xim_set_result_string( HWND hwnd, const char *str, UINT count ) len = ntdll_umbstowcs( str, count, output, count ); output[len] = 0; - post_ime_update( hwnd, 0, ime_comp_buf, output ); + post_ime_update( hwnd, 0, NULL, output ); free( output ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
Fix a crash introduced while removing process_events, when ProcessEvents is called without a thread data. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2698#note_35795
In the latest artifacts #11413 the VBA issue remains. Next, I personally built only with: * winex11: Make sure HIMC is opened before sending IME updates. * winex11: Report empty preedit string when result string is committed. But still no luck. It looks like VBA code editor really is something special when it comes to IME. Let me know when there's anything I can do to help like providing additional logs. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2698#note_35829
FYI, when I tested VBA editor in !3051, IME result strings would at some probability input correctly without doubling. But in turn, this means at some probability it would still be doubled even though the composition text were sent before the result text. Very strange. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2698#note_35832
participants (2)
-
Masahito Suzuki (＠firelzrd) -
Rémi Bernon