[PATCH v2 1/3] winex11.drv: Make it possible to merge events across process_events calls.
We will have to wait for several process_events calls while delaying FocusIn/WM_TAKE_FOCUS events.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/winex11.drv/event.c | 73 +++++++++++++++++++++++++-------------- dlls/winex11.drv/x11drv.h | 1 + 2 files changed, 48 insertions(+), 26 deletions(-)
diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 25730192d3d..32fdf23bf35 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -404,22 +404,50 @@ static inline BOOL call_event_handler( Display *display, XEvent *event ) return ret; }
+static inline BOOL merge_and_handle_events( Display *display, XEvent *prev, XEvent *next ) +{ + enum event_merge_action action = MERGE_DISCARD; + BOOL queued = FALSE; + + if (prev->type) action = merge_events( prev, next ); + switch( action ) + { + case MERGE_HANDLE: /* handle prev, keep new */ + queued |= call_event_handler( display, prev ); + /* fall through */ + case MERGE_DISCARD: /* discard prev, keep new */ + free_event_data( prev ); + *prev = *next; + break; + case MERGE_KEEP: /* handle new, keep prev for future merging */ + if (!next->type) break; + queued |= call_event_handler( display, next ); + /* fall through */ + case MERGE_IGNORE: /* ignore new, keep prev for future merging */ + free_event_data( next ); + break; + } + + return queued; +}
/*********************************************************************** * process_events */ static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,XPointer), ULONG_PTR arg ) { - XEvent event, prev_event; + struct x11drv_thread_data *thread_data = x11drv_thread_data(); + XEvent event, *next = &event, *prev = &thread_data->prev_event; int count = 0; BOOL queued = FALSE; - enum event_merge_action action = MERGE_DISCARD;
- prev_event.type = 0; - while (XCheckIfEvent( display, &event, filter, (char *)arg )) + next->type = 0; + if (prev->type) get_event_data( prev ); + + while (XCheckIfEvent( display, next, filter, (char *)arg )) { count++; - if (XFilterEvent( &event, None )) + if (XFilterEvent( next, None )) { /* * SCIM on linux filters key events strangely. It does not filter the @@ -433,7 +461,7 @@ static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,X if (event.type == KeyRelease) { KeySym keysym = 0; - XKeyEvent *keyevent = &event.xkey; + XKeyEvent *keyevent = &next->xkey;
XLookupString(keyevent, NULL, 0, &keysym, NULL); if (!(keysym == XK_Shift_L || @@ -449,27 +477,20 @@ static BOOL process_events( Display *display, Bool (*filter)(Display*, XEvent*,X else 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; - } + get_event_data( next ); + queued |= merge_and_handle_events( display, prev, next ); } - if (prev_event.type) queued |= call_event_handler( display, &prev_event ); - free_event_data( &prev_event ); + + if (prev->type && !thread_data->current_event) + { + next->type = 0; + queued |= merge_and_handle_events( display, prev, next ); + free_event_data( prev ); + + /* retry handling prev next time */ + if (prev->type) queued |= 1; + } + XFlush( gdi_display ); if (count) TRACE( "processed %d events, returning %d\n", count, queued ); return queued; diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index d4e476facb2..65cb5ecf6f2 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -323,6 +323,7 @@ struct x11drv_valuator_data struct x11drv_thread_data { Display *display; + XEvent prev_event; /* event to be eventually merged */ XEvent *current_event; /* event currently being processed */ HWND grab_hwnd; /* window that currently grabs the mouse */ HWND last_focus; /* last window that had focus */
The FocusIn/WM_TAKE_FOCUS events are sent as soon as a window or its frame is clicked, but sometimes the WM is still controlling the window position. Waiting for the cursor grab to be released before sending WM_ACTIVATE message helps with this situation.
When using WM_TAKE_FOCUS we are not going to receive FocusIn - or the eventual NotifyGrab/NotifyUngrab focus events until we set the input focus for ourselves, so the merging is simpler but we may miss the keyboard ungrab notifications.
When not, we pass through any NotifyGrab/NotifyUngrab focus events unless we are delaying a FocusIn event. In this case, we have to merge these notifications to reproduce the keyboard grab state as we cannot process them out of order.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com ---
This is roughly the same as the previous version [1], although instead of having a dedicated loop waiting for the cursor to be released, this delays the FocusIn event handling until XGrabPointer succeeds, while processing the other events normally.
It retries to grab the cursor once for every process_events call, even if no event has been received. In this case we could maybe retry less often, or there could also be a small delay before trying to grab the cursor, which would also be nicer w.r.t. grab transitions between the WM and an eventual desktop shell, as described in [2].
Also, as discussed in the other thread [1], for environments where XGrabPointer would never return successfully - if that exists - this has the same behavior as the previous implementation and the focus event would be delayed forever. In this case, we could make this delay depend on the GrabPointer driver option - or add a dedicated one - and/or check during driver initialization whether it is possible to grab the pointer at least once.
[1] https://www.winehq.org/pipermail/wine-devel/2019-September/150953.html [2] https://www.winehq.org/pipermail/wine-devel/2019-September/150956.html
dlls/winex11.drv/event.c | 94 +++++++++++++++++++++++++++++++++++++++- dlls/winex11.drv/mouse.c | 4 +- 2 files changed, 95 insertions(+), 3 deletions(-)
diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 32fdf23bf35..79369854720 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -314,15 +314,101 @@ static enum event_merge_action merge_raw_motion_events( XIRawEvent *prev, XIRawE } #endif
+static int try_grab_pointer( Display *display ) +{ + if (clipping_cursor) + return 1; + + if (XGrabPointer( display, root_window, False, 0, GrabModeAsync, GrabModeAsync, + None, None, CurrentTime ) != GrabSuccess) + return 0; + + XUngrabPointer( display, CurrentTime ); + return 1; +} + /*********************************************************************** * merge_events * * Try to merge 2 consecutive events. */ -static enum event_merge_action merge_events( XEvent *prev, XEvent *next ) +static enum event_merge_action merge_events( Display *display, XEvent *prev, XEvent *next ) { switch (prev->type) { + case ClientMessage: + if (!use_take_focus) + break; + if (root_window != DefaultRootWindow( display )) + break; + if (prev->xclient.message_type != x11drv_atom(WM_PROTOCOLS) || (Atom)prev->xclient.data.l[0] != x11drv_atom(WM_TAKE_FOCUS)) + break; + + switch (next->type) + { + case ClientMessage: + if (next->xclient.message_type == x11drv_atom(WM_PROTOCOLS) && (Atom)next->xclient.data.l[0] == x11drv_atom(WM_TAKE_FOCUS)) + { + TRACE( "Discarding old WM_TAKE_FOCUS message for window %lx\n", prev->xany.window ); + return MERGE_DISCARD; + } + } + + if (try_grab_pointer( display )) + break; + + TRACE( "Unable to grab pointer yet, delaying WM_TAKE_FOCUS event\n" ); + return MERGE_KEEP; + + case FocusIn: + if (use_take_focus) + break; + if (root_window != DefaultRootWindow( display )) + break; + if (prev->xfocus.detail == NotifyPointer || prev->xfocus.detail == NotifyPointerRoot) + break; + if (prev->xfocus.mode == NotifyGrab || prev->xfocus.mode == NotifyUngrab) + break; + + switch (next->type) + { + case FocusIn: + case FocusOut: + if (next->xfocus.detail == NotifyPointer || next->xfocus.detail == NotifyPointerRoot) + return MERGE_KEEP; + if (prev->xany.window != next->xany.window) + break; + + /* merge grab notifications with the delayed focus event */ + if (next->xfocus.mode == NotifyGrab) + { + prev->xfocus.mode = NotifyWhileGrabbed; + return MERGE_IGNORE; + } + else if (next->xfocus.mode == NotifyUngrab) + { + prev->xfocus.mode = NotifyNormal; + return MERGE_IGNORE; + } + + if (next->type == FocusOut) + { + prev->type = 0; /* FocusIn/FocusOut sequence, discard prev as well */ + TRACE( "Discarding FocusIn/FocusOut sequence for window %lx\n", prev->xany.window ); + return MERGE_IGNORE; + } + else + { + TRACE( "Discarding old FocusIn event for window %lx\n", prev->xany.window ); + return MERGE_DISCARD; + } + } + + if (try_grab_pointer( display )) + break; + + TRACE( "Unable to grab pointer yet, delaying FocusIn event\n" ); + return MERGE_KEEP; case ConfigureNotify: switch (next->type) { @@ -409,7 +495,7 @@ static inline BOOL merge_and_handle_events( Display *display, XEvent *prev, XEve enum event_merge_action action = MERGE_DISCARD; BOOL queued = FALSE;
- if (prev->type) action = merge_events( prev, next ); + if (prev->type) action = merge_events( display, prev, next ); switch( action ) { case MERGE_HANDLE: /* handle prev, keep new */ @@ -721,6 +807,10 @@ static void handle_wm_protocols( HWND hwnd, XClientMessageEvent *event ) hwnd, IsWindowEnabled(hwnd), IsWindowVisible(hwnd), GetWindowLongW(hwnd, GWL_STYLE), GetFocus(), GetActiveWindow(), GetForegroundWindow(), last_focus );
+ /* as we delayed the WM_TAKE_FOCUS event, we missed the grab notifications, + * but as we could grab the cursor we assume the keyboard is ungrabbed as well */ + keyboard_grabbed = FALSE; + if (can_activate_window(hwnd)) { /* simulate a mouse click on the caption to find out diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index 8d1dc5e35d7..a12998a40ff 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -395,7 +395,9 @@ static BOOL grab_clipping_window( const RECT *clip ) GetModuleHandleW(0), NULL ))) return TRUE;
- if (keyboard_grabbed) + if (keyboard_grabbed || + XGrabPointer( data->display, root_window, False, 0, + GrabModeAsync, GrabModeAsync, None, None, CurrentTime ) != GrabSuccess) { WARN( "refusing to clip to %s\n", wine_dbgstr_rect(clip) ); last_clip_refused = TRUE; -- 2.23.0
This reverts commit 8d29f1c84b3da8c5b86d594bab3ac7b628dba1a9.
This is causing some issues and delay in some games (ie: Alan Wake), it should be safer now to only rely on the keyboard_grabbed flag.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/winex11.drv/mouse.c | 13 ------------- 1 file changed, 13 deletions(-)
diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index a12998a40ff..dc7eb8841d3 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -1469,21 +1469,8 @@ BOOL CDECL X11DRV_SetCursorPos( INT x, INT y ) return FALSE; }
- if (!clipping_cursor && - XGrabPointer( data->display, root_window, False, - PointerMotionMask | ButtonPressMask | ButtonReleaseMask, - GrabModeAsync, GrabModeAsync, root_window, None, CurrentTime ) != GrabSuccess) - { - WARN( "refusing to warp pointer to %u, %u without exclusive grab\n", pos.x, pos.y ); - return FALSE; - } - XWarpPointer( data->display, root_window, root_window, 0, 0, 0, 0, pos.x, pos.y ); data->warp_serial = NextRequest( data->display ); - - if (!clipping_cursor) - XUngrabPointer( data->display, CurrentTime ); - XNoOp( data->display ); XFlush( data->display ); /* avoids bad mouse lag in games that do their own mouse warping */ TRACE( "warped to %d,%d serial %lu\n", x, y, data->warp_serial );
-
Rémi Bernon