Re: [PATCH resend] winex11.drv: Try harder to grab pointer.

[Rémi Bernon]

On 9/4/19 2:09 PM, Zhiyi Zhang wrote:
> Some window managers temporarily grab the pointer during window transition.
> We need to wait a while for window manager to release its grab, otherwise
> XGrabPointer may fail with AlreadyGrabbed, causing cursor clipping to fail.
> Signed-off-by: Zhiyi Zhang
>
> dlls/winex11.drv/mouse.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c
> index f737a306a5..7501959369 100644
> --- a/dlls/winex11.drv/mouse.c
> +++ b/dlls/winex11.drv/mouse.c
> @@ -378,9 +378,13 @@ static BOOL grab_clipping_window( const RECT *clip )
>   {
>       static const WCHAR messageW[] = {'M','e','s','s','a','g','e',0};
>       struct x11drv_thread_data *data = x11drv_thread_data();
> -
>
> static DWORD timeout = 5000;
> -
>
> static DWORD step = 100;
> -
>
> DWORD time = 0;
>  Window clip_window;
>  HWND msg_hwnd = 0;
>  POINT pos;
> -
>
> INT ret;
>
> if (GetWindowThreadProcessId( GetDesktopWindow(), NULL ) == GetCurrentThreadId())
>      return TRUE;  /* don't clip in the desktop process */
>
> @@ -416,13 +420,24 @@ static BOOL grab_clipping_window( const RECT *clip )
>           clip->right < clip_rect.right || clip->bottom < clip_rect.bottom)
>           data->warp_serial = NextRequest( data->display );
> - if (!XGrabPointer( data->display, clip_window, False,
> -
>                   PointerMotionMask | ButtonPressMask | ButtonReleaseMask,
> -
>                   GrabModeAsync, GrabModeAsync, clip_window, None, CurrentTime ))
> -
>
> /* Some windows managers temporarily grab the pointer during window transition. Retry grabbing. */
> -
>
> do
> -
>
> {
> -
>    ret = XGrabPointer( data->display, clip_window, False, PointerMotionMask | ButtonPressMask | ButtonReleaseMask,
> -
>                        GrabModeAsync, GrabModeAsync, clip_window, None, CurrentTime );
> -
>    if (ret == AlreadyGrabbed || ret == GrabFrozen)
> -
>    {
> -
>        time += step;
> -
>        Sleep(step);
> -
>    }
> -
>
> } while ((ret == AlreadyGrabbed || ret == GrabFrozen) && time < timeout);
> -
> -
>
> if (ret == GrabSuccess)
>      clipping_cursor = TRUE;
>
> if (!clipping_cursor)
>  {
> -
>    ERR("Failed to grab pointer\n");
>     disable_xinput2();
>     DestroyWindow( msg_hwnd );
>     return FALSE;

I think this will not fix the issue in a reliable way. If the 
application fails to grab the cursor, it's usually because the WM is 
already grabbing it. The timeout makes the behavior arbitrarily differ 
when the user moves a window for more than 5s, although IMHO we should 
aim for consistent behavior - systematically fail to clip the cursor 
when the user moves the window for example, XOR retry indefinitely until 
it succeeds.

Some games already retry clipping the cursor at every frame, so in this 
case they will eventually succeed. Some others don't and expect it to 
succeed, but for these I believe we should instead delay and retry to 
clip the cursor after the WM releases its grab.