[PATCH libinput 2/2] evdev-mt-touchpad-buttons: Add support for top softbutton area

Peter Hutterer peter.hutterer at who-t.net
Wed May 28 23:53:46 PDT 2014


On Wed, May 28, 2014 at 03:19:56PM +0200, Hans de Goede wrote:
> Add support for the top softbutton area found on some laptops.
> 
> For details of how this works, see the updated
> doc/touchpad-softbutton-state-machine.svg diagram.
> 
> Basically this mirrors the state-machine for the bottom softbutton area, with
> one exception, if a finger stays at least inner timeout milliseconds in the
> top button area and then moves out of it, it will be ignored rather then
> become the pointer. This is done so that people using the top buttons together
> with a trackstick and accidentally move their finger out of the upper area
> don't get spurious pointer movements from the finger on the trackpad.
> 
> This behavior is indentical to xf86-input-synaptics, which also ignores
> movements from touches which start in the top button area.
> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  doc/touchpad-softbutton-state-machine.svg | 602 ++++++++++++++++++------------
>  src/evdev-mt-touchpad-buttons.c           | 241 +++++++++++-
>  src/evdev-mt-touchpad.h                   |  21 +-
>  3 files changed, 615 insertions(+), 249 deletions(-)
> 
> diff --git a/doc/touchpad-softbutton-state-machine.svg b/doc/touchpad-softbutton-state-machine.svg
> index 1142343..b7028ef 100644
> --- a/doc/touchpad-softbutton-state-machine.svg
> +++ b/doc/touchpad-softbutton-state-machine.svg

[...]

> diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> index b4d3920..566880f 100644
> --- a/src/evdev-mt-touchpad-buttons.c
> +++ b/src/evdev-mt-touchpad-buttons.c
> @@ -31,6 +31,10 @@
>  
>  #include "evdev-mt-touchpad.h"
>  
> +#ifndef INPUT_PROP_TOPBUTTONPAD
> +#define INPUT_PROP_TOPBUTTONPAD 0x04
> +#endif
> +
>  #define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* 2% of size */
>  #define DEFAULT_BUTTON_ENTER_TIMEOUT 100 /* ms */
>  #define DEFAULT_BUTTON_LEAVE_TIMEOUT 300 /* ms */
> @@ -56,6 +60,10 @@ button_state_to_str(enum button_state state) {
>  	CASE_RETURN_STRING(BUTTON_STATE_BOTTOM);
>  	CASE_RETURN_STRING(BUTTON_STATE_BOTTOM_NEW);
>  	CASE_RETURN_STRING(BUTTON_STATE_BOTTOM_TO_AREA);
> +	CASE_RETURN_STRING(BUTTON_STATE_TOP);
> +	CASE_RETURN_STRING(BUTTON_STATE_TOP_NEW);
> +	CASE_RETURN_STRING(BUTTON_STATE_TOP_TO_IGNORE);
> +	CASE_RETURN_STRING(BUTTON_STATE_IGNORE);
>  	}
>  	return NULL;
>  }
> @@ -65,6 +73,9 @@ button_event_to_str(enum button_event event) {
>  	switch(event) {
>  	CASE_RETURN_STRING(BUTTON_EVENT_IN_BOTTOM_R);
>  	CASE_RETURN_STRING(BUTTON_EVENT_IN_BOTTOM_L);
> +	CASE_RETURN_STRING(BUTTON_EVENT_IN_TOP_R);
> +	CASE_RETURN_STRING(BUTTON_EVENT_IN_TOP_M);
> +	CASE_RETURN_STRING(BUTTON_EVENT_IN_TOP_L);
>  	CASE_RETURN_STRING(BUTTON_EVENT_IN_AREA);
>  	CASE_RETURN_STRING(BUTTON_EVENT_UP);
>  	CASE_RETURN_STRING(BUTTON_EVENT_PRESS);
> @@ -94,6 +105,34 @@ is_inside_bottom_left_area(struct tp_dispatch *tp, struct tp_touch *t)
>  	       !is_inside_bottom_right_area(tp, t);
>  }
>  
> +static inline bool
> +is_inside_top_button_area(struct tp_dispatch *tp, struct tp_touch *t)
> +{
> +	return t->y <= tp->buttons.top_area.bottom_edge;
> +}
> +
> +static inline bool
> +is_inside_top_right_area(struct tp_dispatch *tp, struct tp_touch *t)
> +{
> +	return is_inside_top_button_area(tp, t) &&
> +	       t->x > tp->buttons.top_area.rightbutton_left_edge;
> +}
> +
> +static inline bool
> +is_inside_top_left_area(struct tp_dispatch *tp, struct tp_touch *t)
> +{
> +	return is_inside_top_button_area(tp, t) &&
> +	       t->x < tp->buttons.top_area.leftbutton_right_edge;
> +}
> +
> +static inline bool
> +is_inside_top_middle_area(struct tp_dispatch *tp, struct tp_touch *t)
> +{
> +	return is_inside_top_button_area(tp, t) &&
> +	       t->x >= tp->buttons.top_area.leftbutton_right_edge &&
> +	       t->x <= tp->buttons.top_area.rightbutton_left_edge;
> +}
> +
>  static void
>  tp_button_set_timer(struct tp_dispatch *tp, uint64_t timeout)
>  {
> @@ -153,6 +192,18 @@ tp_button_set_state(struct tp_dispatch *tp, struct tp_touch *t,
>  	case BUTTON_STATE_BOTTOM_TO_AREA:
>  		tp_button_set_leave_timer(tp, t);
>  		break;
> +	case BUTTON_STATE_TOP:
> +		break;
> +	case BUTTON_STATE_TOP_NEW:
> +		t->button.curr = event;
> +		tp_button_set_enter_timer(tp, t);
> +		break;
> +	case BUTTON_STATE_TOP_TO_IGNORE:
> +		tp_button_set_leave_timer(tp, t);
> +		break;
> +	case BUTTON_STATE_IGNORE:
> +		t->button.curr = 0;

can you send a follow-up to replace the 0s in this file with an NONE enum
value? or squash it into this, either one is fine with me.

also, one thing I noticed is that in the diagram we use "inner/outer"
timeout, vs "enter/leave" timer here. This should be adjusted.

> +		break;
>  	}
>  }
>  
> @@ -166,6 +217,11 @@ tp_button_none_handle_event(struct tp_dispatch *tp,
>  	case BUTTON_EVENT_IN_BOTTOM_L:
>  		tp_button_set_state(tp, t, BUTTON_STATE_BOTTOM_NEW, event);
>  		break;
> +	case BUTTON_EVENT_IN_TOP_R:
> +	case BUTTON_EVENT_IN_TOP_M:
> +	case BUTTON_EVENT_IN_TOP_L:
> +		tp_button_set_state(tp, t, BUTTON_STATE_TOP_NEW, event);
> +		break;
>  	case BUTTON_EVENT_IN_AREA:
>  		tp_button_set_state(tp, t, BUTTON_STATE_AREA, event);
>  		break;
> @@ -187,6 +243,9 @@ tp_button_area_handle_event(struct tp_dispatch *tp,
>  	switch (event) {
>  	case BUTTON_EVENT_IN_BOTTOM_R:
>  	case BUTTON_EVENT_IN_BOTTOM_L:
> +	case BUTTON_EVENT_IN_TOP_R:
> +	case BUTTON_EVENT_IN_TOP_M:
> +	case BUTTON_EVENT_IN_TOP_L:
>  	case BUTTON_EVENT_IN_AREA:
>  		break;
>  	case BUTTON_EVENT_UP:
> @@ -212,6 +271,9 @@ tp_button_bottom_handle_event(struct tp_dispatch *tp,
>  					    event);
>  		break;
>  	case BUTTON_EVENT_IN_AREA:
> +	case BUTTON_EVENT_IN_TOP_R:
> +	case BUTTON_EVENT_IN_TOP_M:
> +	case BUTTON_EVENT_IN_TOP_L:

nitpick: you have AREA, then TOP_RML, in the two hunks above it's the other
way round. I'd prefer this to be the same order everywhere, this especially
applies down in tp_button_top*_handle_event.

>  		tp_button_set_state(tp, t, BUTTON_STATE_BOTTOM_TO_AREA, event);
>  		break;
>  	case BUTTON_EVENT_UP:

[...]

> @@ -412,6 +613,8 @@ tp_init_buttons(struct tp_dispatch *tp,
>  
>  	tp->buttons.is_clickpad = libevdev_has_property(device->evdev,
>  							INPUT_PROP_BUTTONPAD);
> +	tp->buttons.has_topbuttons = libevdev_has_property(device->evdev,
> +						     INPUT_PROP_TOPBUTTONPAD);

make sure this is aligned please, even if it goes over the 78.


>  	if (libevdev_has_event_code(device->evdev, EV_KEY, BTN_MIDDLE) ||
>  	    libevdev_has_event_code(device->evdev, EV_KEY, BTN_RIGHT)) {
> @@ -434,8 +637,16 @@ tp_init_buttons(struct tp_dispatch *tp,
>  	if (tp->buttons.is_clickpad && !tp->buttons.use_clickfinger) {
>  		tp->buttons.bottom_area.top_edge = height * .8 + device->abs.min_y;
>  		tp->buttons.bottom_area.rightbutton_left_edge = width/2 + device->abs.min_x;
> -		tp->buttons.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
>  
> +		if (tp->buttons.has_topbuttons) {
> +			tp->buttons.top_area.bottom_edge = height * .08 + device->abs.min_y;
> +			tp->buttons.top_area.rightbutton_left_edge = width * .58 + device->abs.min_x;
> +			tp->buttons.top_area.leftbutton_right_edge = width * .42 + device->abs.min_x;
> +		} else {
> +			tp->buttons.top_area.bottom_edge = 0;
> +		}
> +
> +		tp->buttons.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
>  		if (tp->buttons.timer_fd == -1)
>  			return -1;
>  
> @@ -448,6 +659,7 @@ tp_init_buttons(struct tp_dispatch *tp,
>  			return -1;
>  	} else {
>  		tp->buttons.bottom_area.top_edge = INT_MAX;
> +		tp->buttons.top_area.bottom_edge = 0;

INT_MIN maybe? none of the current TOP_BUTTONPAD devices have a negative y
axes but better safe than sorry. I remember one bugreport where a touchpad
had a range of [-foo, +bar] though I suspect that was a local kernel issue.

tested on the t440s, and Reviewed-by: Peter Hutterer
<peter.hutterer at who-t.net> for both, with the changes above.

And I suspect the patch to add the tests for this just accidentally got
stuck in your outbox ;)

Cheers,
   Peter


More information about the wayland-devel mailing list