[PATCH libinput 05/11] touchpad: use the top-most touch for fake finger positions

[Hans de Goede]

Hi,

On 22-07-15 07:09, Peter Hutterer wrote:
> The average human hand has four fingers but only one thumb, i.e. the chance of
> a fake finger being close to the top-most touch is higher than to whatever the
> first touch was (which may be a thumb at the bottom of the touchpad).
> So search for the top-most real touch and copy its position into the fake
> touches.
>
> This also fixes another bug with the previous code - the first slot may not be
> active but we still used its position for the fake touches. Whether that was
> really triggerable is questionable though.
>
> The test is only run for the T440 touchpad - we know it's big enough to
> enable thumb detection and that way we don't have to double-check in the how
> big the touchpad is, etc.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Patches 1 - 5 look good to me:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans



> ---
>   src/evdev-mt-touchpad.c | 50 ++++++++++++++++++++++++++++++++------
>   test/touchpad.c         | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 107 insertions(+), 7 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 8629e40..b4399d2 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -722,16 +722,58 @@ tp_unhover_touches(struct tp_dispatch *tp, uint64_t time)
>
>   }
>
> +static inline void
> +tp_position_fake_touches(struct tp_dispatch *tp)
> +{
> +	struct tp_touch *t;
> +	struct tp_touch *topmost = NULL;
> +	unsigned int start, i;
> +
> +	if (tp_fake_finger_count(tp) <= tp->num_slots)
> +		return;
> +
> +	/* We have at least one fake touch down. Find the top-most real
> +	 * touch and copy its coordinates over to to all fake touches.
> +	 * This is more reliable than just taking the first touch.
> +	 */
> +	for (i = 0; i < tp->num_slots; i++) {
> +		t = tp_get_touch(tp, i);
> +		if (t->state == TOUCH_END ||
> +		    t->state == TOUCH_NONE)
> +			continue;
> +
> +		if (topmost == NULL || t->point.y < topmost->point.y)
> +			topmost = t;
> +	}
> +
> +	if (!topmost) {
> +		log_bug_libinput(tp_libinput_context(tp),
> +				 "Unable to find topmost touch\n");
> +		return;
> +	}
> +
> +	start = tp->has_mt ? tp->num_slots : 1;
> +	for (i = start; i < tp->ntouches; i++) {
> +		t = tp_get_touch(tp, i);
> +		if (t->state == TOUCH_NONE)
> +			continue;
> +
> +		t->point = topmost->point;
> +		if (!t->dirty)
> +			t->dirty = topmost->dirty;
> +	}
> +}
> +
>   static void
>   tp_process_state(struct tp_dispatch *tp, uint64_t time)
>   {
>   	struct tp_touch *t;
> -	struct tp_touch *first = tp_get_touch(tp, 0);
>   	unsigned int i;
>   	bool restart_filter = false;
>
>   	tp_process_fake_touches(tp, time);
>   	tp_unhover_touches(tp, time);
> +	tp_position_fake_touches(tp);
>
>   	for (i = 0; i < tp->ntouches; i++) {
>   		t = tp_get_touch(tp, i);
> @@ -740,12 +782,6 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
>   		if (tp->semi_mt && tp->nfingers_down != tp->old_nfingers_down)
>   			tp_motion_history_reset(t);
>
> -		if (i >= tp->num_slots && t->state != TOUCH_NONE) {
> -			t->point = first->point;
> -			if (!t->dirty)
> -				t->dirty = first->dirty;
> -		}
> -
>   		if (!t->dirty)
>   			continue;
>
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 40d9c33..7b8324c 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -795,6 +795,66 @@ START_TEST(touchpad_area_to_clickfinger_method_while_down)
>   }
>   END_TEST
>
> +START_TEST(touchpad_clickfinger_3fg_tool_position)
> +{
> +	struct litest_device *dev = litest_current_device();
> +	struct libinput *li = dev->libinput;
> +
> +	enable_clickfinger(dev);
> +	litest_drain_events(li);
> +
> +	/* one in thumb area, one in normal area. spread is wide so the two
> +	 * real fingers don't count together. we expect a 2-finger click */
> +	litest_touch_down(dev, 0, 5, 99);
> +	litest_touch_down(dev, 1, 90, 15);
> +	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);
> +	libinput_dispatch(li);
> +
> +	litest_assert_button_event(li, BTN_RIGHT,
> +				   LIBINPUT_BUTTON_STATE_PRESSED);
> +	litest_assert_button_event(li, BTN_RIGHT,
> +				   LIBINPUT_BUTTON_STATE_RELEASED);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_clickfinger_4fg_tool_position)
> +{
> +	struct litest_device *dev = litest_current_device();
> +	struct libinput *li = dev->libinput;
> +
> +	enable_clickfinger(dev);
> +	litest_drain_events(li);
> +
> +	litest_touch_down(dev, 0, 5, 99);
> +	litest_touch_down(dev, 1, 90, 15);
> +	litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0);
> +	litest_event(dev, EV_KEY, BTN_TOOL_QUADTAP, 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);
> +	libinput_dispatch(li);
> +
> +	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_btn_left)
>   {
>   	struct litest_device *dev = litest_current_device();
> @@ -4348,6 +4408,10 @@ litest_setup_tests(void)
>   	litest_add("touchpad:clickfinger", touchpad_area_to_clickfinger_method, LITEST_CLICKPAD, LITEST_ANY);
>   	litest_add("touchpad:clickfinger",
>   		   touchpad_area_to_clickfinger_method_while_down, LITEST_CLICKPAD, LITEST_ANY);
> +	/* run those two for the T440 one only so we don't have to worry
> +	 * about small touchpads messing with thumb detection expectations */
> +	litest_add_for_device("touchpad:clickfinger", touchpad_clickfinger_3fg_tool_position, LITEST_SYNAPTICS_TOPBUTTONPAD);
> +	litest_add_for_device("touchpad:clickfinger", touchpad_clickfinger_4fg_tool_position, LITEST_SYNAPTICS_TOPBUTTONPAD);
>
>   	litest_add("touchpad:click", touchpad_click_defaults_clickfinger, LITEST_APPLE_CLICKPAD, LITEST_ANY);
>   	litest_add("touchpad:click", touchpad_click_defaults_btnarea, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD);
>