On Fri, Jul 24, 2015 at 10:41:41AM +0200, Hans de Goede wrote:
> 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.hutte...@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 <hdego...@redhat.com>
both fixed, thanks.
Cheers,
Peter
> >@@ -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",
> >
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
