[PATCH xserver 2/4 v5] xwayland: add a server sync before repeating keys

[Peter Hutterer]

On Tue, May 24, 2016 at 09:51:42AM +0200, Olivier Fourdan wrote:
> Key repeat is handled by the X server, but input events need to be
> processed and forwarded by the Wayland compositor first.
> 
> Make sure the Wayland compositor is actually processing events, to
> avoid repeating keys in Xwayland while the Wayland compositor cannot
> deal with input events for whatever reason, thus not dispatching key
> release events, leading to repeated keys while the user has already
> released the key.
> 
> Bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=762618
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  v2: A slightly less hack-y-ish version of the patch
>      Use xwl_seat to store the pending sync, set up callback fpr keyboard
>      in seat_handle_capabilities() and add a call to _dispatch_pending()
>      in the callback function to make sure we didn't miss a reply
>  v3: Small clean-up,
>      Remove the call to wl_display_dispatch_pending() that we added
>      in v2 as we shall do it in two other separate patches to follow.
>  v4: Use a list to store the devices pending a compositor sync per
>      xwl_seat, add the check repeat handler to xwl_seat->keyboard as
>      well, as per Daniel's last review.
>  v5: Remove device from pending sync on keyboard removal as well, to be
>      on the safe side.
> 
>  hw/xwayland/xwayland-input.c | 74 +++++++++++++++++++++++++++++++++++++++++++-
>  hw/xwayland/xwayland.h       |  2 ++
>  2 files changed, 75 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> index cbc1bf2..5c979f5 100644
> --- a/hw/xwayland/xwayland-input.c
> +++ b/hw/xwayland/xwayland-input.c
> @@ -41,6 +41,11 @@
>          (miPointerPtr)dixLookupPrivate(&(dev)->devPrivates, miPointerPrivKey): \
>          (miPointerPtr)dixLookupPrivate(&(GetMaster(dev, MASTER_POINTER))->devPrivates, miPointerPrivKey))
>  
> +struct sync_pending {
> +    struct xorg_list l;
> +    DeviceIntPtr pending_dev;
> +};
> +
>  static void
>  xwl_pointer_control(DeviceIntPtr device, PtrCtrl *ctrl)
>  {
> @@ -527,6 +532,59 @@ keyboard_handle_modifiers(void *data, struct wl_keyboard *keyboard,
>  }
>  
>  static void
> +remove_sync_pending(DeviceIntPtr dev)
> +{
> +    struct xwl_seat *xwl_seat = dev->public.devicePrivate;
> +    struct sync_pending *p, *npd;
> +
> +    xorg_list_for_each_entry_safe(p, npd, &xwl_seat->sync_pending, l)
> +        if (p->pending_dev == dev) {
> +            xorg_list_del(&xwl_seat->sync_pending);
> +            free (p);
> +            return;
> +        }


style nit: please use { } for the xorg_list, it's a bit dangerous otherwise


Cheers,
   Peter


> +}
> +
> +static void
> +sync_callback(void *data, struct wl_callback *callback, uint32_t serial)
> +{
> +    DeviceIntPtr dev = (DeviceIntPtr) data;
> +
> +    remove_sync_pending(dev);
> +    wl_callback_destroy(callback);
> +}
> +
> +static const struct wl_callback_listener sync_listener = {
> +   sync_callback
> +};
> +
> +static Bool
> +keyboard_check_repeat (DeviceIntPtr dev, XkbSrvInfoPtr xkbi, unsigned key)
> +{
> +    struct xwl_seat *xwl_seat = dev->public.devicePrivate;
> +    struct xwl_screen *xwl_screen = xwl_seat->xwl_screen;
> +    struct wl_callback *callback;
> +    struct sync_pending *p;
> +
> +    xorg_list_for_each_entry(p, &xwl_seat->sync_pending, l)
> +        if (p->pending_dev == dev) {
> +            ErrorF("Key repeat discarded, Wayland compositor doesn't "
> +                   "seem to be processing events fast enough!\n");
> +
> +            return FALSE;
> +        }
> +
> +    p = xnfalloc(sizeof(struct sync_pending));
> +    p->pending_dev = dev;
> +    callback = wl_display_sync (xwl_screen->display);
> +    xorg_list_add(&p->l, &xwl_seat->sync_pending);
> +
> +    wl_callback_add_listener(callback, &sync_listener, dev);
> +
> +    return TRUE;
> +}
> +
> +static void
>  keyboard_handle_repeat_info (void *data, struct wl_keyboard *keyboard,
>                               int32_t rate, int32_t delay)
>  {
> @@ -716,6 +774,7 @@ seat_handle_capabilities(void *data, struct wl_seat *seat,
>                           enum wl_seat_capability caps)
>  {
>      struct xwl_seat *xwl_seat = data;
> +    DeviceIntPtr master;
>  
>      if (caps & WL_SEAT_CAPABILITY_POINTER && xwl_seat->wl_pointer == NULL) {
>          xwl_seat->wl_pointer = wl_seat_get_pointer(seat);
> @@ -748,12 +807,18 @@ seat_handle_capabilities(void *data, struct wl_seat *seat,
>              ActivateDevice(xwl_seat->keyboard, TRUE);
>          }
>          EnableDevice(xwl_seat->keyboard, TRUE);
> +        xwl_seat->keyboard->key->xkbInfo->checkRepeat = keyboard_check_repeat;
> +        master = GetMaster(xwl_seat->keyboard, MASTER_KEYBOARD);
> +        if (master)
> +            master->key->xkbInfo->checkRepeat = keyboard_check_repeat;
>      } else if (!(caps & WL_SEAT_CAPABILITY_KEYBOARD) && xwl_seat->wl_keyboard) {
>          wl_keyboard_release(xwl_seat->wl_keyboard);
>          xwl_seat->wl_keyboard = NULL;
>  
> -        if (xwl_seat->keyboard)
> +        if (xwl_seat->keyboard) {
> +            remove_sync_pending(xwl_seat->keyboard);
>              DisableDevice(xwl_seat->keyboard, TRUE);
> +        }
>      }
>  
>      if (caps & WL_SEAT_CAPABILITY_TOUCH && xwl_seat->wl_touch == NULL) {
> @@ -814,12 +879,14 @@ create_input_device(struct xwl_screen *xwl_screen, uint32_t id, uint32_t version
>      wl_array_init(&xwl_seat->keys);
>  
>      xorg_list_init(&xwl_seat->touches);
> +    xorg_list_init(&xwl_seat->sync_pending);
>  }
>  
>  void
>  xwl_seat_destroy(struct xwl_seat *xwl_seat)
>  {
>      struct xwl_touch *xwl_touch, *next_xwl_touch;
> +    struct sync_pending *p, *npd;
>  
>      xorg_list_for_each_entry_safe(xwl_touch, next_xwl_touch,
>                                    &xwl_seat->touches, link_touch) {
> @@ -827,6 +894,11 @@ xwl_seat_destroy(struct xwl_seat *xwl_seat)
>          free(xwl_touch);
>      }
>  
> +    xorg_list_for_each_entry_safe(p, npd, &xwl_seat->sync_pending, l) {
> +        xorg_list_del(&xwl_seat->sync_pending);
> +        free (p);
> +    }
> +
>      wl_seat_destroy(xwl_seat->seat);
>      wl_surface_destroy(xwl_seat->cursor);
>      if (xwl_seat->cursor_frame_cb)
> diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
> index 67b30cb..5c053d9 100644
> --- a/hw/xwayland/xwayland.h
> +++ b/hw/xwayland/xwayland.h
> @@ -138,6 +138,8 @@ struct xwl_seat {
>      size_t keymap_size;
>      char *keymap;
>      struct wl_surface *keyboard_focus;
> +
> +    struct xorg_list sync_pending;
>  };
>  
>  struct xwl_output {
> -- 
> 2.7.4
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
>