Skip to content

Commit

Permalink
touchpad: if a finger in the button area moves by more than 5mm, rele…
Browse files Browse the repository at this point in the history
…ase it

The software button area is currently a partially-dead area. If the finger
moves into or out of the area pointer motion works. Finger motion within the
area however does not generate motion.

The main motivation for this was to avoid accidental pointer motion when a
button is pressed. This is required for stationary fingers but once you move a
significant distance, those bets are off.

So if the finger moves by more than 5mm from where it was put down, release it
and let it move the pointer.

The full impact is largely limited to horizontal movements within the button
area because:
- leaving the finger at the bottom area for 300ms without movement triggers
  the thumb identification, so it won't move anyway.
- moving the finger north is likely to go off the button area before we
  trigger this threshold.

https://gitlab.freedesktop.org/libinput/libinput/issues/86

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
  • Loading branch information
whot committed Aug 13, 2018
1 parent 1bd7976 commit 13bda5a
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 16 deletions.
69 changes: 55 additions & 14 deletions src/evdev-mt-touchpad-buttons.c
Expand Up @@ -457,6 +457,35 @@ tp_button_handle_event(struct tp_dispatch *tp,
button_state_to_str(t->button.state));
}

static inline void
tp_button_check_for_movement(struct tp_dispatch *tp, struct tp_touch *t)
{
struct device_coords delta;
struct phys_coords mm;
double vector_length;

switch (t->button.state) {
case BUTTON_STATE_NONE:
case BUTTON_STATE_AREA:
case BUTTON_STATE_TOP:
case BUTTON_STATE_TOP_NEW:
case BUTTON_STATE_TOP_TO_IGNORE:
case BUTTON_STATE_IGNORE:
/* No point calculating if we're not going to use it */
return;
case BUTTON_STATE_BOTTOM:
break;
}

delta.x = t->point.x - t->button.initial.x;
delta.y = t->point.y - t->button.initial.y;
mm = evdev_device_unit_delta_to_mm(tp->device, &delta);
vector_length = hypot(mm.x, mm.y);

if (vector_length > 5.0 /* mm */)
t->button.has_moved = true;
}

void
tp_button_handle_state(struct tp_dispatch *tp, uint64_t time)
{
Expand All @@ -466,25 +495,37 @@ tp_button_handle_state(struct tp_dispatch *tp, uint64_t time)
if (t->state == TOUCH_NONE || t->state == TOUCH_HOVERING)
continue;

if (t->state == TOUCH_BEGIN) {
t->button.initial = t->point;
t->button.has_moved = false;
}

if (t->state == TOUCH_END) {
tp_button_handle_event(tp, t, BUTTON_EVENT_UP, time);
} else if (t->dirty) {
enum button_event event;

if (is_inside_bottom_right_area(tp, t))
event = BUTTON_EVENT_IN_BOTTOM_R;
else if (is_inside_bottom_middle_area(tp, t))
event = BUTTON_EVENT_IN_BOTTOM_M;
else if (is_inside_bottom_left_area(tp, t))
event = BUTTON_EVENT_IN_BOTTOM_L;
else if (is_inside_top_right_area(tp, t))
event = BUTTON_EVENT_IN_TOP_R;
else if (is_inside_top_middle_area(tp, t))
event = BUTTON_EVENT_IN_TOP_M;
else if (is_inside_top_left_area(tp, t))
event = BUTTON_EVENT_IN_TOP_L;
else
if (is_inside_bottom_button_area(tp, t)) {
if (is_inside_bottom_right_area(tp, t))
event = BUTTON_EVENT_IN_BOTTOM_R;
else if (is_inside_bottom_middle_area(tp, t))
event = BUTTON_EVENT_IN_BOTTOM_M;
else if (is_inside_bottom_left_area(tp, t))
event = BUTTON_EVENT_IN_BOTTOM_L;

/* In the bottom area we check for movement
* within the area. Top area - meh */
tp_button_check_for_movement(tp, t);
} else if (is_inside_top_button_area(tp, t)) {
if (is_inside_top_right_area(tp, t))
event = BUTTON_EVENT_IN_TOP_R;
else if (is_inside_top_middle_area(tp, t))
event = BUTTON_EVENT_IN_TOP_M;
else if (is_inside_top_left_area(tp, t))
event = BUTTON_EVENT_IN_TOP_L;
} else {
event = BUTTON_EVENT_IN_AREA;
}

tp_button_handle_event(tp, t, event, time);
}
Expand Down Expand Up @@ -1211,7 +1252,7 @@ bool
tp_button_touch_active(const struct tp_dispatch *tp,
const struct tp_touch *t)
{
return t->button.state == BUTTON_STATE_AREA;
return t->button.state == BUTTON_STATE_AREA || t->button.has_moved;
}

bool
Expand Down
2 changes: 2 additions & 0 deletions src/evdev-mt-touchpad.h
Expand Up @@ -196,6 +196,8 @@ struct tp_touch {
/* We use button_event here so we can use == on events */
enum button_event current;
struct libinput_timer timer;
struct device_coords initial;
bool has_moved; /* has moved more than threshold */
} button;

struct {
Expand Down
4 changes: 4 additions & 0 deletions test/test-touchpad-buttons.c
Expand Up @@ -1422,6 +1422,8 @@ START_TEST(clickpad_softbutton_left_to_right)

litest_touch_down(dev, 0, 20, 90);
litest_touch_move_to(dev, 0, 20, 90, 90, 90, 10, 0);
litest_drain_events(li);

litest_event(dev, EV_KEY, BTN_LEFT, 1);
litest_event(dev, EV_SYN, SYN_REPORT, 0);

Expand Down Expand Up @@ -1456,6 +1458,8 @@ START_TEST(clickpad_softbutton_right_to_left)

litest_touch_down(dev, 0, 90, 90);
litest_touch_move_to(dev, 0, 90, 90, 20, 90, 10, 0);
litest_drain_events(li);

litest_event(dev, EV_KEY, BTN_LEFT, 1);
litest_event(dev, EV_SYN, SYN_REPORT, 0);

Expand Down
10 changes: 8 additions & 2 deletions test/test-touchpad.c
Expand Up @@ -1008,9 +1008,15 @@ START_TEST(touchpad_edge_scroll_buttonareas_click_stops_scroll)

libinput_event_destroy(event);

/* within button areas -> no movement */
/* move within button areas but we cancelled the scroll so now we
* get pointer motion events when moving.
*
* This is not ideal behavior, but the use-case of horizontal
* edge scrolling, click, then scrolling without lifting the finger
* is so small we'll let it pass.
*/
litest_touch_move_to(dev, 0, 70, 95, 90, 95, 10, 0);
litest_assert_empty_queue(li);
litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);

litest_button_click(dev, BTN_LEFT, false);

Expand Down

0 comments on commit 13bda5a

Please sign in to comment.