[PATCH weston] input: send wl_keyboard.repeat_info with rate and delay info

Jason Ekstrand jason at jlekstrand.net
Tue Aug 5 12:40:22 PDT 2014


Bump.  I just pushed Jasper's simple patch before seeing Daniel's comment
about this one.  Did we ever figure out pekka's crash?  I'd like to see
this committed soon.

--Jason Ekstrand


On Fri, Jul 25, 2014 at 5:01 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Wed, 16 Jul 2014 23:05:10 +0200
> Jonny Lamb <jonny.lamb at collabora.co.uk> wrote:
>
> > The compositor reads the values out from weston.ini, the weston
> > compositor passes on the values, the weston-info client prints out the
> > values, and the values are respected in toytoolkit.
> > ---
> >  clients/weston-info.c    | 89
> +++++++++++++++++++++++++++++++++++++++++++++++-
> >  clients/window.c         | 24 +++++++++++--
> >  src/compositor-wayland.c | 18 ++++++++--
> >  src/compositor.c         |  5 +++
> >  src/compositor.h         |  3 ++
> >  src/input.c              | 12 +++++--
> >  6 files changed, 142 insertions(+), 9 deletions(-)
> >
> > diff --git a/clients/weston-info.c b/clients/weston-info.c
> > index df869e3..c53ac74 100644
> > --- a/clients/weston-info.c
> > +++ b/clients/weston-info.c
> > @@ -32,6 +32,8 @@
> >
> >  #include "../shared/os-compatibility.h"
> >
> > +#define MIN(x,y) (((x) < (y)) ? (x) : (y))
> > +
> >  typedef void (*print_info_t)(void *info);
> >  typedef void (*destroy_info_t)(void *info);
> >
> > @@ -87,9 +89,13 @@ struct shm_info {
> >  struct seat_info {
> >       struct global_info global;
> >       struct wl_seat *seat;
> > +     struct weston_info *info;
> >
> >       uint32_t capabilities;
> >       char *name;
> > +
> > +     int32_t repeat_rate;
> > +     int32_t repeat_delay;
> >  };
> >
> >  struct weston_info {
> > @@ -291,14 +297,89 @@ print_seat_info(void *data)
> >               printf(" touch");
> >
> >       printf("\n");
> > +
> > +     if (seat->repeat_rate > 0)
> > +             printf("\tkeyboard repeat rate: %d\n", seat->repeat_rate);
> > +     if (seat->repeat_delay > 0)
> > +             printf("\tkeyboard repeat delay: %d\n",
> seat->repeat_delay);
> > +}
> > +
> > +static void
> > +keyboard_handle_keymap(void *data, struct wl_keyboard *keyboard,
> > +                    uint32_t format, int fd, uint32_t size)
> > +{
> > +}
> > +
> > +static void
> > +keyboard_handle_enter(void *data, struct wl_keyboard *keyboard,
> > +                   uint32_t serial, struct wl_surface *surface,
> > +                   struct wl_array *keys)
> > +{
> > +}
> > +
> > +static void
> > +keyboard_handle_leave(void *data, struct wl_keyboard *keyboard,
> > +                   uint32_t serial, struct wl_surface *surface)
> > +{
> >  }
> >
> >  static void
> > +keyboard_handle_key(void *data, struct wl_keyboard *keyboard,
> > +                 uint32_t serial, uint32_t time, uint32_t key,
> > +                 uint32_t state)
> > +{
> > +}
> > +
> > +static void
> > +keyboard_handle_modifiers(void *data, struct wl_keyboard *keyboard,
> > +                       uint32_t serial, uint32_t mods_depressed,
> > +                       uint32_t mods_latched, uint32_t mods_locked,
> > +                       uint32_t group)
> > +{
> > +}
> > +
> > +static void
> > +keyboard_handle_repeat_info(void *data, struct wl_keyboard *keyboard,
> > +                         int32_t rate, int32_t delay)
> > +{
> > +     struct seat_info *seat = data;
> > +
> > +     seat->repeat_rate = rate;
> > +     seat->repeat_delay = delay;
> > +}
> > +
> > +static const struct wl_keyboard_listener keyboard_listener = {
> > +     keyboard_handle_keymap,
> > +     keyboard_handle_enter,
> > +     keyboard_handle_leave,
> > +     keyboard_handle_key,
> > +     keyboard_handle_modifiers,
> > +     keyboard_handle_repeat_info,
> > +};
> > +
> > +static void
> >  seat_handle_capabilities(void *data, struct wl_seat *wl_seat,
> >                        enum wl_seat_capability caps)
> >  {
> >       struct seat_info *seat = data;
> > +
> >       seat->capabilities = caps;
> > +
> > +     /* we want listen for repeat_info from wl_keyboard, but only
> > +      * do so if the seat info is >= 4 and if we actually have a
> > +      * keyboard */
> > +     if (seat->global.version < 4)
> > +             return;
> > +
> > +     if (caps & WL_SEAT_CAPABILITY_KEYBOARD) {
> > +             struct wl_keyboard *keyboard;
> > +
> > +             keyboard = wl_seat_get_keyboard(seat->seat);
> > +             wl_keyboard_add_listener(keyboard, &keyboard_listener,
> > +                                      seat);
> > +
> > +             seat->info->roundtrip_needed = true;
> > +     }
> >  }
> >
> >  static void
> > @@ -330,14 +411,20 @@ add_seat_info(struct weston_info *info, uint32_t
> id, uint32_t version)
> >  {
> >       struct seat_info *seat = xzalloc(sizeof *seat);
> >
> > +     /* required to set roundtrip_needed to true in capabilities
> > +      * handler */
> > +     seat->info = info;
> > +
> >       init_global_info(info, &seat->global, id, "wl_seat", version);
> >       seat->global.print = print_seat_info;
> >       seat->global.destroy = destroy_seat_info;
> >
> >       seat->seat = wl_registry_bind(info->registry,
> > -                                   id, &wl_seat_interface, 2);
> > +                                   id, &wl_seat_interface, MIN(version,
> 4));
> >       wl_seat_add_listener(seat->seat, &seat_listener, seat);
> >
> > +     seat->repeat_rate = seat->repeat_delay = -1;
> > +
> >       info->roundtrip_needed = true;
> >  }
> >
> > diff --git a/clients/window.c b/clients/window.c
> > index b82a93e..906dce6 100644
> > --- a/clients/window.c
> > +++ b/clients/window.c
> > @@ -334,6 +334,9 @@ struct input {
> >               xkb_mod_mask_t shift_mask;
> >       } xkb;
> >
> > +     int32_t repeat_rate;
> > +     int32_t repeat_delay;
> > +
> >       struct task repeat_task;
> >       int repeat_timer_fd;
> >       uint32_t repeat_sym;
> > @@ -2865,9 +2868,9 @@ keyboard_handle_key(void *data, struct wl_keyboard
> *keyboard,
> >               input->repeat_key = key;
> >               input->repeat_time = time;
> >               its.it_interval.tv_sec = 0;
> > -             its.it_interval.tv_nsec = 25 * 1000 * 1000;
> > +             its.it_interval.tv_nsec = 1000000000 / input->repeat_rate;
> >               its.it_value.tv_sec = 0;
> > -             its.it_value.tv_nsec = 400 * 1000 * 1000;
> > +             its.it_value.tv_nsec = input->repeat_delay * 1000 * 1000;
>
> This may get nsec > 999999999, which seems to just make the timer fail.
> The first thing I tried was repeat_delay=1000 and got no repeat at
> all. It works, if I set it to 999 instead. :-)
>
> Also repeat_rate=1 would hit a similar problem.
>
> >               timerfd_settime(input->repeat_timer_fd, 0, &its, NULL);
> >       }
> >  }
> > @@ -2899,12 +2902,24 @@ keyboard_handle_modifiers(void *data, struct
> wl_keyboard *keyboard,
> >               input->modifiers |= MOD_SHIFT_MASK;
> >  }
> >
> > +static void
> > +keyboard_handle_repeat_info(void *data, struct wl_keyboard *keyboard,
> > +                         int32_t rate, int32_t delay)
> > +{
> > +     struct input *input = data;
> > +
> > +     input->repeat_rate = rate;
> > +     input->repeat_delay = delay;
> > +}
> > +
> >  static const struct wl_keyboard_listener keyboard_listener = {
> >       keyboard_handle_keymap,
> >       keyboard_handle_enter,
> >       keyboard_handle_leave,
> >       keyboard_handle_key,
> >       keyboard_handle_modifiers,
> > +     keyboard_handle_repeat_info
> > +
> >  };
> >
> >  static void
> > @@ -4944,7 +4959,7 @@ display_add_input(struct display *d, uint32_t id)
> >       input = xzalloc(sizeof *input);
> >       input->display = d;
> >       input->seat = wl_registry_bind(d->registry, id, &wl_seat_interface,
> > -                                    MIN(d->seat_version, 3));
> > +                                    MIN(d->seat_version, 4));
> >       input->touch_focus = NULL;
> >       input->pointer_focus = NULL;
> >       input->keyboard_focus = NULL;
> > @@ -4965,6 +4980,9 @@ display_add_input(struct display *d, uint32_t id)
> >
> >       input->pointer_surface =
> wl_compositor_create_surface(d->compositor);
> >
> > +     input->repeat_rate = 40;
> > +     input->repeat_delay = 400;
> > +
> >       input->repeat_timer_fd = timerfd_create(CLOCK_MONOTONIC,
> >                                               TFD_CLOEXEC |
> TFD_NONBLOCK);
> >       input->repeat_task.run = keyboard_repeat_func;
> > diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> > index a4dcec2..c57b5dc 100644
> > --- a/src/compositor-wayland.c
> > +++ b/src/compositor-wayland.c
> > @@ -1573,12 +1573,24 @@ input_handle_modifiers(void *data, struct
> wl_keyboard *keyboard,
> >       notify_modifiers(&input->base, serial_out);
> >  }
> >
> > +static void
> > +input_handle_repeat_info(void *data, struct wl_keyboard *keyboard,
> > +                      int32_t rate, int32_t delay)
> > +{
> > +     struct wayland_input *input = data;
> > +     struct wayland_compositor *c = input->compositor;
> > +
> > +     c->base.kb_repeat_rate = rate;
> > +     c->base.kb_repeat_delay = delay;
> > +}
> > +
> >  static const struct wl_keyboard_listener keyboard_listener = {
> >       input_handle_keymap,
> >       input_handle_keyboard_enter,
> >       input_handle_keyboard_leave,
> >       input_handle_key,
> >       input_handle_modifiers,
> > +     input_handle_repeat_info,
> >  };
> >
> >  static void
> > @@ -1614,7 +1626,7 @@ static const struct wl_seat_listener seat_listener
> = {
> >  };
> >
> >  static void
> > -display_add_seat(struct wayland_compositor *c, uint32_t id)
> > +display_add_seat(struct wayland_compositor *c, uint32_t id, uint32_t
> version)
> >  {
> >       struct wayland_input *input;
> >
> > @@ -1625,7 +1637,7 @@ display_add_seat(struct wayland_compositor *c,
> uint32_t id)
> >       weston_seat_init(&input->base, &c->base, "default");
> >       input->compositor = c;
> >       input->parent.seat = wl_registry_bind(c->parent.registry, id,
> > -                                           &wl_seat_interface, 1);
> > +                                           &wl_seat_interface,
> MIN(version, 4));
>
> You are bumping the wl_seat version straight from 1 up to 4, but you
> didn't implement the additions in versions 2 and 3. This leads to the
> following crash in the Wayland backend:
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x0000000000000000 in ?? ()
> (gdb) bt
> #0  0x0000000000000000 in ?? ()
> #1  0x00007ffff79c7eec in ffi_call_unix64 () from /usr/lib64/libffi.so.6
> #2  0x00007ffff79c78bd in ffi_call () from /usr/lib64/libffi.so.6
> #3  0x00007ffff5dc435e in wl_closure_invoke (closure=<optimized out>,
>     flags=<optimized out>, target=0x6455e0, opcode=1, data=0x6451b0)
>     at src/connection.c:934
> #4  0x00007ffff5dc165f in dispatch_event (display=0x640dc0,
>     queue=<optimized out>) at src/wayland-client.c:1111
> #5  0x00007ffff5dc16e3 in dispatch_queue (display=0x640dc0, queue=0x640ec8)
>     at src/wayland-client.c:1216
> #6  0x00007ffff5dc2748 in wl_display_dispatch_queue (display=0x640dc0,
>     queue=0x640ec8) at src/wayland-client.c:1366
> #7  0x00007ffff63de94e in wayland_compositor_handle_event (fd=15, mask=1,
>     data=0x63f900) at src/compositor-wayland.c:1834
> #8  0x00007ffff7bd3280 in wl_event_loop_dispatch (loop=0x6301e0,
>     timeout=<optimized out>) at src/event-loop.c:419
> #9  0x00007ffff7bd16b5 in wl_display_run (display=0x630150)
>     at src/wayland-server.c:969
> #10 0x0000000000411ddb in main (argc=1, argv=0x7fffffffdcf8)
>     at src/compositor.c:4408
> (gdb)
>
> I didn't look further, but I suppose it is the wl_seat.name event
> causing this.
>
> >       wl_list_insert(c->input_list.prev, &input->link);
> >
> >       wl_seat_add_listener(input->parent.seat, &seat_listener, input);
> > @@ -1781,7 +1793,7 @@ registry_handle_global(void *data, struct
> wl_registry *registry, uint32_t name,
> >                       wl_registry_bind(registry, name,
> >                                        &_wl_fullscreen_shell_interface,
> 1);
> >       } else if (strcmp(interface, "wl_seat") == 0) {
> > -             display_add_seat(c, name);
> > +             display_add_seat(c, name, version);
> >       } else if (strcmp(interface, "wl_output") == 0) {
> >               wayland_compositor_register_output(c, name);
> >       } else if (strcmp(interface, "wl_shm") == 0) {
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 17fce8d..745e194 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -3712,6 +3712,11 @@ weston_compositor_init(struct weston_compositor
> *ec,
> >       if (weston_compositor_xkb_init(ec, &xkb_names) < 0)
> >               return -1;
> >
> > +     weston_config_section_get_int(s, "repeat-rate",
> > +                                   &ec->kb_repeat_rate, 40);
> > +     weston_config_section_get_int(s, "repeat-delay",
> > +                                   &ec->kb_repeat_delay, 400);
> > +
> >       text_backend_init(ec);
> >
> >       wl_data_device_manager_init(ec->wl_display);
> > diff --git a/src/compositor.h b/src/compositor.h
> > index bef5e1d..d0436d1 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -643,6 +643,9 @@ struct weston_compositor {
> >
> >       /* Raw keyboard processing (no libxkbcommon initialization or
> handling) */
> >       int use_xkbcommon;
> > +
> > +     int32_t kb_repeat_rate;
> > +     int32_t kb_repeat_delay;
> >  };
> >
> >  struct weston_buffer {
> > diff --git a/src/input.c b/src/input.c
> > index 2c799f4..ffa2626 100644
> > --- a/src/input.c
> > +++ b/src/input.c
> > @@ -1758,6 +1758,14 @@ seat_get_keyboard(struct wl_client *client,
> struct wl_resource *resource,
> >                   wl_resource_get_link(cr))
> >                       wl_data_device_set_keyboard_focus(seat);
> >       }
> > +
> > +     /* if wl_seat's version is at least 4, the wl_keyboard's
> > +      * version must be 4, so we support repeat_info */
> > +     if (wl_resource_get_version(resource) >= 4) {
> > +             wl_keyboard_send_repeat_info(cr,
> > +
>  seat->compositor->kb_repeat_rate,
> > +
>  seat->compositor->kb_repeat_delay);
> > +     }
> >  }
> >
> >  static void
> > @@ -1813,7 +1821,7 @@ bind_seat(struct wl_client *client, void *data,
> uint32_t version, uint32_t id)
> >       enum wl_seat_capability caps = 0;
> >
> >       resource = wl_resource_create(client,
> > -                                   &wl_seat_interface, MIN(version, 3),
> id);
> > +                                   &wl_seat_interface, MIN(version, 4),
> id);
> >       wl_list_insert(&seat->base_resource_list,
> wl_resource_get_link(resource));
> >       wl_resource_set_implementation(resource, &seat_interface, data,
> >                                      unbind_resource);
> > @@ -2207,7 +2215,7 @@ weston_seat_init(struct weston_seat *seat, struct
> weston_compositor *ec,
> >       wl_signal_init(&seat->destroy_signal);
> >       wl_signal_init(&seat->updated_caps_signal);
> >
> > -     seat->global = wl_global_create(ec->wl_display,
> &wl_seat_interface, 3,
> > +     seat->global = wl_global_create(ec->wl_display,
> &wl_seat_interface, 4,
> >                                       seat, bind_seat);
> >
> >       seat->compositor = ec;
>
> It might have made it a bit easier to patch compositor, wayland-backend
> and clients in separate commits, so I could have applied the compositor
> patch at least.
>
> Also, could you add updates to the weston.ini man page, please?
>
>
> Thanks,
> pq
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140805/679dfdd2/attachment-0001.html>


More information about the wayland-devel mailing list