[PATCH libinput] touchpad: be smarter about clickfinger thumb detection
Hans de Goede
hdegoede at redhat.com
Wed Jul 8 06:01:25 PDT 2015
Hi,
On 06-07-15 06:34, Peter Hutterer wrote:
> Watching a colleague try clickfinger right-click after enabling it the first
> time showed that the vertical distance is too small. Increase it to 30mm
> instead.
>
> Increase the allowed spread between fingers to 40x30mm, but check if one of
> the fingers is in the bottom-most 20mm of the touchpad. If that's the case,
> and the touchpad is large enough to be feasable for resting a thumb on it,
> discard the finger for clickfinger count.
>
> If both fingers are in that area or one finger is in the area and they're
> really close together, the fingers count separately and are not regarded as
> thumb.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=91046
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
LGTM: Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
> ---
> src/evdev-mt-touchpad-buttons.c | 50 +++++++++++++++++++------
> test/touchpad.c | 82 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 121 insertions(+), 11 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> index f085774..5fd8a06 100644
> --- a/src/evdev-mt-touchpad-buttons.c
> +++ b/src/evdev-mt-touchpad-buttons.c
> @@ -825,6 +825,9 @@ tp_check_clickfinger_distance(struct tp_dispatch *tp,
> struct tp_touch *t2)
> {
> double x, y;
> + int within_distance = 0;
> + int xres, yres;
> + int bottom_threshold;
>
> if (!t1 || !t2)
> return 0;
> @@ -839,19 +842,44 @@ tp_check_clickfinger_distance(struct tp_dispatch *tp,
> w = tp->device->abs.dimensions.x;
> h = tp->device->abs.dimensions.y;
>
> - return (x < w * 0.3 && y < h * 0.3) ? 1 : 0;
> - } else {
> - /* maximum spread is 40mm horiz, 20mm vert. Anything wider than that
> - * is probably a gesture. The y spread is small so we ignore clicks
> - * with thumbs at the bottom of the touchpad while the pointer
> - * moving finger is still on the pad */
> -
> - x /= tp->device->abs.absinfo_x->resolution;
> - y /= tp->device->abs.absinfo_y->resolution;
> -
> - return (x < 40 && y < 20) ? 1 : 0;
> + within_distance = (x < w * 0.3 && y < h * 0.3) ? 1 : 0;
> + goto out;
> }
>
> + xres = tp->device->abs.absinfo_x->resolution;
> + yres = tp->device->abs.absinfo_y->resolution;
> + x /= xres;
> + y /= yres;
> +
> + /* maximum horiz spread is 40mm horiz, 30mm vert, anything wider
> + * than that is probably a gesture. */
> + if (x > 40 || y > 30)
> + goto out;
> +
> + within_distance = 1;
> +
> + /* if y spread is <= 20mm, they're definitely together. */
> + if (y <= 20)
> + goto out;
> +
> + /* if they're vertically spread between 20-40mm, they're not
> + * together if:
> + * - the touchpad's vertical size is >50mm, anything smaller is
> + * unlikely to have a thumb resting on it
> + * - and one of the touches is in the bottom 20mm of the touchpad
> + * and the other one isn't
> + */
> +
> + if (tp->device->abs.dimensions.y/yres < 50)
> + goto out;
> +
> + bottom_threshold = tp->device->abs.absinfo_y->maximum - 20 * yres;
> + if ((t1->point.y > bottom_threshold) !=
> + (t2->point.y > bottom_threshold))
> + within_distance = 0;
> +
> +out:
> + return within_distance;
> }
>
> static uint32_t
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 1f11cfa..6ba0694 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -263,6 +263,13 @@ START_TEST(touchpad_2fg_clickfinger_distance)
> {
> struct litest_device *dev = litest_current_device();
> struct libinput *li = dev->libinput;
> + double w, h;
> + bool small_touchpad = false;
> + unsigned int expected_button;
> +
> + if (libinput_device_get_size(dev->libinput_device, &w, &h) == 0 &&
> + h < 50.0)
> + small_touchpad = true;
>
> libinput_device_config_click_set_method(dev->libinput_device,
> LIBINPUT_CONFIG_CLICK_METHOD_CLICKFINGER);
> @@ -295,12 +302,86 @@ START_TEST(touchpad_2fg_clickfinger_distance)
> litest_touch_up(dev, 0);
> litest_touch_up(dev, 1);
>
> + /* if the touchpad is small enough, we expect all fingers to count
> + * for clickfinger */
> + if (small_touchpad)
> + expected_button = BTN_RIGHT;
> + else
> + expected_button = BTN_LEFT;
> +
> + litest_assert_button_event(li,
> + expected_button,
> + LIBINPUT_BUTTON_STATE_PRESSED);
> + litest_assert_button_event(li,
> + expected_button,
> + LIBINPUT_BUTTON_STATE_RELEASED);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_2fg_clickfinger_bottom)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> +
> + /* this test is run for the T440s touchpad only, makes getting the
> + * mm correct easier */
> +
> + libinput_device_config_click_set_method(dev->libinput_device,
> + LIBINPUT_CONFIG_CLICK_METHOD_CLICKFINGER);
> + litest_drain_events(li);
> +
> + /* one above, one below the magic line, vert spread ca 27mm */
> + litest_touch_down(dev, 0, 40, 60);
> + litest_touch_down(dev, 1, 60, 100);
> + litest_event(dev, EV_KEY, BTN_LEFT, 1);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + litest_event(dev, EV_KEY, BTN_LEFT, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + litest_touch_up(dev, 0);
> + litest_touch_up(dev, 1);
> +
> litest_assert_button_event(li,
> BTN_LEFT,
> LIBINPUT_BUTTON_STATE_PRESSED);
> litest_assert_button_event(li,
> BTN_LEFT,
> LIBINPUT_BUTTON_STATE_RELEASED);
> +
> + litest_assert_empty_queue(li);
> +
> + /* both below the magic line */
> + litest_touch_down(dev, 0, 40, 100);
> + litest_touch_down(dev, 1, 60, 95);
> + litest_event(dev, EV_KEY, BTN_LEFT, 1);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + litest_event(dev, EV_KEY, BTN_LEFT, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + litest_touch_up(dev, 0);
> + litest_touch_up(dev, 1);
> +
> + litest_assert_button_event(li,
> + BTN_RIGHT,
> + LIBINPUT_BUTTON_STATE_PRESSED);
> + litest_assert_button_event(li,
> + BTN_RIGHT,
> + LIBINPUT_BUTTON_STATE_RELEASED);
> +
> + /* one above, one below the magic line, vert spread 17mm */
> + litest_touch_down(dev, 0, 50, 75);
> + litest_touch_down(dev, 1, 55, 100);
> + litest_event(dev, EV_KEY, BTN_LEFT, 1);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + litest_event(dev, EV_KEY, BTN_LEFT, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + litest_touch_up(dev, 0);
> + litest_touch_up(dev, 1);
> +
> + litest_assert_button_event(li,
> + BTN_RIGHT,
> + LIBINPUT_BUTTON_STATE_PRESSED);
> + litest_assert_button_event(li,
> + BTN_RIGHT,
> + LIBINPUT_BUTTON_STATE_RELEASED);
> }
> END_TEST
>
> @@ -3489,6 +3570,7 @@ litest_setup_tests(void)
> litest_add("touchpad:clickfinger", touchpad_1fg_clickfinger_no_touch, LITEST_CLICKPAD, LITEST_ANY);
> litest_add("touchpad:clickfinger", touchpad_2fg_clickfinger, LITEST_CLICKPAD, LITEST_ANY);
> litest_add("touchpad:clickfinger", touchpad_2fg_clickfinger_distance, LITEST_CLICKPAD, LITEST_ANY);
> + litest_add_for_device("touchpad:clickfinger", touchpad_2fg_clickfinger_bottom, LITEST_SYNAPTICS_TOPBUTTONPAD);
> litest_add("touchpad:clickfinger", touchpad_clickfinger_to_area_method, LITEST_CLICKPAD, LITEST_ANY);
> litest_add("touchpad:clickfinger",
> touchpad_clickfinger_to_area_method_while_down, LITEST_CLICKPAD, LITEST_ANY);
>
More information about the wayland-devel
mailing list