On Tue, 2 Feb 2016 21:06:33 +0100 Rui Matos <tiagoma...@gmail.com> wrote:
> If the wayland compositor hides our cursor surface (e.g. because the > pointer moved over a different wayland client) before our last > submitted buffer gets a chance to be displayed, no frame event will be > sent and thus we end up in a state where we'll never do any more > cursor updates since we never clear cursor_frame_cb. > > Signed-off-by: Rui Matos <tiagoma...@gmail.com> > --- > > If you have seen stuck cursors with xwayland windows lately this is > probably why. > > hw/xwayland/xwayland-cursor.c | 6 ++++-- > hw/xwayland/xwayland-input.c | 30 ++++++++++++++++++++++++++++++ > hw/xwayland/xwayland.h | 1 + > 3 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c > index 76729db..a6ee78f 100644 > --- a/hw/xwayland/xwayland-cursor.c > +++ b/hw/xwayland/xwayland-cursor.c > @@ -140,8 +140,10 @@ xwl_seat_set_cursor(struct xwl_seat *xwl_seat) > xwl_seat->x_cursor->bits->width, > xwl_seat->x_cursor->bits->height); > > - xwl_seat->cursor_frame_cb = wl_surface_frame(xwl_seat->cursor); > - wl_callback_add_listener(xwl_seat->cursor_frame_cb, &frame_listener, > xwl_seat); > + if (xwl_seat->cursor_visible) { > + xwl_seat->cursor_frame_cb = wl_surface_frame(xwl_seat->cursor); > + wl_callback_add_listener(xwl_seat->cursor_frame_cb, &frame_listener, > xwl_seat); > + } > > wl_surface_commit(xwl_seat->cursor); > } > diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c > index 61ca70b..bdf1fbc 100644 > --- a/hw/xwayland/xwayland-input.c > +++ b/hw/xwayland/xwayland-input.c > @@ -781,6 +781,34 @@ static const struct wl_seat_listener seat_listener = { > }; > > static void > +cursor_surface_handle_enter(void *data, struct wl_surface *surface, > + struct wl_output *output) > +{ > + struct xwl_seat *xwl_seat = data; > + xwl_seat->cursor_visible = TRUE; > + if (xwl_seat->cursor_needs_update) { > + xwl_seat->cursor_needs_update = FALSE; > + xwl_seat_set_cursor(xwl_seat); > + } > +} > + > +static void > +cursor_surface_handle_leave(void *data, struct wl_surface *surface, > + struct wl_output *output) > +{ > + struct xwl_seat *xwl_seat = data; > + xwl_seat->cursor_visible = FALSE; > + if (xwl_seat->cursor_frame_cb) > + wl_callback_destroy(xwl_seat->cursor_frame_cb); > + xwl_seat->cursor_frame_cb = NULL; > +} > + > +static const struct wl_surface_listener cursor_surface_listener = { > + cursor_surface_handle_enter, > + cursor_surface_handle_leave > +}; > + > +static void > create_input_device(struct xwl_screen *xwl_screen, uint32_t id, uint32_t > version) > { > struct xwl_seat *xwl_seat; > @@ -800,6 +828,8 @@ create_input_device(struct xwl_screen *xwl_screen, > uint32_t id, uint32_t version > xwl_seat->id = id; > > xwl_seat->cursor = wl_compositor_create_surface(xwl_screen->compositor); > + wl_surface_add_listener(xwl_seat->cursor, &cursor_surface_listener, > xwl_seat); > + > wl_seat_add_listener(xwl_seat->seat, &seat_listener, xwl_seat); > wl_array_init(&xwl_seat->keys); > > diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h > index a7d7119..a0af07a 100644 > --- a/hw/xwayland/xwayland.h > +++ b/hw/xwayland/xwayland.h > @@ -131,6 +131,7 @@ struct xwl_seat { > struct wl_surface *cursor; > struct wl_callback *cursor_frame_cb; > Bool cursor_needs_update; > + Bool cursor_visible; > > struct xorg_list touches; > Hi, I'm not completely convinced that using the wl_surface.enter/leave(output) events is totally correct for determining whether you will get a frame callback or not. I do not recall anything in the protocol implying or guaranteering this. Compositors are encouraged to stop issuing frame callbacks or to reduce their rate if the wl_surface is not visible in a normal way. This can also happen when the surface is mapped on an output but completely obscured by other surfaces, though I'm not sure if compositors today implement that. Stopping frame callbacks does not imply a wl_surface.leave. Of course, getting a cursor obscured like that can usually happen only by another cursor, so it's very hard to trigger, but not impossible. However, when the cursor surface becomes visible again, I'd expect all pending frame callbacks to be emitted. So why does the surface not become visible? I suppose the compositor is expecting the cursor role to be re-set on the surface, otherwise it won't get mapped again. How does the original problem of a stuck cursor happen? 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? Thanks, pq
pgp64Hy3XtYcw.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel