[PATCH v2 libinput] tablet: if a serial comes in late, discard it
Jason Gerecke
killertofu at gmail.com
Thu Sep 1 21:01:00 UTC 2016
On 08/31/2016 11:43 PM, Peter Hutterer wrote:
> If a tool starts reporting with serial 0 and later updates to a real serial,
> discard that serial and keep reporting as serial 0. We cannot really change
> the tool after proximity in as we don't know when callers query for the serial
> (well, we could know but any well-written caller will ask for the serial on
> the proximity in event, so what's the point).
>
> Thus if we do get a serial in and the matching tool, check if we have a tool
> with the serial 0 already. If so, re-use that. This means we lose correct tool
> tracking on such tablets but so far these seem to only be on devices where the
> use of multiple tools is unlikely.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=97526
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> Changes to v1:
> - change tool_list assignment to be more obvious
> - fix proximity out in the new test
>
> src/evdev-tablet.c | 27 ++++++++++++-----
> test/tablet.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 107 insertions(+), 8 deletions(-)
>
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 36ff6a5..70ac0f7 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -900,13 +900,12 @@ tablet_get_tool(struct tablet_dispatch *tablet,
> uint32_t tool_id,
> uint32_t serial)
> {
> + struct libinput *libinput = tablet_libinput_context(tablet);
> struct libinput_tablet_tool *tool = NULL, *t;
> struct list *tool_list;
>
> if (serial) {
> - struct libinput *libinput = tablet_libinput_context(tablet);
> tool_list = &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) {
> @@ -914,20 +913,32 @@ tablet_get_tool(struct tablet_dispatch *tablet,
> break;
> }
> }
> - } else {
> + }
> +
> + /* If we get a tool with a delayed serial number, we already created
> + * a 0-serial number tool for it earlier. Re-use that, even though
> + * it means we can't distinguish this tool from others.
> + * https://bugs.freedesktop.org/show_bug.cgi?id=97526
> + */
> + if (!tool) {
> + tool_list = &tablet->tool_list;
> /* We can't guarantee that tools without serial numbers are
> * unique, so we keep them local to the tablet that they come
> * into proximity of instead of storing them in the global tool
> - * list */
> - tool_list = &tablet->tool_list;
> -
> - /* Same as above, but don't bother checking the serial number */
> - list_for_each(t, tool_list, link) {
> + * list
> + * Same as above, but don't bother checking the serial number
> + */
> + list_for_each(t, &tablet->tool_list, link) {
Nit: Just use 'tool_list' instead of '&tablet->tool_list' here?
Aside from that, for this set:
Reviewed-by: Jason Gerecke <jason.gerecke at wacom.com>
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
> if (type == t->type) {
> tool = t;
> break;
> }
> }
> +
> + /* Didn't find the tool but we have a serial. Switch
> + * tool_list back so we create in the correct list */
> + if (!tool && serial)
> + tool_list = &libinput->tool_list;
> }
>
> /* If we didn't already have the new_tool in our list of tools,
> diff --git a/test/tablet.c b/test/tablet.c
> index 7bf6ac6..abb826a 100644
> --- a/test/tablet.c
> +++ b/test/tablet.c
> @@ -2153,6 +2153,93 @@ START_TEST(tools_without_serials)
> }
> END_TEST
>
> +START_TEST(tool_delayed_serial)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> + struct libinput_event *event;
> + struct libinput_event_tablet_tool *tev;
> + struct libinput_tablet_tool *tool;
> + unsigned int serial;
> +
> + litest_drain_events(li);
> +
> + litest_event(dev, EV_ABS, ABS_X, 4500);
> + litest_event(dev, EV_ABS, ABS_Y, 2000);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0);
> + litest_event(dev, EV_KEY, BTN_TOOL_PEN, 1);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + libinput_dispatch(li);
> +
> + event = libinput_get_event(li);
> + tev = litest_is_tablet_event(event,
> + LIBINPUT_EVENT_TABLET_TOOL_PROXIMITY);
> + tool = libinput_event_tablet_tool_get_tool(tev);
> + serial = libinput_tablet_tool_get_serial(tool);
> + ck_assert_int_eq(serial, 0);
> + libinput_event_destroy(event);
> +
> + for (int x = 4500; x < 8000; x += 1000) {
> + litest_event(dev, EV_ABS, ABS_X, x);
> + litest_event(dev, EV_ABS, ABS_Y, 2000);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + libinput_dispatch(li);
> + }
> + litest_drain_events(li);
> +
> + /* Now send the serial */
> + litest_event(dev, EV_ABS, ABS_X, 4500);
> + litest_event(dev, EV_ABS, ABS_Y, 2000);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 1234566);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + libinput_dispatch(li);
> +
> + event = libinput_get_event(li);
> + tev = litest_is_tablet_event(event,
> + LIBINPUT_EVENT_TABLET_TOOL_AXIS);
> + tool = libinput_event_tablet_tool_get_tool(tev);
> + serial = libinput_tablet_tool_get_serial(tool);
> + ck_assert_int_eq(serial, 0);
> + libinput_event_destroy(event);
> +
> + for (int x = 4500; x < 8000; x += 500) {
> + litest_event(dev, EV_ABS, ABS_X, x);
> + litest_event(dev, EV_ABS, ABS_Y, 2000);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 1234566);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + libinput_dispatch(li);
> + }
> +
> + event = libinput_get_event(li);
> + do {
> + tev = litest_is_tablet_event(event,
> + LIBINPUT_EVENT_TABLET_TOOL_AXIS);
> + tool = libinput_event_tablet_tool_get_tool(tev);
> + serial = libinput_tablet_tool_get_serial(tool);
> + ck_assert_int_eq(serial, 0);
> + libinput_event_destroy(event);
> + event = libinput_get_event(li);
> + } while (event != NULL);
> +
> + /* Quirk: tool out event is a serial of 0 */
> + litest_event(dev, EV_ABS, ABS_X, 4500);
> + litest_event(dev, EV_ABS, ABS_Y, 2000);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0);
> + litest_event(dev, EV_KEY, BTN_TOOL_PEN, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + libinput_dispatch(li);
> +
> + event = libinput_get_event(li);
> + tev = litest_is_tablet_event(event,
> + LIBINPUT_EVENT_TABLET_TOOL_PROXIMITY);
> + tool = libinput_event_tablet_tool_get_tool(tev);
> + serial = libinput_tablet_tool_get_serial(tool);
> + ck_assert_int_eq(serial, 0);
> + libinput_event_destroy(event);
> +}
> +END_TEST
> +
> START_TEST(tool_capabilities)
> {
> struct libinput *li = litest_create_context();
> @@ -4034,6 +4121,7 @@ litest_setup_tests_tablet(void)
> 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_for_device("tablet:tool_serial", tool_delayed_serial, LITEST_WACOM_HID4800_PEN);
> 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", proximity_in_button_down, LITEST_TABLET, LITEST_ANY);
>
More information about the wayland-devel
mailing list