Re: [PATCH 2/2] touchpad: Add edge-scrolling support
Hi,
On 11/20/2014 07:19 AM, Peter Hutterer wrote:
On Wed, Nov 19, 2014 at 11:50:13AM +0100, Hans de Goede wrote:
[...]
+
+ 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:
the unibody macbooks already had a clickpad, so this isn't needed
+ edge_width = width * .085;
+ edge_height = height * .085;
+ break;
+ default:
+ edge_width = width * .04;
+ edge_height = height * .054;
+ }
fwiw, there is a bit of history there and many oddities are based on a
the edges in synaptics being 'wrong'. the kernel exports the synaptics
dimensions as the coordinates produced by 'an average finger', but it's not
hard to go beyond min/max. iirc for alps and other devices the edges are the
actual edges (and for the T440-style synaptics pads that we manually fixed
up). Hence the need for different edge zones on synaptics.
And the default numbers come from the synaptics user interface guide.
Typical bezel limits: x 1472–5472 y 1408–4448
Typical edge margins: x 1632–5312 y 1568–4288
i.e. 4% and 5.4% are interpolated from those
so in short, we don't need MODEL_SYNAPTICS, it's covered by the defaults.
that just leaves APPLETOUCH and ALPS, I wonder if these have resolutions
set (the synaptics code pre-dates resolution support in the kernel). if so,
just defining a sensible size for the edge is enough (10mm?). or based on
the diagonal.
As also mentioned in a private mail not all alps devices have resolution set,
so at least for some devices we will need to take a percentage of width / height
(no idea why you mention diagonal here).
pls see comments in the other email
static int
@@ -1096,6 +1133,9 @@ tp_init(struct tp_dispatch *tp,
if (tp_init_scroll(tp) != 0)
return -1;
+ if (tp_edge_scroll_init(tp, device) != 0)
+ return -1;
tp_scroll_init() - tp_edge_scroll_init()
+
device-seat_caps |= EVDEV_DEVICE_POINTER;
return 0;
@@ -1239,6 +1279,7 @@ evdev_mt_touchpad_create(struct evdev_device *device)
tp-model = tp_get_model(device);
+ device-dispatch = tp-base;
I guess this is needed for some check or another? if so, can we either pass
in the data that's needed, or alternatively make it consistent that the
create method sets device-dispatch for the fallback interface as well.
This is needed because the initial value for tp-scroll.mode is set
by calling tp_scroll_config_scroll_mode_get_default_mode(device).
yeah, I think we need to wrap this differently. The current approach works,
but it's a bit of a cardhouse. specifically, in a failure case the order of
events is:
evdev_mt_touchpad_create() sets device-dispatch but returns NULL
evdev_configure_device() takes the NULL and overwrites device-dispatch
evdev_configure_device() calls evdev_device_destroy() which frees
device-dispatch
so again, it works right now but if we ever rearrange
evdev_configure_device() we'll be dealing with a freed pointer here. and the
failure case isn't easy to test for.
simple way to split this is to have an inline that takes the tp_dispatch and
is called from get_default_mode() and tp_init_touch().
Ok, fixed as suggested for V3.
Regards,
Hans
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/2] touchpad: Add edge-scrolling support
Hi,
On 11/11/2014 02:20 AM, Peter Hutterer wrote:
sorry for the duplicate Hans, wayland-devel got dropped of the first reply.
On Fri, Nov 07, 2014 at 02:25:06PM +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.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
src/Makefile.am | 1 +
src/evdev-mt-touchpad-edge-scroll.c | 366
src/evdev-mt-touchpad.c | 59 +-
src/evdev-mt-touchpad.h | 46 +
4 files changed, 463 insertions(+), 9 deletions(-)
create mode 100644 src/evdev-mt-touchpad-edge-scroll.c
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 000..2968db4
--- /dev/null
+++ b/src/evdev-mt-touchpad-edge-scroll.c
@@ -0,0 +1,366 @@
+/*
+ * 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
+
+enum scroll_event {
+SCROLL_EVENT_TOUCH,
+SCROLL_EVENT_MOTION,
+SCROLL_EVENT_RELEASE,
+SCROLL_EVENT_TIMEOUT,
+};
+
+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;
+
+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)
fwiw, I prefer all args on separate lines if you need to split them anway.
Ok, will fix for v2.
+{
+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:
+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;
+}
+}
can we make a pretty diagram for this statemachine? makes it a lot easier to
visualize.
Done for v2.
+
Re: [PATCH 2/2] touchpad: Add edge-scrolling support
On Wed, Nov 19, 2014 at 11:50:13AM +0100, Hans de Goede wrote:
[...]
+
+ 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:
the unibody macbooks already had a clickpad, so this isn't needed
+ edge_width = width * .085;
+ edge_height = height * .085;
+ break;
+ default:
+ edge_width = width * .04;
+ edge_height = height * .054;
+ }
fwiw, there is a bit of history there and many oddities are based on a
the edges in synaptics being 'wrong'. the kernel exports the synaptics
dimensions as the coordinates produced by 'an average finger', but it's not
hard to go beyond min/max. iirc for alps and other devices the edges are the
actual edges (and for the T440-style synaptics pads that we manually fixed
up). Hence the need for different edge zones on synaptics.
And the default numbers come from the synaptics user interface guide.
Typical bezel limits: x 1472–5472 y 1408–4448
Typical edge margins: x 1632–5312 y 1568–4288
i.e. 4% and 5.4% are interpolated from those
so in short, we don't need MODEL_SYNAPTICS, it's covered by the defaults.
that just leaves APPLETOUCH and ALPS, I wonder if these have resolutions
set (the synaptics code pre-dates resolution support in the kernel). if so,
just defining a sensible size for the edge is enough (10mm?). or based on
the diagonal.
As also mentioned in a private mail not all alps devices have resolution set,
so at least for some devices we will need to take a percentage of width /
height
(no idea why you mention diagonal here).
pls see comments in the other email
static int
@@ -1096,6 +1133,9 @@ tp_init(struct tp_dispatch *tp,
if (tp_init_scroll(tp) != 0)
return -1;
+ if (tp_edge_scroll_init(tp, device) != 0)
+ return -1;
tp_scroll_init() - tp_edge_scroll_init()
+
device-seat_caps |= EVDEV_DEVICE_POINTER;
return 0;
@@ -1239,6 +1279,7 @@ evdev_mt_touchpad_create(struct evdev_device *device)
tp-model = tp_get_model(device);
+ device-dispatch = tp-base;
I guess this is needed for some check or another? if so, can we either pass
in the data that's needed, or alternatively make it consistent that the
create method sets device-dispatch for the fallback interface as well.
This is needed because the initial value for tp-scroll.mode is set
by calling tp_scroll_config_scroll_mode_get_default_mode(device).
yeah, I think we need to wrap this differently. The current approach works,
but it's a bit of a cardhouse. specifically, in a failure case the order of
events is:
evdev_mt_touchpad_create() sets device-dispatch but returns NULL
evdev_configure_device() takes the NULL and overwrites device-dispatch
evdev_configure_device() calls evdev_device_destroy() which frees
device-dispatch
so again, it works right now but if we ever rearrange
evdev_configure_device() we'll be dealing with a freed pointer here. and the
failure case isn't easy to test for.
simple way to split this is to have an inline that takes the tp_dispatch and
is called from get_default_mode() and tp_init_touch().
Cheers,
Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/2] touchpad: Add edge-scrolling support
sorry for the duplicate Hans, wayland-devel got dropped of the first reply.
On Fri, Nov 07, 2014 at 02:25:06PM +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.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
src/Makefile.am | 1 +
src/evdev-mt-touchpad-edge-scroll.c | 366
src/evdev-mt-touchpad.c | 59 +-
src/evdev-mt-touchpad.h | 46 +
4 files changed, 463 insertions(+), 9 deletions(-)
create mode 100644 src/evdev-mt-touchpad-edge-scroll.c
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 000..2968db4
--- /dev/null
+++ b/src/evdev-mt-touchpad-edge-scroll.c
@@ -0,0 +1,366 @@
+/*
+ * 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
+
+enum scroll_event {
+ SCROLL_EVENT_TOUCH,
+ SCROLL_EVENT_MOTION,
+ SCROLL_EVENT_RELEASE,
+ SCROLL_EVENT_TIMEOUT,
+};
+
+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;
+
+ 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)
fwiw, I prefer all args on separate lines if you need to split them anway.
+{
+ 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:
+ 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;
+ }
+}
can we make a pretty diagram for this statemachine? makes it a lot easier to
visualize.
+
+static void
+tp_edge_scroll_handle_none(struct
[PATCH 2/2] touchpad: Add edge-scrolling support
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.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
src/Makefile.am | 1 +
src/evdev-mt-touchpad-edge-scroll.c | 366
src/evdev-mt-touchpad.c | 59 +-
src/evdev-mt-touchpad.h | 46 +
4 files changed, 463 insertions(+), 9 deletions(-)
create mode 100644 src/evdev-mt-touchpad-edge-scroll.c
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 000..2968db4
--- /dev/null
+++ b/src/evdev-mt-touchpad-edge-scroll.c
@@ -0,0 +1,366 @@
+/*
+ * 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
+
+enum scroll_event {
+ SCROLL_EVENT_TOUCH,
+ SCROLL_EVENT_MOTION,
+ SCROLL_EVENT_RELEASE,
+ SCROLL_EVENT_TIMEOUT,
+};
+
+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;
+
+ 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:
+ 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:
+ t-scroll.edge = tp_touch_get_edge(tp, t);
+ if (t-scroll.edge) {
+ tp_edge_scroll_set_state(tp, t,
+
