[PATCH libinput v2 3/3] touchpad: Add edge-scrolling support

Hans de Goede hdegoede at redhat.com
Thu Nov 20 01:34:51 PST 2014


Hi,

On 11/20/2014 07:21 AM, Peter Hutterer wrote:
> On Wed, Nov 19, 2014 at 01:45:35PM +0100, Hans de Goede wrote:
>> Add edge-scrolling support for non multi-touch touchpads as well as for
>> users who prefer edge-scrolling (as long as they don't have a clickpad).
>>
>> Note the percentage to use of the width / height as scroll-edge differs from
>> one manufacturer to the next, the various per model percentages were taken
>> from xf86-input-synaptics.
>>
>> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=85635
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>  doc/touchpad-edge-scrolling-state-machine.svg | 262 ++++++++++++++++++
>>  src/Makefile.am                               |   1 +
>>  src/evdev-mt-touchpad-edge-scroll.c           | 373 ++++++++++++++++++++++++++
>>  src/evdev-mt-touchpad.c                       |  93 ++++++-
>>  src/evdev-mt-touchpad.h                       |  46 ++++
>>  5 files changed, 763 insertions(+), 12 deletions(-)
>>  create mode 100644 doc/touchpad-edge-scrolling-state-machine.svg
>>  create mode 100644 src/evdev-mt-touchpad-edge-scroll.c
>>
>> diff --git a/doc/touchpad-edge-scrolling-state-machine.svg b/doc/touchpad-edge-scrolling-state-machine.svg
>> new file mode 100644
>> index 0000000..7a82d88
> [...]
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 5cc52a6..027e08c 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -15,6 +15,7 @@ libinput_la_SOURCES =			\
>>  	evdev-mt-touchpad.h		\
>>  	evdev-mt-touchpad-tap.c		\
>>  	evdev-mt-touchpad-buttons.c	\
>> +	evdev-mt-touchpad-edge-scroll.c	\
>>  	filter.c			\
>>  	filter.h			\
>>  	filter-private.h		\
>> diff --git a/src/evdev-mt-touchpad-edge-scroll.c b/src/evdev-mt-touchpad-edge-scroll.c
>> new file mode 100644
>> index 0000000..3647546
>> --- /dev/null
>> +++ b/src/evdev-mt-touchpad-edge-scroll.c
>> @@ -0,0 +1,373 @@
>> +/*
>> + * Copyright © 2014 Red Hat, Inc.
>> + *
>> + * Permission to use, copy, modify, distribute, and sell this software and
>> + * its documentation for any purpose is hereby granted without fee, provided
>> + * that the above copyright notice appear in all copies and that both that
>> + * copyright notice and this permission notice appear in supporting
>> + * documentation, and that the name of the copyright holders not be used in
>> + * advertising or publicity pertaining to distribution of the software
>> + * without specific, written prior permission.  The copyright holders make
>> + * no representations about the suitability of this software for any
>> + * purpose.  It is provided "as is" without express or implied warranty.
>> + *
>> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
>> + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
>> + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
>> + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
>> + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
>> + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
>> + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>> +
>> +#include <errno.h>
>> +#include <limits.h>
>> +#include <math.h>
>> +#include <string.h>
>> +#include <unistd.h>
>> +#include "linux/input.h"
>> +
>> +#include "evdev-mt-touchpad.h"
>> +
>> +#define DEFAULT_SCROLL_LOCK_TIMEOUT 300 /* ms */
>> +/* Use a reasonably large threshold until locked into scrolling mode, to
>> +   avoid accidentally locking in scrolling mode when trying to use the entire
>> +   touchpad to move the pointer. The user can wait for the timeout to trigger
>> +   to do a small scroll. */
>> +#define DEFAULT_SCROLL_THRESHOLD 10.0
> 
> what's the unit of this? should be mentioned

Ok, will fix.

> 
>> +
>> +enum scroll_event {
>> +	SCROLL_EVENT_TOUCH,
>> +	SCROLL_EVENT_MOTION,
>> +	SCROLL_EVENT_RELEASE,
>> +	SCROLL_EVENT_TIMEOUT,
>> +	SCROLL_EVENT_POSTED,
>> +};
>> +
>> +static uint32_t
>> +tp_touch_get_edge(struct tp_dispatch *tp, struct tp_touch *touch)
>> +{
>> +	uint32_t edge = EDGE_NONE;
>> +
>> +	if (tp->scroll.mode != LIBINPUT_CONFIG_SCROLL_EDGE)
>> +		return 0;
>> +
> 
> could just return edge (or EDGE_NONE if you want to be specific)

Good catch, will fix.

> 
>> +	if (touch->x > tp->scroll.right_edge)
>> +		edge |= EDGE_RIGHT;
>> +
>> +	if (touch->y > tp->scroll.bottom_edge)
>> +		edge |= EDGE_BOTTOM;
>> +
>> +	return edge;
>> +}
>> +
>> +static void
>> +tp_edge_scroll_set_state(struct tp_dispatch *tp,
>> +			 struct tp_touch *t,
>> +			 enum tp_edge_scroll_touch_state state)
>> +{
>> +	libinput_timer_cancel(&t->scroll.timer);
>> +
>> +	t->scroll.state = state;
>> +
>> +	switch (state) {
>> +	case EDGE_SCROLL_TOUCH_STATE_NONE:
>> +		t->scroll.edge = EDGE_NONE;
>> +		t->scroll.threshold = DEFAULT_SCROLL_THRESHOLD;
>> +		break;
>> +	case EDGE_SCROLL_TOUCH_STATE_EDGE_NEW:
>> +		t->scroll.edge = tp_touch_get_edge(tp, t);
>> +		libinput_timer_set(&t->scroll.timer,
>> +				   t->millis + DEFAULT_SCROLL_LOCK_TIMEOUT);
>> +		break;
>> +	case EDGE_SCROLL_TOUCH_STATE_EDGE:
>> +		t->scroll.threshold = 0.01; /* Do not allow 0.0 events */
>> +		break;
>> +	case EDGE_SCROLL_TOUCH_STATE_AREA:
>> +		t->scroll.edge = EDGE_NONE;
>> +		tp_set_pointer(tp, t);
>> +		break;
>> +	}
>> +}
>> +
>> +static void
>> +tp_edge_scroll_handle_none(struct tp_dispatch *tp,
>> +			   struct tp_touch *t,
>> +			   enum scroll_event event)
>> +{
>> +	struct libinput *libinput = tp->device->base.seat->libinput;
>> +
>> +	switch (event) {
>> +	case SCROLL_EVENT_TOUCH:
>> +		if (tp_touch_get_edge(tp, t)) {
>> +			tp_edge_scroll_set_state(tp, t,
>> +					EDGE_SCROLL_TOUCH_STATE_EDGE_NEW);
>> +		} else {
>> +			tp_edge_scroll_set_state(tp, t,
>> +					EDGE_SCROLL_TOUCH_STATE_AREA);
>> +		}
>> +		break;
>> +	case SCROLL_EVENT_MOTION:
>> +	case SCROLL_EVENT_RELEASE:
>> +	case SCROLL_EVENT_TIMEOUT:
>> +	case SCROLL_EVENT_POSTED:
>> +		log_bug_libinput(libinput,
>> +				 "unexpect scroll event in none state\n");
>> +		break;
>> +	}
>> +}
>> +
>> +static void
>> +tp_edge_scroll_handle_edge_new(struct tp_dispatch *tp,
>> +			       struct tp_touch *t,
>> +			       enum scroll_event event)
>> +{
>> +	struct libinput *libinput = tp->device->base.seat->libinput;
>> +
>> +	switch (event) {
>> +	case SCROLL_EVENT_TOUCH:
>> +		log_bug_libinput(libinput,
>> +				 "unexpect scroll event in edge new state\n");
>> +		break;
>> +	case SCROLL_EVENT_MOTION:
>> +		t->scroll.edge &= tp_touch_get_edge(tp, t);
>> +		if (!t->scroll.edge)
>> +			tp_edge_scroll_set_state(tp, t,
>> +					EDGE_SCROLL_TOUCH_STATE_AREA);
>> +		break;
>> +	case SCROLL_EVENT_RELEASE:
>> +		tp_edge_scroll_set_state(tp, t, EDGE_SCROLL_TOUCH_STATE_NONE);
>> +		break;
>> +	case SCROLL_EVENT_TIMEOUT:
>> +	case SCROLL_EVENT_POSTED:
>> +		tp_edge_scroll_set_state(tp, t, EDGE_SCROLL_TOUCH_STATE_EDGE);
>> +		break;
>> +	}
>> +}
>> +
>> +static void
>> +tp_edge_scroll_handle_edge(struct tp_dispatch *tp,
>> +			   struct tp_touch *t,
>> +			   enum scroll_event event)
>> +{
>> +	struct libinput *libinput = tp->device->base.seat->libinput;
>> +
>> +	switch (event) {
>> +	case SCROLL_EVENT_TOUCH:
>> +	case SCROLL_EVENT_TIMEOUT:
>> +		log_bug_libinput(libinput,
>> +				 "unexpect scroll event in edge state\n");
>> +		break;
>> +	case SCROLL_EVENT_MOTION:
>> +		/* If started at the bottom right, decide in which dir to scroll */
>> +		if (t->scroll.edge == (EDGE_RIGHT | EDGE_BOTTOM)) {
>> +			t->scroll.edge &= tp_touch_get_edge(tp, t);
>> +			if (!t->scroll.edge)
>> +				tp_edge_scroll_set_state(tp, t,
>> +						EDGE_SCROLL_TOUCH_STATE_AREA);
>> +		}
>> +		break;
>> +	case SCROLL_EVENT_RELEASE:
>> +		tp_edge_scroll_set_state(tp, t, EDGE_SCROLL_TOUCH_STATE_NONE);
>> +		break;
>> +	case SCROLL_EVENT_POSTED:
>> +		break;
>> +	}
>> +}
>> +
>> +static void
>> +tp_edge_scroll_handle_area(struct tp_dispatch *tp,
>> +			   struct tp_touch *t,
>> +			   enum scroll_event event)
>> +{
>> +	struct libinput *libinput = tp->device->base.seat->libinput;
>> +
>> +	switch (event) {
>> +	case SCROLL_EVENT_TOUCH:
>> +	case SCROLL_EVENT_TIMEOUT:
>> +	case SCROLL_EVENT_POSTED:
>> +		log_bug_libinput(libinput,
>> +				 "unexpect scroll event in area state\n");
>> +		break;
>> +	case SCROLL_EVENT_MOTION:
>> +		break;
>> +	case SCROLL_EVENT_RELEASE:
>> +		tp_edge_scroll_set_state(tp, t, EDGE_SCROLL_TOUCH_STATE_NONE);
>> +		break;
>> +	}
>> +}
>> +
>> +static void
>> +tp_edge_scroll_handle_event(struct tp_dispatch *tp,
>> +			    struct tp_touch *t,
>> +			    enum scroll_event event)
>> +{
>> +	switch (t->scroll.state) {
>> +	case EDGE_SCROLL_TOUCH_STATE_NONE:
>> +		tp_edge_scroll_handle_none(tp, t, event);
>> +		break;
>> +	case EDGE_SCROLL_TOUCH_STATE_EDGE_NEW:
>> +		tp_edge_scroll_handle_edge_new(tp, t, event);
>> +		break;
>> +	case EDGE_SCROLL_TOUCH_STATE_EDGE:
>> +		tp_edge_scroll_handle_edge(tp, t, event);
>> +		break;
>> +	case EDGE_SCROLL_TOUCH_STATE_AREA:
>> +		tp_edge_scroll_handle_area(tp, t, event);
>> +		break;
>> +	}
>> +}
>> +
>> +static void
>> +tp_edge_scroll_handle_timeout(uint64_t now, void *data)
>> +{
>> +	struct tp_touch *t = data;
>> +
>> +	tp_edge_scroll_handle_event(t->tp, t, SCROLL_EVENT_TIMEOUT);
>> +}
>> +
>> +int
>> +tp_edge_scroll_init(struct tp_dispatch *tp, struct evdev_device *device)
>> +{
>> +	struct tp_touch *t;
>> +	int width, height;
>> +	int edge_width, edge_height;
>> +
>> +	width = device->abs.absinfo_x->maximum - device->abs.absinfo_x->minimum;
>> +	height = device->abs.absinfo_y->maximum - device->abs.absinfo_y->minimum;
>> +
>> +	switch (tp->model) {
>> +	case MODEL_SYNAPTICS:
>> +		edge_width = width * .07;
>> +		edge_height = height * .07;
>> +		break;
>> +	case MODEL_ALPS:
>> +		edge_width = width * .15;
>> +		edge_height = height * .15;
>> +		break;
>> +	case MODEL_APPLETOUCH:
>> +	case MODEL_UNIBODY_MACBOOK:
> 
> unless there's one I didn't find in my quick search, the unibodies all had
> clickpads so we should skip this here and maybe leave a comment for that.

But keep the APPLETOUCH ?

> 
>> +		edge_width = width * .085;
>> +		edge_height = height * .085;
>> +		break;
>> +	default:
>> +		edge_width = width * .04;
>> +		edge_height = height * .054;
> 
> make MODEL_SYNAPTICS the same as default please

So use .04 and .054 for synaptics too, and drop the SYNAPTICS case ?



> 
>> +	}
>> +
>> +	tp->scroll.right_edge = device->abs.absinfo_x->maximum - edge_width;
>> +	tp->scroll.bottom_edge = device->abs.absinfo_y->maximum - edge_height;
>> +
>> +	tp_for_each_touch(tp, t) {
>> +		t->scroll.last_axis = -1;
> 
> can we name this direction like in the main scrolling code? or will this
> lead to confusion?

direction wfm, will fix.

> 
>> +		t->scroll.threshold = DEFAULT_SCROLL_THRESHOLD;
>> +		libinput_timer_init(&t->scroll.timer, 
>> +				    device->base.seat->libinput,
>> +				    tp_edge_scroll_handle_timeout, t);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +void
>> +tp_destroy_edge_scroll(struct tp_dispatch *tp)
>> +{
>> +	struct tp_touch *t;
>> +
>> +	tp_for_each_touch(tp, t)
>> +		libinput_timer_cancel(&t->scroll.timer);
>> +}
>> +
>> +void
>> +tp_edge_scroll_handle_state(struct tp_dispatch *tp, uint64_t time)
>> +{
>> +	struct tp_touch *t;
>> +
>> +	tp_for_each_touch(tp, t) {
>> +		if (!t->dirty)
>> +			continue;
>> +
>> +		switch (t->state) {
>> +		case TOUCH_NONE:
>> +			break;
>> +		case TOUCH_BEGIN:
>> +			tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_TOUCH);
>> +			break;
>> +		case TOUCH_UPDATE:
>> +			tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_MOTION);
>> +			break;
>> +		case TOUCH_END:
>> +			tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_RELEASE);
>> +			break;
>> +		}
>> +	}
>> +}
>> +
>> +int 
>> +tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t time)
>> +{
>> +	struct libinput_device *device = &tp->device->base;
>> +	struct tp_touch *t;
>> +	enum libinput_pointer_axis axis;
>> +	double dx, dy, *delta;
>> +
>> +	tp_for_each_touch(tp, t) {
>> +		if (!t->dirty)
>> +			continue;
>> +
>> +		switch (t->scroll.edge) {
>> +			case EDGE_NONE:
>> +				if (t->scroll.last_axis != -1) {
>> +					/* Send stop scroll event */
>> +					pointer_notify_axis(device, time,
>> +						t->scroll.last_axis, 0.0);
>> +					t->scroll.last_axis = -1;
>> +				}
>> +				continue;
> 
> this needs a big comment, I read over it a couple of times :)
> but I think it's be better to initialize *delta to NULL and then have a 
> if (delta == NULL) continue; check after the switch. much less suprising.
> 
> ack to the rest, but the series doesn't apply here anymore. just push the
> next one to your fdo repo and I can merge/c-p it from there. thanks.

Ok, I'll respin with the few minor fixes you've mentioned and drop you
a private mail that the patches are sitting in my tree, as soon as I know how
you want to deal with getting the edge width / height on a per vendor basis
(see my questions above).

Regards,

Hans


More information about the wayland-devel mailing list