[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