Re: [PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter
On Fri, 2016-02-05 at 07:54 +0800, Jonas Ådahl wrote: > > On Feb 5, 2016 12:53 AM, "Rui Matos" 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 > > Reviewed-by: Daniel Stone > Reviewed-by: Jonas Ådahl To ssh://git.freedesktop.org/git/xorg/xserver b7d3929..87d5534 master -> master - ajax ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter
On Feb 5, 2016 12:53 AM, "Rui Matos" 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 > Reviewed-by: Daniel Stone Reviewed-by: Jonas Ådahl > --- > > v2: as suggested by Jonas, moved the hunk further up to stay close to > another related hack we already have and also removed the > xwl_seat_set_cursor() call since CheckMotion() will do that too. > > I think Daniel's r-b still stands anyway. > > hw/xwayland/xwayland-input.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c > index 61ca70b..d6cbf32 100644 > --- a/hw/xwayland/xwayland-input.c > +++ b/hw/xwayland/xwayland-input.c > @@ -251,6 +251,15 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer, > mipointer = MIPOINTER(master); > mipointer->pSpriteCursor = (CursorPtr) 1; > > +/* 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; > +} > + > CheckMotion(NULL, master); > > /* Ideally, X clients shouldn't see these button releases. When > -- > 2.5.0 > > ___ > xorg-de...@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter
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 Reviewed-by: Daniel Stone --- v2: as suggested by Jonas, moved the hunk further up to stay close to another related hack we already have and also removed the xwl_seat_set_cursor() call since CheckMotion() will do that too. I think Daniel's r-b still stands anyway. hw/xwayland/xwayland-input.c | 9 + 1 file changed, 9 insertions(+) diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c index 61ca70b..d6cbf32 100644 --- a/hw/xwayland/xwayland-input.c +++ b/hw/xwayland/xwayland-input.c @@ -251,6 +251,15 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer, mipointer = MIPOINTER(master); mipointer->pSpriteCursor = (CursorPtr) 1; +/* 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; +} + CheckMotion(NULL, master); /* Ideally, X clients shouldn't see these button releases. When -- 2.5.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter
On 3 February 2016 at 15:14, Rui Matos 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 Ah, that explains a lot ... thanks Rui! Reviewed-by: Daniel Stone Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter
On Wed, 3 Feb 2016 16:14:09 +0100 Rui Matos 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 > --- > > On Wed, Feb 3, 2016 at 9:30 AM, Pekka Paalanen 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 I have to leave the actual review of xserver code for others. Thanks, pq pgpobWeSCklBz.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter
On Wed, Feb 03, 2016 at 05:46:35PM -0800, Bryce Harrington wrote: > On Wed, Feb 03, 2016 at 04:14:09PM +0100, Rui Matos 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 > > --- > > > > On Wed, Feb 3, 2016 at 9:30 AM, Pekka Paalanen 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); > > +} If think it'd be better to move this to just below "mipointer->pSpriteCursor = (CursorPtr) 1;" and skip the xwl_seat_set_cursor(xwl_seat) call. The "pSpriteCursor = 1" thing is another hack to avoid skipping to set the cursor on enter, and it'd be good to keep them close. xwl_seat_set_cursor() will be called as well indirectly by the "CheckMotion()" call. > > } > > > > static void > > Reviewed-by: Bryce Harrington > > How noticeable/severe is this problem? Can this be left to 1.11? This patch is for the xserver, not weston, so no need to worry about this for the 1.10 release. Jonas > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter
On Wed, Feb 03, 2016 at 04:14:09PM +0100, Rui Matos 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 > --- > > On Wed, Feb 3, 2016 at 9:30 AM, Pekka Paalanen 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); > +} > } > > static void Reviewed-by: Bryce Harrington How noticeable/severe is this problem? Can this be left to 1.11? ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] xwayland: Clear pending cursor frame callbacks on pointer enter
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 --- On Wed, Feb 3, 2016 at 9:30 AM, Pekka Paalanen 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); +} } static void -- 2.5.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel