[PATCH libinput] tablet: don't send tip up/button release/prox out events for unknown tools
Jason Gerecke
killertofu at gmail.com
Thu Aug 18 16:39:16 UTC 2016
On 08/17/2016 10:20 PM, Peter Hutterer wrote:
> If the kernel does not send the MSC_SERIAL information with every event, we
> end up creating a tool with a serial of 0. When the MSC_SERIAL comes in
> later for this tool libinput will think it's a new tool. On proximity out of
> that new tool we send the required release events even though we never sent
> the proximity in/button down/tip down events.
>
> Simply ditch those events and log a bug message instead. This causes the
> first tool with serial 0 to remain in proximity/down state forever but it's a
> made-up situation anyway, let's deal with that when we have devices other
> than our own test devices triggering this.
>
Hmmm. It sounds like a tablet PC which uses one of Wacom's "AES"
sensors* might be able to trigger this. It often takes several packets
for the sensor to send a non-zero pen serial number, and continues to
report a serial until the pen leaves prox. I don't know if this
could/should be fixed in the kernel since in some cases the delay from
prox to serial can be several tenths of a second -- large enough that if
the kernel drops events until a serial is seen the user may think
something is malfunctioning.
I've attached an evemu log from an AES tablet I have handy for review.
* Examples include the Dell Venue 10 Pro 5055, HP Envy 8 Note 5000,
Lenovo ThinkPad X1 Yoga, ThinkPad Yoga 260, ThinkPad Yoga 460, etc.
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....
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> src/evdev-tablet.c | 58 +++++++++++++++++++++++++++-------------
> src/libinput-private.h | 2 ++
> test/tablet.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 114 insertions(+), 18 deletions(-)
>
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 8f24555..bc85655 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -1198,6 +1198,24 @@ tablet_mark_all_axes_changed(struct tablet_dispatch *tablet,
> sizeof(tablet->changed_axes));
> }
>
> +static inline bool
> +tool_was_in_proximity(struct tablet_dispatch *tablet,
> + struct libinput_tablet_tool *tool)
> +{
> + if (!tool->in_proximity) {
> + struct libinput *libinput = tablet_libinput_context(tablet);
> + log_bug_kernel(libinput,
> + "%s: prox out for tool type %d tool_id %#x serial %#x, "
> + "but no prox in recorded\n",
> + tablet->device->devname,
> + tool->type,
> + tool->serial,
> + tool->tool_id);
> + }
> +
> + return tool->in_proximity;
> +}
> +
> static void
> tablet_update_proximity_state(struct tablet_dispatch *tablet,
> struct evdev_device *device,
> @@ -1295,6 +1313,7 @@ tablet_send_axis_proximity_tip_down_events(struct tablet_dispatch *tablet,
> LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_IN,
> tablet->changed_axes,
> &axes);
> + tool->in_proximity = true;
> tablet_unset_status(tablet, TABLET_TOOL_ENTERING_PROXIMITY);
> tablet_unset_status(tablet, TABLET_AXES_UPDATED);
> }
> @@ -1310,12 +1329,13 @@ tablet_send_axis_proximity_tip_down_events(struct tablet_dispatch *tablet,
> tablet_unset_status(tablet, TABLET_TOOL_ENTERING_CONTACT);
> tablet_set_status(tablet, TABLET_TOOL_IN_CONTACT);
> } else if (tablet_has_status(tablet, TABLET_TOOL_LEAVING_CONTACT)) {
> - tablet_notify_tip(&device->base,
> - time,
> - tool,
> - LIBINPUT_TABLET_TOOL_TIP_UP,
> - tablet->changed_axes,
> - &tablet->axes);
> + if (tool_was_in_proximity(tablet, tool))
> + tablet_notify_tip(&device->base,
> + time,
> + tool,
> + LIBINPUT_TABLET_TOOL_TIP_UP,
> + tablet->changed_axes,
> + &tablet->axes);
> tablet_unset_status(tablet, TABLET_AXES_UPDATED);
> tablet_unset_status(tablet, TABLET_TOOL_LEAVING_CONTACT);
> tablet_unset_status(tablet, TABLET_TOOL_IN_CONTACT);
> @@ -1388,11 +1408,12 @@ tablet_flush(struct tablet_dispatch *tablet,
> time);
>
> if (tablet_has_status(tablet, TABLET_BUTTONS_RELEASED)) {
> - tablet_notify_buttons(tablet,
> - device,
> - time,
> - tool,
> - LIBINPUT_BUTTON_STATE_RELEASED);
> + if (tool_was_in_proximity(tablet, tool))
> + tablet_notify_buttons(tablet,
> + device,
> + time,
> + tool,
> + LIBINPUT_BUTTON_STATE_RELEASED);
> tablet_unset_status(tablet, TABLET_BUTTONS_RELEASED);
> }
>
> @@ -1407,13 +1428,14 @@ tablet_flush(struct tablet_dispatch *tablet,
>
> if (tablet_has_status(tablet, TABLET_TOOL_LEAVING_PROXIMITY)) {
> memset(tablet->changed_axes, 0, sizeof(tablet->changed_axes));
> - tablet_notify_proximity(&device->base,
> - time,
> - tool,
> - LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_OUT,
> - tablet->changed_axes,
> - &tablet->axes);
> -
> + if (tool_was_in_proximity(tablet, tool))
> + tablet_notify_proximity(&device->base,
> + time,
> + tool,
> + LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_OUT,
> + tablet->changed_axes,
> + &tablet->axes);
> + tool->in_proximity = false;
> tablet_set_status(tablet, TABLET_TOOL_OUT_OF_PROXIMITY);
> tablet_unset_status(tablet, TABLET_TOOL_LEAVING_PROXIMITY);
>
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index 479da75..a59bc30 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -338,6 +338,8 @@ struct libinput_tablet_tool {
> struct threshold pressure_threshold;
> int pressure_offset; /* in device coordinates */
> bool has_pressure_offset;
> +
> + bool in_proximity;
> };
>
> struct libinput_tablet_pad_mode_group {
> diff --git a/test/tablet.c b/test/tablet.c
> index c6886b0..3802852 100644
> --- a/test/tablet.c
> +++ b/test/tablet.c
> @@ -2272,6 +2272,77 @@ START_TEST(tool_in_prox_before_start)
> }
> END_TEST
>
> +START_TEST(tool_in_prox_before_start_no_serial)
> +{
> + struct libinput *li;
> + struct litest_device *dev = litest_current_device();
> + struct libinput_event *event;
> + struct libinput_event_tablet_tool *tev;
> + struct libinput_tablet_tool *tool;
> + struct axis_replacement axes[] = {
> + { ABS_DISTANCE, 10 },
> + { ABS_PRESSURE, 0 },
> + { ABS_TILT_X, 0 },
> + { ABS_TILT_Y, 0 },
> + { -1, -1 }
> + };
> + const char *devnode;
> + unsigned int serial;
> +
> + /* Move into proximity before we have a context */
> + litest_tablet_proximity_in(dev, 10, 10, axes);
> +
> + /* for simplicity, we create a new litest context */
> + devnode = libevdev_uinput_get_devnode(dev->uinput);
> + li = litest_create_context();
> + libinput_path_add_device(li, devnode);
> +
> + litest_wait_for_event_of_type(li,
> + LIBINPUT_EVENT_DEVICE_ADDED,
> + -1);
> + event = libinput_get_event(li);
> + libinput_event_destroy(event);
> +
> + litest_assert_empty_queue(li);
> +
> + /* send an event without MSC_SERIAL so we won't know the serial
> + number. This should never happen on a real device unless the
> + kernel is buggy */
> + litest_event(dev, EV_KEY, BTN_STYLUS, 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);
> + libinput_event_destroy(event);
> +
> + event = libinput_get_event(li);
> + tev = litest_is_tablet_event(event,
> + LIBINPUT_EVENT_TABLET_TOOL_BUTTON);
> + tool = libinput_event_tablet_tool_get_tool(tev);
> + ck_assert_int_eq(serial,
> + libinput_tablet_tool_get_serial(tool));
> + libinput_event_destroy(event);
> +
> + litest_tablet_proximity_out(dev);
> +
> + litest_disable_log_handler(li);
> + libinput_dispatch(li);
> + litest_restore_log_handler(li);
> +
> + /* The proximity out added the MSC_SERIAL, but the original tool
> + * (without the serial) never left proximity so we don't expect
> + * button release events or a proximity out here
> + */
> + litest_assert_empty_queue(li);
> +
> + libinput_unref(li);
> +}
> +END_TEST
> +
> START_TEST(mouse_tool)
> {
> struct litest_device *dev = litest_current_device();
> @@ -3676,6 +3747,7 @@ litest_setup_tests_tablet(void)
> litest_add("tablet:tool", tool_ref, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> litest_add_no_device("tablet:tool", tool_capabilities);
> litest_add("tablet:tool", tool_in_prox_before_start, LITEST_TABLET, LITEST_ANY);
> + litest_add("tablet:tool", tool_in_prox_before_start_no_serial, LITEST_TABLET|LITEST_TOOL_SERIAL, LITEST_ANY);
> litest_add("tablet:tool_serial", tool_unique, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
> 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);
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.evemu.log
Type: text/x-log
Size: 32526 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160818/528bf567/attachment-0001.bin>
More information about the wayland-devel
mailing list