[PATCH libinput 1/2] tablet: Use separate tool objects for tools without serials
Peter Hutterer
peter.hutterer at who-t.net
Wed Aug 6 22:35:21 PDT 2014
this patch really needs a extensive commit message explaining why we keep
those separate. we've discussed that on IRC, but for everyone else it needs
to be in the commit msg.
On Thu, Aug 07, 2014 at 12:09:22AM -0400, Stephen Chandler Paul wrote:
> Signed-off-by: Stephen Chandler Paul <thatslyude at gmail.com>
> ---
> src/evdev-tablet.c | 32 ++++++++++++++++------
> src/evdev-tablet.h | 4 +++
> test/tablet.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 107 insertions(+), 8 deletions(-)
>
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 31dd8d7..6ae6844 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -259,17 +259,32 @@ tablet_process_misc(struct tablet_dispatch *tablet,
> }
>
> static struct libinput_tool *
> -tablet_get_tool(struct libinput *li,
> +tablet_get_tool(struct tablet_dispatch *tablet,
> enum libinput_tool_type type,
> uint32_t serial)
> {
> struct libinput_tool *tool = NULL, *t;
> + struct list *tool_list;
>
> - /* Check if we already have the tool in our list of tools */
> - list_for_each(t, &li->tool_list, link) {
> - if (type == t->type && serial == t->serial) {
> - tool = t;
> - break;
> + if (serial) {
> + tool_list = &tablet->device->base.seat->libinput->tool_list;
> +
> + /* Check if we already have the tool in our list of tools */
> + list_for_each(t, tool_list, link) {
> + if (type == t->type && serial == t->serial) {
> + tool = t;
> + break;
> + }
> + }
> + } else {
> + tool_list = &tablet->tool_list;
> +
> + /* Same as above, but don't bother checking the serial number */
> + list_for_each(t, tool_list, link) {
> + if (type == t->type) {
> + tool = t;
> + break;
> + }
here too, an explanatory comment should help making the code more obvious to
the casual reader.
> }
> }
>
> @@ -283,7 +298,7 @@ tablet_get_tool(struct libinput *li,
> .refcount = 1,
> };
>
> - list_insert(&li->tool_list, &tool->link);
> + list_insert(tool_list, &tool->link);
> }
>
> return tool;
> @@ -382,7 +397,7 @@ tablet_flush(struct tablet_dispatch *tablet,
> uint32_t time)
> {
> struct libinput_tool *tool =
> - tablet_get_tool(device->base.seat->libinput,
> + tablet_get_tool(tablet,
> tablet->current_tool_type,
> tablet->current_tool_serial);
>
> @@ -490,6 +505,7 @@ tablet_init(struct tablet_dispatch *tablet,
> tablet->device = device;
> tablet->status = TABLET_NONE;
> tablet->current_tool_type = LIBINPUT_TOOL_NONE;
> + list_init(&tablet->tool_list);
>
> tablet_mark_all_axes_changed(tablet, device);
>
> diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
> index 1b53d20..d16aef3 100644
> --- a/src/evdev-tablet.h
> +++ b/src/evdev-tablet.h
> @@ -26,6 +26,7 @@
> #define EVDEV_TABLET_H
>
> #include "evdev.h"
> +#include <stdbool.h>
this include looks a bit lost here...
> enum tablet_status {
> TABLET_NONE = 0,
> @@ -49,6 +50,9 @@ struct tablet_dispatch {
> unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
> double axes[LIBINPUT_TABLET_AXIS_CNT];
>
> + /* Only used for tablets that don't report serial numbers */
> + struct list tool_list;
> +
> struct button_state button_state;
> struct button_state prev_button_state;
>
> diff --git a/test/tablet.c b/test/tablet.c
> index 0cd6357..60a4240 100644
> --- a/test/tablet.c
> +++ b/test/tablet.c
> @@ -681,6 +681,83 @@ START_TEST(pad_buttons_ignored)
> }
> END_TEST
>
> +START_TEST(tools_with_serials)
> +{
> + struct libinput *li = litest_create_context();
> + struct litest_device *dev[2];
> + struct libinput_tool *tool[2];
> + struct libinput_event *event;
> + int i;
> +
> + for (i = 0; i < 2; i++) {
> + dev[i] = litest_add_device_with_overrides(li,
> + LITEST_WACOM_INTUOS,
> + NULL,
> + NULL,
> + NULL,
> + NULL);
> +
> + litest_event(dev[i], EV_KEY, BTN_TOOL_PEN, 1);
> + litest_event(dev[i], EV_MSC, MSC_SERIAL, 100);
> + litest_event(dev[i], EV_SYN, SYN_REPORT, 0);
> +
> + libinput_dispatch(li);
> + while ((event = libinput_get_event(li))) {
> + if (libinput_event_get_type(event) ==
> + LIBINPUT_EVENT_TABLET_PROXIMITY_IN) {
> + struct libinput_event_tablet *t =
> + libinput_event_get_tablet_event(event);
> +
> + tool[i] = libinput_event_tablet_get_tool(t);
> + }
> +
> + libinput_event_destroy(event);
> + }
> + }
> +
> + /* We should get the same object for both devices */
> + ck_assert_ptr_eq(tool[0], tool[1]);
did you run make check on this? you're leaving the devices and the context
hanging, valgrind should've picked this up.
same for the second test, the rest of the patch looks good though.
Cheers,
Peter
> +}
> +END_TEST
> +
> +START_TEST(tools_without_serials)
> +{
> + struct libinput *li = litest_create_context();
> + struct litest_device *dev[2];
> + struct libinput_tool *tool[2];
> + struct libinput_event *event;
> + int i;
> +
> + for (i = 0; i < 2; i++) {
> + dev[i] = litest_add_device_with_overrides(li,
> + LITEST_WACOM_ISDV4,
> + NULL,
> + NULL,
> + NULL,
> + NULL);
> +
> + litest_event(dev[i], EV_KEY, BTN_TOOL_PEN, 1);
> + litest_event(dev[i], EV_SYN, SYN_REPORT, 0);
> +
> + libinput_dispatch(li);
> + while ((event = libinput_get_event(li))) {
> + if (libinput_event_get_type(event) ==
> + LIBINPUT_EVENT_TABLET_PROXIMITY_IN) {
> + struct libinput_event_tablet *t =
> + libinput_event_get_tablet_event(event);
> +
> + tool[i] = libinput_event_tablet_get_tool(t);
> + }
> +
> + libinput_event_destroy(event);
> + }
> + }
> +
> + /* We should get different tool objects for each device */
> + ck_assert_ptr_ne(tool[0], tool[1]);
> +}
> +END_TEST
> +
> int
> main(int argc, char **argv)
> {
> @@ -688,6 +765,8 @@ main(int argc, char **argv)
> litest_add("tablet:tool_serial", tool_serial, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> litest_add("tablet:tool_serial", serial_changes_tool, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> litest_add("tablet:tool_serial", invalid_serials, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> + litest_add_no_device("tablet:tool_serial", tools_with_serials);
> + litest_add_no_device("tablet:tool_serial", tools_without_serials);
> litest_add("tablet:proximity", proximity_out_clear_buttons, LITEST_TABLET, LITEST_ANY);
> litest_add("tablet:proximity", proximity_in_out, LITEST_TABLET, LITEST_ANY);
> litest_add("tablet:proximity", bad_distance_events, LITEST_TABLET | LITEST_DISTANCE, LITEST_ANY);
> --
> 1.8.5.5
>
More information about the wayland-devel
mailing list