[PATCH v3 0/6] winex11.drv: Fix several cursor and focus issues
This is a resend of several patches with some addtional ones. This should address cursor and focus related issues in winex11.drv. Many games are grabbing and warping the cursor to keep it inside their window. There's apparently no way in Xlib to prevent XWarpPointer to succeed even if some other application have active grab on the pointer, so each application should behave. The first two patches should make Wine be nicer in this regard. The focus issue is covered by the four other patches. As keyboard grab notifications are send as focus events, Wine sometimes loses tracks of the current focus state. Fixing this and using these notifications helps deciding whether it is safe or not to ask for a cursor grab / warp. Rémi Bernon (6): winex11.drv: Do not set clipping_cursor when clip window is mapped winex11.drv: Only call XWarpPointer if we can get exclusive grab winex11.drv: Print the FocusIn/FocusOut mode in trace message winex11.drv: Do not react to keyboard grab focus events winex11.drv: Only grab or warp the cursor when keyboard isn't grabbed winex11.drv: Retry last ClipCursor when grab is released dlls/winex11.drv/event.c | 57 ++++++++++++++++++++++++++++++++++++--- dlls/winex11.drv/mouse.c | 46 +++++++++++++++++++++++++++++++ dlls/winex11.drv/x11drv.h | 2 ++ 3 files changed, 102 insertions(+), 3 deletions(-) -- 2.20.1
This flag should only indicate a successful call to XGrabPointer. If not then we could assume we have ownership of the pointer when we don't. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 1 - 1 file changed, 1 deletion(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 5c465aa0335..96ccbea5458 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -934,7 +934,6 @@ static BOOL X11DRV_MapNotify( HWND hwnd, XEvent *event ) if (event->xany.window == x11drv_thread_data()->clip_window) { - clipping_cursor = TRUE; return TRUE; } if (!(data = get_win_data( hwnd ))) return FALSE; -- 2.20.1
XWarpPointer will always succeed regardless of grabs. If the pointer is already grabbed by some other application, we should not ask to warp it. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/mouse.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index f737a306a56..c3a9a027957 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -1430,6 +1430,18 @@ BOOL CDECL X11DRV_SetCursorPos( INT x, INT y ) struct x11drv_thread_data *data = x11drv_init_thread_data(); POINT pos = virtual_screen_to_root( x, y ); + 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; + } + + if (!clipping_cursor) + XUngrabPointer( data->display, CurrentTime ); + XWarpPointer( data->display, root_window, root_window, 0, 0, 0, 0, pos.x, pos.y ); data->warp_serial = NextRequest( data->display ); XNoOp( data->display ); -- 2.20.1
On Thu, Jun 27, 2019 at 04:30:11PM +0200, Rémi Bernon wrote:
XWarpPointer will always succeed regardless of grabs. If the pointer is already grabbed by some other application, we should not ask to warp it.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/mouse.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index f737a306a56..c3a9a027957 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -1430,6 +1430,18 @@ BOOL CDECL X11DRV_SetCursorPos( INT x, INT y ) struct x11drv_thread_data *data = x11drv_init_thread_data(); POINT pos = virtual_screen_to_root( x, y );
+ 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; + } + + if (!clipping_cursor) + XUngrabPointer( data->display, CurrentTime );
Shouldn't we hold onto the grab until we call XWarpPointer(), in case something else grabs it in between?
XWarpPointer( data->display, root_window, root_window, 0, 0, 0, 0, pos.x, pos.y ); data->warp_serial = NextRequest( data->display ); XNoOp( data->display );
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 96ccbea5458..92d35466a35 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -749,6 +749,14 @@ static const char * const focus_details[] = "NotifyDetailNone" }; +static const char * const focus_modes[] = +{ + "NotifyNormal", + "NotifyGrab", + "NotifyUngrab", + "NotifyWhileGrabbed" +}; + /********************************************************************** * X11DRV_FocusIn */ @@ -759,7 +767,7 @@ static BOOL X11DRV_FocusIn( HWND hwnd, XEvent *xev ) if (!hwnd) return FALSE; - TRACE( "win %p xwin %lx detail=%s\n", hwnd, event->window, focus_details[event->detail] ); + TRACE( "win %p xwin %lx detail=%s mode=%s\n", hwnd, event->window, focus_details[event->detail], focus_modes[event->mode] ); if (event->detail == NotifyPointer) return FALSE; if (hwnd == GetDesktopWindow()) return FALSE; @@ -839,7 +847,7 @@ static BOOL X11DRV_FocusOut( HWND hwnd, XEvent *xev ) { XFocusChangeEvent *event = &xev->xfocus; - TRACE( "win %p xwin %lx detail=%s\n", hwnd, event->window, focus_details[event->detail] ); + TRACE( "win %p xwin %lx detail=%s mode=%s\n", hwnd, event->window, focus_details[event->detail], focus_modes[event->mode] ); if (event->detail == NotifyPointer) { -- 2.20.1
Signed-off-by: Huw Davies <huw(a)codeweavers.com>
Several window managers are sending FocusOut with NotifyGrab mode then FocusOut with NotifyWhileGrabbed mode when a window focus is lost, as a consequence of grabbing the keyboard input before changing window focus. This is the case during alt-tab, but keyboard can also be grabbed when bringing activity view or clicking on the title bar. In this cases NotifyWhileGrabbed events aren't sent until the window really loses foreground. In the same manner, when focus is restored, they usually send FocusIn with NotifyWhileGrabbed mode followed by FocusIn with NotifyUngrab mode when the keyboard grab is released. When pressing Super key, or clicking on the title bar, only NotifyGrab and NotifyUngrab mode events will be sent. In order to be consistent across WM and to help simplifying focus handling, just ignore focus events related to keyboard grabs. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 92d35466a35..83e09477ba7 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -772,6 +772,19 @@ static BOOL X11DRV_FocusIn( HWND hwnd, XEvent *xev ) if (event->detail == NotifyPointer) return FALSE; if (hwnd == GetDesktopWindow()) return FALSE; + switch (event->mode) + { + case NotifyGrab: + WARN( "unexpected FocusIn event with NotifyGrab mode\n" ); + break; + case NotifyWhileGrabbed: + break; + case NotifyNormal: + break; + case NotifyUngrab: + return FALSE; /* ignore wm specific NotifyUngrab / NotifyGrab events w.r.t focus */ + } + if ((xic = X11DRV_get_ic( hwnd ))) XSetICFocus( xic ); if (use_take_focus) { @@ -855,6 +868,20 @@ static BOOL X11DRV_FocusOut( HWND hwnd, XEvent *xev ) return TRUE; } if (!hwnd) return FALSE; + + switch (event->mode) + { + case NotifyUngrab: + WARN( "unexpected FocusOut event with NotifyUngrab mode\n" ); + break; + case NotifyNormal: + break; + case NotifyWhileGrabbed: + break; + case NotifyGrab: + return FALSE; /* ignore wm specific NotifyUngrab / NotifyGrab events w.r.t focus */ + } + focus_out( event->display, hwnd ); return TRUE; } -- 2.20.1
When some other window has a keyboard grab, it is good citizenship to wait for it to end. Grabbing or moving the cursor at the same time could interfer with it. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 9 +++++++++ dlls/winex11.drv/mouse.c | 12 ++++++++++++ dlls/winex11.drv/x11drv.h | 1 + 3 files changed, 22 insertions(+) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 83e09477ba7..21ee2aa084e 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -155,6 +155,9 @@ static const char * event_names[MAX_EVENT_HANDLERS] = "SelectionNotify", "ColormapNotify", "ClientMessage", "MappingNotify", "GenericEvent" }; +/* is someone else grabbing the keyboard, for example the WM, when manipulating the window */ +BOOL keyboard_grabbed = FALSE; + int xinput2_opcode = 0; /* return the name of an X event */ @@ -778,10 +781,13 @@ static BOOL X11DRV_FocusIn( HWND hwnd, XEvent *xev ) WARN( "unexpected FocusIn event with NotifyGrab mode\n" ); break; case NotifyWhileGrabbed: + keyboard_grabbed = TRUE; break; case NotifyNormal: + keyboard_grabbed = FALSE; break; case NotifyUngrab: + keyboard_grabbed = FALSE; return FALSE; /* ignore wm specific NotifyUngrab / NotifyGrab events w.r.t focus */ } @@ -875,10 +881,13 @@ static BOOL X11DRV_FocusOut( HWND hwnd, XEvent *xev ) WARN( "unexpected FocusOut event with NotifyUngrab mode\n" ); break; case NotifyNormal: + keyboard_grabbed = FALSE; break; case NotifyWhileGrabbed: + keyboard_grabbed = TRUE; break; case NotifyGrab: + keyboard_grabbed = TRUE; return FALSE; /* ignore wm specific NotifyUngrab / NotifyGrab events w.r.t focus */ } diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index c3a9a027957..fa976cae7fb 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -392,6 +392,12 @@ static BOOL grab_clipping_window( const RECT *clip ) GetModuleHandleW(0), NULL ))) return TRUE; + if (keyboard_grabbed) + { + WARN( "refusing to clip to %s\n", wine_dbgstr_rect(clip) ); + return FALSE; + } + /* enable XInput2 unless we are already clipping */ if (!data->clip_hwnd) enable_xinput2(); @@ -1430,6 +1436,12 @@ BOOL CDECL X11DRV_SetCursorPos( INT x, INT y ) struct x11drv_thread_data *data = x11drv_init_thread_data(); POINT pos = virtual_screen_to_root( x, y ); + if (keyboard_grabbed) + { + WARN( "refusing to warp to %u, %u\n", pos.x, pos.y ); + return FALSE; + } + if (!clipping_cursor && XGrabPointer( data->display, root_window, False, PointerMotionMask | ButtonPressMask | ButtonReleaseMask, diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 9ab948f7246..2dfa2a20ce9 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -382,6 +382,7 @@ extern Colormap default_colormap DECLSPEC_HIDDEN; extern XPixmapFormatValues **pixmap_formats DECLSPEC_HIDDEN; extern Window root_window DECLSPEC_HIDDEN; extern BOOL clipping_cursor DECLSPEC_HIDDEN; +extern BOOL keyboard_grabbed DECLSPEC_HIDDEN; extern unsigned int screen_bpp DECLSPEC_HIDDEN; extern BOOL use_xkb DECLSPEC_HIDDEN; extern BOOL usexrandr DECLSPEC_HIDDEN; -- 2.20.1
As we ignore these NotifyGrab / NotifyUngrab w.r.t focus decisions, some applications are unaware of mouse grabs being lost and sometimes cursor clipping is lost. We have to keep the last clip rectangle and restore it when grab is released. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/event.c | 8 ++++++++ dlls/winex11.drv/mouse.c | 22 ++++++++++++++++++++++ dlls/winex11.drv/x11drv.h | 1 + 3 files changed, 31 insertions(+) diff --git a/dlls/winex11.drv/event.c b/dlls/winex11.drv/event.c index 21ee2aa084e..10b61482e82 100644 --- a/dlls/winex11.drv/event.c +++ b/dlls/winex11.drv/event.c @@ -788,6 +788,7 @@ static BOOL X11DRV_FocusIn( HWND hwnd, XEvent *xev ) break; case NotifyUngrab: keyboard_grabbed = FALSE; + retry_grab_clipping_window(); return FALSE; /* ignore wm specific NotifyUngrab / NotifyGrab events w.r.t focus */ } @@ -888,6 +889,13 @@ static BOOL X11DRV_FocusOut( HWND hwnd, XEvent *xev ) break; case NotifyGrab: keyboard_grabbed = TRUE; + + /* This will do nothing due to keyboard_grabbed == TRUE, but it + * will save the current clipping rect so we can restore it on + * FocusIn with NotifyUngrab mode. + */ + retry_grab_clipping_window(); + return FALSE; /* ignore wm specific NotifyUngrab / NotifyGrab events w.r.t focus */ } diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c index fa976cae7fb..3bf6582c9db 100644 --- a/dlls/winex11.drv/mouse.c +++ b/dlls/winex11.drv/mouse.c @@ -125,6 +125,8 @@ XContext cursor_context = 0; static HWND cursor_window; static HCURSOR last_cursor; static DWORD last_cursor_change; +static RECT last_clip_rect; +static BOOL last_clip_refused; static RECT clip_rect; static Cursor create_cursor( HANDLE handle ); @@ -395,8 +397,14 @@ static BOOL grab_clipping_window( const RECT *clip ) if (keyboard_grabbed) { WARN( "refusing to clip to %s\n", wine_dbgstr_rect(clip) ); + last_clip_refused = TRUE; + last_clip_rect = *clip; return FALSE; } + else + { + last_clip_refused = FALSE; + } /* enable XInput2 unless we are already clipping */ if (!data->clip_hwnd) enable_xinput2(); @@ -470,6 +478,20 @@ void reset_clipping_window(void) ClipCursor( NULL ); /* make sure the clip rectangle is reset too */ } +/*********************************************************************** + * retry_grab_clipping_window + * + * Restore the current clip rectangle or retry the last one if it has + * been refused because of an active keyboard grab. + */ +void retry_grab_clipping_window(void) +{ + if (clipping_cursor) + ClipCursor( &clip_rect ); + else if (last_clip_refused) + ClipCursor( &last_clip_rect ); +} + BOOL CDECL X11DRV_ClipCursor( const RECT *clip ); /*********************************************************************** diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 2dfa2a20ce9..582f802be84 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -624,6 +624,7 @@ extern void sync_window_cursor( Window window ) DECLSPEC_HIDDEN; extern LRESULT clip_cursor_notify( HWND hwnd, HWND new_clip_hwnd ) DECLSPEC_HIDDEN; extern void ungrab_clipping_window(void) DECLSPEC_HIDDEN; extern void reset_clipping_window(void) DECLSPEC_HIDDEN; +extern void retry_grab_clipping_window(void) DECLSPEC_HIDDEN; extern BOOL clip_fullscreen_window( HWND hwnd, BOOL reset ) DECLSPEC_HIDDEN; extern void move_resize_window( HWND hwnd, int dir ) DECLSPEC_HIDDEN; extern void X11DRV_InitKeyboard( Display *display ) DECLSPEC_HIDDEN; -- 2.20.1
participants (2)
-
Huw Davies -
Rémi Bernon