[PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter

[Pekka Paalanen]

On Wed,  3 Feb 2016 16:14:09 +0100
Rui Matos <tiagomatos at gmail.com> wrote:

> The last cursor frame we commited before the pointer left one of our
> surfaces might not have been shown. In that case we'll have a cursor
> surface frame callback pending which we need to clear so that we can
> continue submitting new cursor frames.
> 
> Signed-off-by: Rui Matos <tiagomatos at gmail.com>
> ---
> 
> On Wed, Feb 3, 2016 at 9:30 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > Xwayland commits a wl_buffer to a cursor wl_surface with a frame
> > callback, and the frame callback may never be emitted by the
> > compositor, right?
> >
> > Is Xwayland waiting for any previous frame callback to be signalled
> > before it commits a buffer or re-sets the cursor role on the
> > wl_surface? Even if the commit and re-set is caused by wl_pointer.enter?
> >
> > Would it be a better fix to destroy any pending frame callback and
> > commit and re-set the role unconditionally on wl_pointer.enter?  
> 
> Yes, this seems like the proper fix indeed. Thanks,
> 
> Rui
> 
>  hw/xwayland/xwayland-input.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> index 61ca70b..f9e3255 100644
> --- a/hw/xwayland/xwayland-input.c
> +++ b/hw/xwayland/xwayland-input.c
> @@ -267,6 +267,16 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer,
>      for (i = 0; i < dev->button->numButtons; i++)
>          if (BitIsOn(dev->button->down, i))
>              QueuePointerEvents(dev, ButtonRelease, i, 0, &mask);
> +
> +    /* The last cursor frame we commited before the pointer left one
> +     * of our surfaces might not have been shown. In that case we'll
> +     * have a cursor surface frame callback pending which we need to
> +     * clear so that we can continue submitting new cursor frames. */
> +    if (xwl_seat->cursor_frame_cb) {
> +        wl_callback_destroy(xwl_seat->cursor_frame_cb);
> +        xwl_seat->cursor_frame_cb = NULL;
> +        xwl_seat_set_cursor(xwl_seat);
> +    }
>  }

Hi,

this idea looks fine to me, so consider this:
Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

I have to leave the actual review of xserver code for others.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20160204/03b7acc2/attachment.sig>