[PATCH libinput] touchpad: don't check clickfinger distance for three fingers
Hans de Goede
hdegoede at redhat.com
Fri Jul 24 01:41:41 PDT 2015
Hi,
On 24-07-15 02:35, Peter Hutterer wrote:
> It's reasonable to expect a thumb (or the other hand's index finger) to click
> a button while a finger is down for movement. It's less reasonable to expect
> this when two fingers are interacting with the touchpad, or when two fingers
> click the touchpad (even on a large touchpad that's an awkward position).
>
> Simplify the clickfinger detection mechanism - if we have three touches down,
> it's always a three-finger click. Two fingers may be a right click or a index
> + thumb click.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> src/evdev-mt-touchpad-buttons.c | 23 ++++---------
> test/touchpad-buttons.c | 74 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 80 insertions(+), 17 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> index a2fb4b4..0e6426c 100644
> --- a/src/evdev-mt-touchpad-buttons.c
> +++ b/src/evdev-mt-touchpad-buttons.c
> @@ -856,11 +856,9 @@ tp_clickfinger_set_button(struct tp_dispatch *tp)
> unsigned int nfingers = tp->nfingers_down;
> struct tp_touch *t;
> struct tp_touch *first = NULL,
> - *second = NULL,
> - *third = NULL;
> - uint32_t close_touches = 0;
> + *second = NULL;
>
> - if (nfingers < 2 || nfingers > 3)
> + if (nfingers < 2 || nfingers >= 3)
> goto out;
>
> /* two or three fingers down on the touchpad. Check for distance
This comment should now be:
/* two fingers down on the touchpad. Check for distance
And you should also simplify the test above the comment to match and make it:
if (nfingers != 2)
goto out;
Which is what the code in your patch also does in a somewhat complicated manner.
With this fixed: Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
> @@ -873,10 +871,6 @@ tp_clickfinger_set_button(struct tp_dispatch *tp)
> first = t;
> else if (!second)
> second = t;
> - else if (!third) {
> - third = t;
> - break;
> - }
> }
>
> if (!first || !second) {
> @@ -884,15 +878,10 @@ tp_clickfinger_set_button(struct tp_dispatch *tp)
> goto out;
> }
>
> - close_touches |= tp_check_clickfinger_distance(tp, first, second) << 0;
> - close_touches |= tp_check_clickfinger_distance(tp, second, third) << 1;
> - close_touches |= tp_check_clickfinger_distance(tp, first, third) << 2;
> -
> - switch(__builtin_popcount(close_touches)) {
> - case 0: nfingers = 1; break;
> - case 1: nfingers = 2; break;
> - default: nfingers = 3; break;
> - }
> + if (tp_check_clickfinger_distance(tp, first, second))
> + nfingers = 2;
> + else
> + nfingers = 1;
>
> out:
> switch (nfingers) {
> diff --git a/test/touchpad-buttons.c b/test/touchpad-buttons.c
> index ccc3b88..5767114 100644
> --- a/test/touchpad-buttons.c
> +++ b/test/touchpad-buttons.c
> @@ -444,6 +444,78 @@ START_TEST(touchpad_2fg_clickfinger_distance)
> }
> END_TEST
>
> +START_TEST(touchpad_3fg_clickfinger_distance)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> +
> + if (libevdev_get_num_slots(dev->evdev) < 3)
> + return;
> +
> + enable_clickfinger(dev);
> +
> + litest_drain_events(li);
> +
> + litest_touch_down(dev, 0, 90, 90);
> + litest_touch_down(dev, 1, 10, 15);
> + litest_touch_down(dev, 2, 10, 15);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + 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_event(dev, EV_SYN, SYN_REPORT, 0);
> + litest_touch_up(dev, 0);
> + litest_touch_up(dev, 1);
> + litest_touch_up(dev, 2);
> +
> + litest_assert_button_event(li,
> + BTN_MIDDLE,
> + LIBINPUT_BUTTON_STATE_PRESSED);
> + litest_assert_button_event(li,
> + BTN_MIDDLE,
> + LIBINPUT_BUTTON_STATE_RELEASED);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_3fg_clickfinger_distance_btntool)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> +
> + if (libevdev_get_num_slots(dev->evdev) > 2)
> + return;
> +
> + enable_clickfinger(dev);
> +
> + litest_drain_events(li);
> +
> + litest_touch_down(dev, 0, 90, 90);
> + litest_touch_down(dev, 1, 10, 15);
> + libinput_dispatch(li);
> + litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0);
> + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 1);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> + libinput_dispatch(li);
> + 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_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 1);
> + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 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_MIDDLE,
> + LIBINPUT_BUTTON_STATE_PRESSED);
> + litest_assert_button_event(li,
> + BTN_MIDDLE,
> + LIBINPUT_BUTTON_STATE_RELEASED);
> +}
> +END_TEST
> +
> START_TEST(touchpad_2fg_clickfinger_bottom)
> {
> struct litest_device *dev = litest_current_device();
> @@ -1415,6 +1487,8 @@ litest_setup_tests(void)
> litest_add("touchpad:clickfinger", touchpad_4fg_clickfinger_btntool_2slots, LITEST_CLICKPAD, LITEST_ANY);
> litest_add("touchpad:clickfinger", touchpad_4fg_clickfinger_btntool_3slots, LITEST_CLICKPAD, LITEST_ANY);
> litest_add("touchpad:clickfinger", touchpad_2fg_clickfinger_distance, LITEST_CLICKPAD, LITEST_ANY);
> + litest_add("touchpad:clickfinger", touchpad_3fg_clickfinger_distance, LITEST_CLICKPAD, LITEST_ANY);
> + litest_add("touchpad:clickfinger", touchpad_3fg_clickfinger_distance_btntool, 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",
>
More information about the wayland-devel
mailing list