[PATCH 0/2] MR2698: winex11: Avoid breaking XIM XFilterEvent / XmbLookupString sequence.
From: Rémi Bernon <rbernon(a)codeweavers.com> Based on a patch from Byeong-Sik Jeon <bsjeon(a)hanmail.net>. --- dlls/winex11.drv/event.c | 61 ++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 1ae39eb9edf..37f146d3686 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -170,9 +170,10 @@ static inline void get_event_data( XEvent *event ) static inline void free_event_data( XEvent *event ) { #if defined(GenericEvent) && defined(HAVE_XEVENT_XCOOKIE) - if (event->xany.type != GenericEvent) return; - if (event->xcookie.data) pXFreeEventData( event->xany.display, event ); + if (event->xany.type == GenericEvent && event->xcookie.data) + pXFreeEventData( event->xany.display, event ); #endif + event->xany.type = 0; } /*********************************************************************** @@ -260,10 +261,10 @@ static Bool filter_event( Display *display, XEvent *event, char *arg ) enum event_merge_action { - 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 */ + MERGE_HANDLE_OLD = 0x1, + MERGE_FREE_OLD = 0x2, + MERGE_HANDLE_NEW = 0x4, + MERGE_FREE_NEW = 0x8, }; /*********************************************************************** @@ -275,8 +276,8 @@ static enum event_merge_action merge_raw_motion_events( XIRawEvent *prev, XIRawE int i, j, k; 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 MERGE_HANDLE_OLD | MERGE_FREE_OLD; + if (!next->valuators.mask_len) return MERGE_HANDLE_OLD | MERGE_FREE_OLD; mask = prev->valuators.mask[0] | next->valuators.mask[0]; if (mask == next->valuators.mask[0]) /* keep next */ @@ -288,7 +289,7 @@ static enum event_merge_action merge_raw_motion_events( XIRawEvent *prev, XIRawE if (XIMaskIsSet( next->valuators.mask, i )) j++; } TRACE( "merging duplicate GenericEvent\n" ); - return MERGE_DISCARD; + return MERGE_FREE_OLD; } if (mask == prev->valuators.mask[0]) /* keep prev */ { @@ -299,10 +300,10 @@ static enum event_merge_action merge_raw_motion_events( XIRawEvent *prev, XIRawE if (XIMaskIsSet( prev->valuators.mask, i )) j++; } TRACE( "merging duplicate GenericEvent\n" ); - return MERGE_IGNORE; + return MERGE_FREE_NEW; } /* can't merge events with disjoint masks */ - return MERGE_HANDLE; + return MERGE_HANDLE_OLD | MERGE_FREE_OLD; } #endif @@ -313,6 +314,8 @@ static enum event_merge_action merge_raw_motion_events( XIRawEvent *prev, XIRawE */ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) { + enum event_merge_action action = prev->type ? MERGE_HANDLE_OLD | MERGE_FREE_OLD : 0; + switch (prev->type) { case ConfigureNotify: @@ -322,12 +325,12 @@ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) if (prev->xany.window == next->xany.window) { TRACE( "discarding duplicate ConfigureNotify for window %lx\n", prev->xany.window ); - return MERGE_DISCARD; + return MERGE_FREE_OLD; } break; case Expose: case PropertyNotify: - return MERGE_KEEP; + return MERGE_HANDLE_NEW | MERGE_FREE_NEW; } break; case MotionNotify: @@ -337,7 +340,7 @@ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) if (prev->xany.window == next->xany.window) { TRACE( "discarding duplicate MotionNotify for window %lx\n", prev->xany.window ); - return MERGE_DISCARD; + return MERGE_FREE_OLD; } break; #ifdef HAVE_X11_EXTENSIONS_XINPUT2_H @@ -345,7 +348,7 @@ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) if (next->xcookie.extension != xinput2_opcode) break; if (next->xcookie.evtype != XI_RawMotion) break; if (x11drv_thread_data()->warp_serial) break; - return MERGE_KEEP; + return MERGE_HANDLE_NEW | MERGE_FREE_NEW; } break; case GenericEvent: @@ -362,7 +365,8 @@ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) } break; } - return MERGE_HANDLE; + + return action; } @@ -408,7 +412,7 @@ static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,X XEvent event, prev_event; int count = 0; BOOL queued = FALSE; - enum event_merge_action action = MERGE_DISCARD; + enum event_merge_action action; prev_event.type = 0; while (XCheckIfEvent( display, &event, filter, (char *)arg )) @@ -445,23 +449,12 @@ static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,X continue; /* filtered, ignore it */ } get_event_data( &event ); - if (prev_event.type) action = merge_events( &prev_event, &event ); - switch( action ) - { - case MERGE_HANDLE: /* handle prev, keep new */ - queued |= call_event_handler( display, &prev_event ); - /* fall through */ - 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; - } + action = merge_events( &prev_event, &event ); + if (action & MERGE_HANDLE_OLD) queued |= call_event_handler( display, &prev_event ); + if (action & MERGE_FREE_OLD) free_event_data( &prev_event ); + if (action & MERGE_HANDLE_NEW) queued |= call_event_handler( display, &event ); + if (action & MERGE_FREE_NEW) free_event_data( &event ); + else if (action & MERGE_FREE_OLD) prev_event = event; } if (prev_event.type) queued |= call_event_handler( display, &prev_event ); free_event_data( &prev_event ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
From: Rémi Bernon <rbernon(a)codeweavers.com> XFilterEvent generates new ClientMessage events, used for XIM callbacks, as well as passes some KeyPress events through meant for XmbLookupString. Delaying the KeyPress events processing breaks the sequence on the next XFilterEvent call. Based on a patch from Byeong-Sik Jeon <bsjeon(a)hanmail.net>. --- dlls/winex11.drv/event.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 37f146d3686..8d7bec84bd5 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -366,6 +366,13 @@ static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) break; } + switch (next->type) + { + case KeyPress: + case KeyRelease: + return action | MERGE_HANDLE_NEW | MERGE_FREE_NEW; + } + return action; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2698
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=132150 Your paranoid android. === debian11 (32 bit report) === user32: msg: Timeout win.c:9304: Test succeeded inside todo block: Expected (0,0)-(1025,737), got (0,0)-(1025,737). win: Timeout
I've messed this up, sorry. I'll have a look. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2698#note_30997
participants (2)
-
Marvin -
Rémi Bernon