[PATCH libinput] touchpad: add a middle button software area

Hans de Goede hdegoede at redhat.com
Mon Apr 4 11:07:01 UTC 2016


Hi,

On 04-04-16 03:40, Peter Hutterer wrote:
> Middle button interaction is most commonly to paste and it is a single-event
> interaction (button press). We provided middle button in software button mode
> by emulating it with a two-finger press with L+R down at the same time. This
> is also what many touchpads are spectacularly bad at, it is very common to
> detect the physical button down event before the second finger registers,
> resulting in left or right clicks where a middle button should be triggered.
>
> Unless the fingers are resting on the touchpad for at least one scanout, the
> success rate for middle button emulation is only at 70% or so.
>
> This patch adds a 25%-width middle button area between the left and the right
> software button, everything else stays the same. To avoid immediate breakage,
> the middle button emulation remains but may be removed in the future.
> The doc is updated to only refer to the middle button area now.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=94755
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Patch looks good to me:

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

Regards,

Hans


> ---
>   doc/clickpad-softbuttons.dox    | 14 +++++++++-----
>   doc/svg/software-buttons.svg    | 30 +++---------------------------
>   src/evdev-mt-touchpad-buttons.c | 31 ++++++++++++++++++++++++++++++-
>   src/evdev-mt-touchpad.h         |  2 ++
>   test/touchpad-buttons.c         | 30 ++++++++++++++++++++++++++++++
>   5 files changed, 74 insertions(+), 33 deletions(-)
>
> diff --git a/doc/clickpad-softbuttons.dox b/doc/clickpad-softbuttons.dox
> index a4f2e44..cf4c8c9 100644
> --- a/doc/clickpad-softbuttons.dox
> +++ b/doc/clickpad-softbuttons.dox
> @@ -20,16 +20,20 @@ generated by libinput and passed to the caller in response to a click.
>   @section software_buttons Software button areas
>
>   On most clickpads, this is the default behavior. The bottom of the touchpad
> -is split in the middle to generate left or right button events on click. The
> -height of the button area depends on the hardware but is usually around
> -10mm.
> +is split into three distinct areas generate left, middle or right button
> +events on click. The height of the button area depends on the hardware but
> +is usually around 10mm.
>
>   Left, right and middle button events can be triggered as follows:
>   - if a finger is in the main area or the left button area, a click generates
>     left button events.
>   - if a finger is in the right area, a click generates right button events.
> -- if there is a finger in both the left and right button area, a click
> -  generates middle button events.
> +- if a finger is in the middle area, a click generates middle button events.
> +
> +The middle button is always centered on the touchpad and smaller in size
> +than the left or right button. The actual size is device-dependent though as
> +many touchpads do not have visible markings for the middle button the exact
> +location of the button is not visibly obvious.
>
>   @image html software-buttons.svg "Left, right and middle-button click with software button areas"
>
> diff --git a/doc/svg/software-buttons.svg b/doc/svg/software-buttons.svg
> index 903535c..c0bc610 100644
> --- a/doc/svg/software-buttons.svg
> +++ b/doc/svg/software-buttons.svg
> @@ -41,8 +41,8 @@
>        id="namedview4312"
>        showgrid="false"
>        inkscape:zoom="0.57798581"
> -     inkscape:cx="1134.9723"
> -     inkscape:cy="-71.183873"
> +     inkscape:cx="842.57758"
> +     inkscape:cy="-74.644166"
>        inkscape:window-x="0"
>        inkscape:window-y="27"
>        inkscape:window-maximized="1"
> @@ -106,7 +106,7 @@
>        id="rect2858-7"
>        style="color:#000000;display:inline;overflow:visible;visibility:visible;fill:#b3b3b3;fill-opacity:1;fill-rule:nonzero;stroke:#000000;stroke-width:5.19376326;stroke-miterlimit:4;stroke-dasharray:none;stroke-opacity:1;marker:none;enable-background:accumulate" />
>     <g
> -     transform="matrix(0.79657897,0.11742288,-0.14814182,0.631399,665.11943,-345.64117)"
> +     transform="matrix(0.79657897,0.11742288,-0.14814182,0.631399,749.8966,-336.99044)"
>        id="g3663-9-8">
>       <path
>          d="m 388.57143,893.79076 -57.14285,-130 c 0,0 -30.0247,-58.84827 4.28571,-70.00001 27.07438,-8.79984 37.32196,9.59496 40,14.64286 27.54455,51.91936 84.64285,173.21429 84.64285,173.21429 l -0.71428,0 -71.07143,12.14286 z"
> @@ -125,30 +125,6 @@
>          inkscape:connector-curvature="0" />
>     </g>
>     <g
> -     id="g4656"
> -     transform="translate(-0.31997204,0.28487182)">
> -    <g
> -       id="g4639">
> -      <path
> -         inkscape:connector-curvature="0"
> -         style="fill:none;stroke:#000000;stroke-width:0.72135597px;stroke-linecap:butt;stroke-linejoin:miter;stroke-opacity:1"
> -         id="path2820-0-5"
> -         d="m 970.15914,263.93369 32.58666,-97.47985 c 0,0 12.2603,-46.32869 38.5506,-33.925 20.7457,9.78778 17.4301,24.72594 16.4399,28.75425 -10.1846,41.43257 -30.105,105.84033 -30.105,105.84033 l -0.428,-0.37827 z"
> -         sodipodi:nodetypes="ccssccc" />
> -      <path
> -         inkscape:connector-curvature="0"
> -         style="color:#000000;display:inline;overflow:visible;visibility:visible;fill:#ffccaa;fill-opacity:1;fill-rule:nonzero;stroke:#000000;stroke-width:0.00144271;stroke-miterlimit:4;stroke-dasharray:none;stroke-opacity:1;marker:none;enable-background:accumulate"
> -         id="path2824-9-3"
> -         d="m 987.16367,214.84092 c 8.97014,-26.84686 16.75933,-50.19953 17.30923,-51.89481 3.8651,-11.91483 10.3862,-22.76212 16.5622,-27.54951 6.8496,-5.30946 13.8243,-5.75615 21.9108,-1.40323 3.7999,2.04543 6.0302,3.61208 8.2265,5.77836 2.5214,2.487 3.6881,4.17002 5.1008,7.35828 1.3655,3.08181 1.9391,7.11725 1.5051,10.58923 -0.597,4.77663 -10.2821,40.41668 -20.9931,77.25236 -7.7256,26.56907 -9.371,31.11182 -9.5644,31.10964 -0.1479,-0.002 -55.70067,-1.83937 -56.08626,-2.18017 -0.16723,-0.1478 6.29681,-19.93218 16.02913,-49.06015 z"
> -         sodipodi:nodetypes="scsssscsccs" />
> -      <path
> -         inkscape:connector-curvature="0"
> -         style="color:#000000;display:inline;overflow:visible;visibility:visible;opacity:0.92000002;fill:#ffe6d5;fill-opacity:1;fill-rule:nonzero;stroke:none;stroke-width:0.2;marker:none;enable-background:accumulate"
> -         id="path2824-7-8-8"
> -         d="m 1008.3595,164.57667 c 3.8651,-11.91483 7.6606,-19.35039 13.8366,-24.13778 6.8495,-5.30946 12.0833,-5.57765 20.1698,-1.22474 9.3061,4.73331 12.9905,9.62271 11.9094,19.03362 -6.3459,19.3209 -6.9054,22.12042 -9.2168,26.32727 -0.1479,-0.002 -33.6651,-14.70742 -35.0296,-15.23839 -1.4035,-0.54616 -1.8884,-3.70289 -1.6694,-4.75998 z" />
> -    </g>
> -  </g>
> -  <g
>        transform="translate(-386.56163,2.2570367)"
>        id="g4656-3">
>       <g
> diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> index 076eab0..a9b7521 100644
> --- a/src/evdev-mt-touchpad-buttons.c
> +++ b/src/evdev-mt-touchpad-buttons.c
> @@ -64,6 +64,7 @@ static inline const char*
>   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_M);
>   	CASE_RETURN_STRING(BUTTON_EVENT_IN_BOTTOM_L);
>   	CASE_RETURN_STRING(BUTTON_EVENT_IN_TOP_R);
>   	CASE_RETURN_STRING(BUTTON_EVENT_IN_TOP_M);
> @@ -93,10 +94,20 @@ is_inside_bottom_right_area(const struct tp_dispatch *tp,
>   }
>
>   static inline bool
> +is_inside_bottom_middle_area(const struct tp_dispatch *tp,
> +			   const struct tp_touch *t)
> +{
> +	return is_inside_bottom_button_area(tp, t) &&
> +	       !is_inside_bottom_right_area(tp, t) &&
> +	       t->point.x > tp->buttons.bottom_area.middlebutton_left_edge;
> +}
> +
> +static inline bool
>   is_inside_bottom_left_area(const struct tp_dispatch *tp,
>   			   const struct tp_touch *t)
>   {
>   	return is_inside_bottom_button_area(tp, t) &&
> +	       !is_inside_bottom_middle_area(tp, t) &&
>   	       !is_inside_bottom_right_area(tp, t);
>   }
>
> @@ -192,6 +203,7 @@ tp_button_none_handle_event(struct tp_dispatch *tp,
>   {
>   	switch (event) {
>   	case BUTTON_EVENT_IN_BOTTOM_R:
> +	case BUTTON_EVENT_IN_BOTTOM_M:
>   	case BUTTON_EVENT_IN_BOTTOM_L:
>   		tp_button_set_state(tp, t, BUTTON_STATE_BOTTOM, event);
>   		break;
> @@ -220,6 +232,7 @@ tp_button_area_handle_event(struct tp_dispatch *tp,
>   {
>   	switch (event) {
>   	case BUTTON_EVENT_IN_BOTTOM_R:
> +	case BUTTON_EVENT_IN_BOTTOM_M:
>   	case BUTTON_EVENT_IN_BOTTOM_L:
>   	case BUTTON_EVENT_IN_TOP_R:
>   	case BUTTON_EVENT_IN_TOP_M:
> @@ -243,6 +256,7 @@ tp_button_bottom_handle_event(struct tp_dispatch *tp,
>   {
>   	switch (event) {
>   	case BUTTON_EVENT_IN_BOTTOM_R:
> +	case BUTTON_EVENT_IN_BOTTOM_M:
>   	case BUTTON_EVENT_IN_BOTTOM_L:
>   		if (event != t->button.curr)
>   			tp_button_set_state(tp,
> @@ -273,6 +287,7 @@ tp_button_top_handle_event(struct tp_dispatch *tp,
>   {
>   	switch (event) {
>   	case BUTTON_EVENT_IN_BOTTOM_R:
> +	case BUTTON_EVENT_IN_BOTTOM_M:
>   	case BUTTON_EVENT_IN_BOTTOM_L:
>   		tp_button_set_state(tp, t, BUTTON_STATE_TOP_TO_IGNORE, event);
>   		break;
> @@ -305,6 +320,7 @@ tp_button_top_new_handle_event(struct tp_dispatch *tp,
>   {
>   	switch(event) {
>   	case BUTTON_EVENT_IN_BOTTOM_R:
> +	case BUTTON_EVENT_IN_BOTTOM_M:
>   	case BUTTON_EVENT_IN_BOTTOM_L:
>   		tp_button_set_state(tp, t, BUTTON_STATE_AREA, event);
>   		break;
> @@ -355,6 +371,7 @@ tp_button_top_to_ignore_handle_event(struct tp_dispatch *tp,
>   					    event);
>   		break;
>   	case BUTTON_EVENT_IN_BOTTOM_R:
> +	case BUTTON_EVENT_IN_BOTTOM_M:
>   	case BUTTON_EVENT_IN_BOTTOM_L:
>   	case BUTTON_EVENT_IN_AREA:
>   		break;
> @@ -377,6 +394,7 @@ tp_button_ignore_handle_event(struct tp_dispatch *tp,
>   {
>   	switch (event) {
>   	case BUTTON_EVENT_IN_BOTTOM_R:
> +	case BUTTON_EVENT_IN_BOTTOM_M:
>   	case BUTTON_EVENT_IN_BOTTOM_L:
>   	case BUTTON_EVENT_IN_TOP_R:
>   	case BUTTON_EVENT_IN_TOP_M:
> @@ -450,6 +468,8 @@ tp_button_handle_state(struct tp_dispatch *tp, uint64_t time)
>
>   			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))
> @@ -543,7 +563,14 @@ tp_init_softbuttons(struct tp_dispatch *tp,
>   	} else {
>   		tp->buttons.bottom_area.top_edge = height * .85 + yoffset;
>   	}
> -	tp->buttons.bottom_area.rightbutton_left_edge = width/2 + xoffset;
> +
> +	/* The middle button is 25% of the touchpad and centered. Many
> +	 * touchpads don't have markings for the middle button at all so we
> +	 * need to make it big enough to reliably hit it but not too big so
> +	 * it takes away all the space.
> +	 */
> +	tp->buttons.bottom_area.middlebutton_left_edge = width * 0.375 + xoffset;
> +	tp->buttons.bottom_area.rightbutton_left_edge = width * 0.625 + xoffset;
>   }
>
>   void
> @@ -989,6 +1016,8 @@ tp_post_clickpadbutton_buttons(struct tp_dispatch *tp, uint64_t time)
>   				break;
>   			case BUTTON_EVENT_IN_TOP_M:
>   				is_top = 1;
> +				/* fallthrough */
> +			case BUTTON_EVENT_IN_BOTTOM_M:
>   				area |= MIDDLE;
>   				break;
>   			case BUTTON_EVENT_IN_TOP_R:
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index 1f05a03..7277726 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -70,6 +70,7 @@ enum touch_palm_state {
>
>   enum button_event {
>   	BUTTON_EVENT_IN_BOTTOM_R = 30,
> +	BUTTON_EVENT_IN_BOTTOM_M,
>   	BUTTON_EVENT_IN_BOTTOM_L,
>   	BUTTON_EVENT_IN_TOP_R,
>   	BUTTON_EVENT_IN_TOP_M,
> @@ -283,6 +284,7 @@ struct tp_dispatch {
>   		struct {
>   			int32_t top_edge;	/* in device coordinates */
>   			int32_t rightbutton_left_edge; /* in device coordinates */
> +			int32_t middlebutton_left_edge; /* in device coordinates */
>   		} bottom_area;
>
>   		struct {
> diff --git a/test/touchpad-buttons.c b/test/touchpad-buttons.c
> index 064c29e..080c670 100644
> --- a/test/touchpad-buttons.c
> +++ b/test/touchpad-buttons.c
> @@ -987,6 +987,35 @@ START_TEST(clickpad_softbutton_left)
>   }
>   END_TEST
>
> +START_TEST(clickpad_softbutton_middle)
> +{
> +	struct litest_device *dev = litest_current_device();
> +	struct libinput *li = dev->libinput;
> +
> +	litest_drain_events(li);
> +
> +	litest_touch_down(dev, 0, 50, 90);
> +	litest_event(dev, EV_KEY, BTN_LEFT, 1);
> +	litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +
> +	litest_assert_button_event(li,
> +				   BTN_MIDDLE,
> +				   LIBINPUT_BUTTON_STATE_PRESSED);
> +
> +	litest_event(dev, EV_KEY, BTN_LEFT, 0);
> +	litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +	litest_touch_up(dev, 0);
> +
> +	litest_assert_button_event(li,
> +				   BTN_MIDDLE,
> +				   LIBINPUT_BUTTON_STATE_RELEASED);
> +
> +	libinput_dispatch(li);
> +
> +	litest_assert_empty_queue(li);
> +}
> +END_TEST
> +
>   START_TEST(clickpad_softbutton_right)
>   {
>   	struct litest_device *dev = litest_current_device();
> @@ -1585,6 +1614,7 @@ litest_setup_tests(void)
>   	litest_add("touchpad:click", clickpad_finger_pin, LITEST_CLICKPAD, LITEST_ANY);
>
>   	litest_add("touchpad:softbutton", clickpad_softbutton_left, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD);
> +	litest_add("touchpad:softbutton", clickpad_softbutton_middle, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD);
>   	litest_add("touchpad:softbutton", clickpad_softbutton_right, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD);
>   	litest_add("touchpad:softbutton", clickpad_softbutton_left_tap_n_drag, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD);
>   	litest_add("touchpad:softbutton", clickpad_softbutton_right_tap_n_drag, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD);
>


More information about the wayland-devel mailing list