[PATCH libinput 3/3] touchpad: detect and warn about kernel tracking pointer jumps

Benjamin Tissoires benjamin.tissoires at gmail.com
Wed Apr 27 12:13:16 UTC 2016


On Wed, Apr 27, 2016 at 12:36 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> If a touch moves by more than 20mm within a single frame, reset the motion
> history, effectively discarding the movement. This is a relatively common bug
> and almost always needs a kernel fix, so add an explanatory page to the docs.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---

The patch looks good to me:

Reviewed-by: Benjamin Tissoires <benjamin.tissoires at gmail.com>

(this applies for 2/3 and 1/3 if you expand the commit message of 1/3).

Cheers,
Benjamin

>  doc/Makefile.am                  |  1 +
>  doc/page-hierarchy.dox           |  1 +
>  doc/touchpad-jumping-cursors.dox | 54 ++++++++++++++++++++++++++++++++++++++++
>  src/evdev-mt-touchpad.c          | 27 ++++++++++++++++++++
>  test/touchpad.c                  | 39 +++++++++++++++++++++++++++++
>  5 files changed, 122 insertions(+)
>  create mode 100644 doc/touchpad-jumping-cursors.dox
>
> diff --git a/doc/Makefile.am b/doc/Makefile.am
> index 7a7c6cf..f56ed6a 100644
> --- a/doc/Makefile.am
> +++ b/doc/Makefile.am
> @@ -27,6 +27,7 @@ header_files = \
>         $(srcdir)/tapping.dox \
>         $(srcdir)/test-suite.dox \
>         $(srcdir)/tools.dox \
> +       $(srcdir)/touchpad-jumping-cursors.dox \
>         $(srcdir)/touchpads.dox
>
>  diagram_files = \
> diff --git a/doc/page-hierarchy.dox b/doc/page-hierarchy.dox
> index 1351a5e..e47e98e 100644
> --- a/doc/page-hierarchy.dox
> +++ b/doc/page-hierarchy.dox
> @@ -7,6 +7,7 @@
>  - @subpage gestures
>  - @subpage palm_detection
>  - @subpage t440_support
> +- @subpage touchpad_jumping_cursor
>
>  @page touchscreens Touchscreens
>
> diff --git a/doc/touchpad-jumping-cursors.dox b/doc/touchpad-jumping-cursors.dox
> new file mode 100644
> index 0000000..e581eb1
> --- /dev/null
> +++ b/doc/touchpad-jumping-cursors.dox
> @@ -0,0 +1,54 @@
> +/**
> + at page touchpad_jumping_cursor Touchpad jumping cursor bugs
> +
> +A common bug encountered on touchpads is a cursor jump when alternating
> +between fingers on a multi-touch-capable touchpad. For example, after moving
> +the cursor a user may use a second finger in the software button area to
> +physically click the touchpad. Upon setting the finger down, the cursor
> +exhibits a jump towards the bottom left or right, depending on the finger
> +position.
> +
> +When libinput detects a cursor jump it prints a bug warning to the log with
> +the text <b>"Touch jump detected and discarded."</b> and a link to this page.
> +
> +In most cases, this is a bug in the kernel driver and to libinput it appears
> +that the touch point moves from its previous position. The pointer jump can
> +usually be seen in the evemu-record output for the device:
> +
> +<pre>
> + E: 249.206319 0000 0000 0000    # ------------ SYN_REPORT (0) ----------
> + E: 249.218008 0003 0035 3764    # EV_ABS / ABS_MT_POSITION_X    3764
> + E: 249.218008 0003 0036 2221    # EV_ABS / ABS_MT_POSITION_Y    2221
> + E: 249.218008 0003 003a 0065    # EV_ABS / ABS_MT_PRESSURE      65
> + E: 249.218008 0003 0000 3764    # EV_ABS / ABS_X                3764
> + E: 249.218008 0003 0001 2216    # EV_ABS / ABS_Y                2216
> + E: 249.218008 0003 0018 0065    # EV_ABS / ABS_PRESSURE         65
> + E: 249.218008 0000 0000 0000    # ------------ SYN_REPORT (0) ----------
> + E: 249.230881 0003 0035 3752    # EV_ABS / ABS_MT_POSITION_X    3752
> + E: 249.230881 0003 003a 0046    # EV_ABS / ABS_MT_PRESSURE      46
> + E: 249.230881 0003 0000 3758    # EV_ABS / ABS_X                3758
> + E: 249.230881 0003 0018 0046    # EV_ABS / ABS_PRESSURE         46
> + E: 249.230881 0000 0000 0000    # ------------ SYN_REPORT (0) ----------
> + E: 249.242648 0003 0035 1640    # EV_ABS / ABS_MT_POSITION_X    1640
> + E: 249.242648 0003 0036 4681    # EV_ABS / ABS_MT_POSITION_Y    4681
> + E: 249.242648 0003 003a 0025    # EV_ABS / ABS_MT_PRESSURE      25
> + E: 249.242648 0003 0000 1640    # EV_ABS / ABS_X                1640
> + E: 249.242648 0003 0001 4681    # EV_ABS / ABS_Y                4681
> + E: 249.242648 0003 0018 0025    # EV_ABS / ABS_PRESSURE         25
> + E: 249.242648 0000 0000 0000    # ------------ SYN_REPORT (0) ----------
> + E: 249.254568 0003 0035 1648    # EV_ABS / ABS_MT_POSITION_X    1648
> + E: 249.254568 0003 003a 0027    # EV_ABS / ABS_MT_PRESSURE      27
> + E: 249.254568 0003 0000 1644    # EV_ABS / ABS_X                1644
> + E: 249.254568 0003 0018 0027    # EV_ABS / ABS_PRESSURE         27
> +</pre>
> +
> +In this recording, the pointer jumps from its position 3752/2216 to
> +1640/4681 within a single frame. On this particular touchpad, this would
> +represent a physical move of almost 50mm. libinput detects some of these
> +jumps and discards the movement but otherwise continues as usual. However,
> +the bug should be fixed at the kernel level.
> +
> +When you encounter the warning in the log, please generate an evemu
> +recording of your touchpad and file a bug. See @ref reporting_bugs for more
> +details.
> +*/
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 1974e2a..4c8c3a3 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -906,6 +906,25 @@ tp_need_motion_history_reset(struct tp_dispatch *tp)
>         return rc;
>  }
>
> +static bool
> +tp_detect_jumps(const struct tp_dispatch *tp, struct tp_touch *t)
> +{
> +       struct device_coords *last;
> +       double dx, dy;
> +       const int JUMP_THRESHOLD_MM = 20;
> +
> +       if (t->history.count == 0)
> +               return false;
> +
> +       /* called before tp_motion_history_push, so offset 0 is the most
> +        * recent coordinate */
> +       last = tp_motion_history_offset(t, 0);
> +       dx = fabs(t->point.x - last->x) / tp->device->abs.absinfo_x->resolution;
> +       dy = fabs(t->point.y - last->y) / tp->device->abs.absinfo_y->resolution;
> +
> +       return hypot(dx, dy) > JUMP_THRESHOLD_MM;
> +}
> +
>  static void
>  tp_process_state(struct tp_dispatch *tp, uint64_t time)
>  {
> @@ -937,6 +956,14 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
>                 if (t->pressure_delta < -7)
>                         tp_motion_history_reset(t);
>
> +               if (tp_detect_jumps(tp, t)) {
> +                       log_bug_kernel(tp_libinput_context(tp),
> +                                      "Touch jump detected and discarded.\n"
> +                                      "See %stouchpad_jumping_cursor for details\n",
> +                                      HTTP_DOC_LINK);
> +                       tp_motion_history_reset(t);
> +               }
> +
>                 tp_thumb_detect(tp, t, time);
>                 tp_palm_detect(tp, t, time);
>
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 6f483ee..2dbb346 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -4024,6 +4024,43 @@ START_TEST(touchpad_time_usec)
>  }
>  END_TEST
>
> +START_TEST(touchpad_jump_finger_motion)
> +{
> +       struct litest_device *dev = litest_current_device();
> +       struct libinput *li = dev->libinput;
> +       struct libinput_event *event;
> +       struct libinput_event_pointer *ptrev;
> +
> +       litest_touch_down(dev, 0, 20, 30);
> +       litest_touch_move_to(dev, 0, 20, 30, 90, 30, 10, 0);
> +       litest_drain_events(li);
> +
> +       litest_disable_log_handler(li);
> +       litest_touch_move_to(dev, 0, 90, 30, 20, 80, 1, 0);
> +       litest_assert_empty_queue(li);
> +       litest_restore_log_handler(li);
> +
> +       litest_touch_move_to(dev, 0, 20, 80, 21, 81, 10, 0);
> +       litest_touch_up(dev, 0);
> +
> +       /* expect lots of little events, no big jump */
> +       libinput_dispatch(li);
> +       event = libinput_get_event(li);
> +       do {
> +               double dx, dy;
> +
> +               ptrev = litest_is_motion_event(event);
> +               dx = libinput_event_pointer_get_dx(ptrev);
> +               dy = libinput_event_pointer_get_dy(ptrev);
> +               ck_assert_int_lt(abs(dx), 20);
> +               ck_assert_int_lt(abs(dy), 20);
> +
> +               libinput_event_destroy(event);
> +               event = libinput_get_event(li);
> +       } while (event != NULL);
> +}
> +END_TEST
> +
>  void
>  litest_setup_tests(void)
>  {
> @@ -4144,4 +4181,6 @@ litest_setup_tests(void)
>         litest_add_for_device("touchpad:bugs", touchpad_t450_motion_drops, LITEST_SYNAPTICS_TRACKPOINT_BUTTONS);
>
>         litest_add("touchpad:time", touchpad_time_usec, LITEST_TOUCHPAD, LITEST_ANY);
> +
> +       litest_add_for_device("touchpad:jumps", touchpad_jump_finger_motion, LITEST_SYNAPTICS_CLICKPAD);
>  }
> --
> 2.7.4
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


More information about the wayland-devel mailing list