[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