[PATCH v6 0/5] MR2698: winex11: Avoid breaking XIM XFilterEvent / XmbLookupString sequence.
-- v6: 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.
From: Rémi Bernon rbernon@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 c3c8d9a4070..f4cafd679b5 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 *
From: Rémi Bernon rbernon@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 f4cafd679b5..bd917ae49c9 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.@) */
From: Rémi Bernon rbernon@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 bd917ae49c9..5018a8f8223 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;
From: Rémi Bernon rbernon@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 5018a8f8223..79b5cbe501b 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 );
From: Rémi Bernon rbernon@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 79b5cbe501b..87a1b111fe0 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;
I have split the IME general fixes to https://gitlab.winehq.org/wine/wine/-/merge_requests/3115, with more fixes to the composition string updates.
I only kept the XIM call sequence breakage here, and although !3115 makes it a little bit less visible, X11 event merging is still causing an inversion and missing composition strings.
This is the case for instance when typing Japanese "nihongo no", when 'n' is entered after the ' ' it is sometimes missing, because the result string is committed from a delayed X11DRV_KeyEvent call, after the preedit update has already been received from the XFilterEvent call.
This merge request was closed by Rémi Bernon.
-
Rémi Bernon -
Rémi Bernon (＠rbernon)