Hi,
First of all patches 1-9 and 11 looks good to me and are:
Reviewed-by: Hans de Goede <hdego...@redhat.com>
I've some remarks inline on this one.
On 30-01-17 01:58, Peter Hutterer wrote:
Don't rely on BTN_TOUCH for "finger down", the value for that is hardcoded in
the kernel and not always suitable. Some devices need a different value to
avoid reacting to accidental touches or hovering fingers.
Implement a basic Schmitt trigger, same as we have in the synaptics driver. We
also take the default values from there but these will likely see some
updates.
A special case is when we have more fingers down than slots. Since we can't
detect the pressure on fake fingers (we only get a bit for 'is down') we
assume that *all* fingers are down with sufficient pressure. It's too much of
a niche case to have this work any other way.
This patch drops the handling of ABS_DISTANCE because it's simply not needed
anymore.
Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
---
src/evdev-mt-touchpad.c | 129 +++++++++++++++++++++++++++++------
src/evdev-mt-touchpad.h | 10 ++-
test/litest-device-alps-dualpoint.c | 14 ++++
test/litest-device-alps-semi-mt.c | 14 ++++
test/litest-device-atmel-hover.c | 14 ++++
test/litest-device-synaptics-hover.c | 14 ++++
test/litest-device-synaptics-st.c | 15 +++-
7 files changed, 185 insertions(+), 25 deletions(-)
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index a47c59f..ff9ec41 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -322,9 +322,6 @@ tp_process_absolute(struct tp_dispatch *tp,
case ABS_MT_SLOT:
tp->slot = e->value;
break;
- case ABS_MT_DISTANCE:
- t->distance = e->value;
- break;
case ABS_MT_TRACKING_ID:
if (e->value != -1)
tp_new_touch(tp, t, time);
@@ -365,6 +362,11 @@ tp_process_absolute_st(struct tp_dispatch *tp,
t->dirty = true;
tp->queued |= TOUCHPAD_EVENT_MOTION;
break;
+ case ABS_PRESSURE:
+ t->pressure = e->value;
+ t->dirty = true;
+ tp->queued |= TOUCHPAD_EVENT_OTHERAXIS;
+ break;
}
}
@@ -795,26 +797,72 @@ out:
}
static void
-tp_unhover_abs_distance(struct tp_dispatch *tp, uint64_t time)
+tp_unhover_pressure(struct tp_dispatch *tp, uint64_t time)
{
struct tp_touch *t;
- unsigned int i;
+ int i;
+ unsigned int nfake_touches;
+ unsigned int real_fingers_down = 0;
- for (i = 0; i < tp->ntouches; i++) {
+ nfake_touches = tp_fake_finger_count(tp);
+ if (nfake_touches == FAKE_FINGER_OVERFLOW)
+ nfake_touches = 0;
+
+ for (i = 0; i < (int)tp->num_slots; i++) {
t = tp_get_touch(tp, i);
- if (!t->dirty)
- continue;
+ if (t->dirty) {
+ if (t->state == TOUCH_HOVERING) {
+ if (t->pressure >= tp->pressure.high) {
+ /* avoid jumps when landing a finger */
+ tp_motion_history_reset(t);
+ tp_begin_touch(tp, t, time);
+ }
+ } else {
+ if (t->pressure < tp->pressure.low)
+ tp_end_touch(tp, t, time);
+ }
+ }
+
+ if (t->state == TOUCH_BEGIN ||
+ t->state == TOUCH_UPDATE)
+ real_fingers_down++;
+ }
+ if (nfake_touches <= tp->num_slots ||
+ tp->nfingers_down == 0)
+ return;
Maybe:
if (tp->nfingers_down >= nfake_touches ||
tp->nfingers_down == 0)
return;
Otherwise this:
+
+ /* if we have more fake fingers down than slots, we assume
+ * _all_ fingers have enough pressure, even if some of the slotted
+ * ones don't. Anything else gets insane quickly.
+ */
+ for (i = 0; i < (int)tp->ntouches; i++) {
+ t = tp_get_touch(tp, i);
if (t->state == TOUCH_HOVERING) {
<snip removed bit>
+ /* avoid jumps when landing a finger */
+ tp_motion_history_reset(t);
+ tp_begin_touch(tp, t, time);
+
+ if (tp->nfingers_down >= nfake_touches)
+ break;
+ }
+ }
Will always unhover one more hovering touch even if we
already have enough, I don't know if we can have more touches
in down + hovering state then nfake_touches but otherwise
the
if (tp->nfingers_down >= nfake_touches)
break;
Is not necessary.
+
+ if (tp->nfingers_down > nfake_touches ||
+ real_fingers_down == 0) {
+ for (i = tp->ntouches - 1; i >= 0; i--) {
+ t = tp_get_touch(tp, i);
+
+ if (t->state == TOUCH_HOVERING ||
+ t->state == TOUCH_NONE)
+ continue;
+
+ tp_end_touch(tp, t, time);
+
+ if (real_fingers_down > 0 &&
+ tp->nfingers_down == nfake_touches)
+ break;
}
}
}
@@ -881,8 +929,8 @@ tp_unhover_fake_touches(struct tp_dispatch *tp, uint64_t
time)
static void
tp_unhover_touches(struct tp_dispatch *tp, uint64_t time)
{
- if (tp->reports_distance)
- tp_unhover_abs_distance(tp, time);
+ if (tp->pressure.use_pressure)
+ tp_unhover_pressure(tp, time);
else
tp_unhover_fake_touches(tp, time);
@@ -926,6 +974,7 @@ tp_position_fake_touches(struct tp_dispatch *tp)
continue;
t->point = topmost->point;
+ t->pressure = topmost->pressure;
if (!t->dirty)
t->dirty = topmost->dirty;
}
@@ -1747,7 +1796,13 @@ tp_sync_touch(struct tp_dispatch *tp,
&t->point.y))
t->point.y = libevdev_get_event_value(evdev, EV_ABS, ABS_Y);
- libevdev_fetch_slot_value(evdev, slot, ABS_MT_DISTANCE, &t->distance);
+ if (!libevdev_fetch_slot_value(evdev,
+ slot,
+ ABS_MT_PRESSURE,
+ &t->pressure))
+ t->pressure = libevdev_get_event_value(evdev,
+ EV_ABS,
+ ABS_PRESSURE);
}
static inline void
@@ -2271,6 +2326,38 @@ tp_init_hysteresis(struct tp_dispatch *tp)
return;
}
+static void
+tp_init_pressure(struct tp_dispatch *tp,
+ struct evdev_device *device)
+{
+ const struct input_absinfo *abs;
+ unsigned int range;
+ unsigned int code = ABS_PRESSURE;
+
+ if (tp->has_mt)
+ code = ABS_MT_PRESSURE;
+
+ if (!libevdev_has_event_code(device->evdev, EV_ABS, code)) {
+ tp->pressure.use_pressure = false;
+ return;
+ }
+
+ tp->pressure.use_pressure = true;
+
+ abs = libevdev_get_abs_info(device->evdev, code);
+ assert(abs);
+
+ range = abs->maximum - abs->minimum;
+
+ /* Approximately the synaptics defaults */
+ tp->pressure.high = abs->minimum + 0.12 * range;
+ tp->pressure.low = abs->minimum + 0.10 * range;
+
+ log_debug(evdev_libinput_context(device),
+ "%s: using pressure-based touch detection\n",
+ device->devname);
+}
+
static int
tp_init(struct tp_dispatch *tp,
struct evdev_device *device)
@@ -2288,9 +2375,7 @@ tp_init(struct tp_dispatch *tp,
evdev_device_init_abs_range_warnings(device);
- tp->reports_distance = libevdev_has_event_code(device->evdev,
- EV_ABS,
- ABS_MT_DISTANCE);
+ tp_init_pressure(tp, device);
/* Set the dpi to that of the x axis, because that's what we normalize
to when needed*/
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index abe885f..eae0af9 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -153,7 +153,6 @@ struct tp_touch {
bool dirty;
struct device_coords point;
uint64_t millis;
- int distance; /* distance == 0 means touch */
int pressure;
bool was_down; /* if distance == 0, false for pure hovering
@@ -232,7 +231,6 @@ struct tp_dispatch {
unsigned int slot; /* current slot */
bool has_mt;
bool semi_mt;
- bool reports_distance; /* does the device support true
hovering */
/* true if we're reading events (i.e. not suspended) but we're
* ignoring them */
@@ -248,6 +246,14 @@ struct tp_dispatch {
*/
unsigned int fake_touches;
+ /* if pressure goes above high -> touch down,
+ if pressure then goes below low -> touch up */
+ struct {
+ bool use_pressure;
+ int high;
+ int low;
+ } pressure;
+
struct device_coords hysteresis_margin;
struct {
diff --git a/test/litest-device-alps-dualpoint.c
b/test/litest-device-alps-dualpoint.c
index de025e5..90fdbca 100644
--- a/test/litest-device-alps-dualpoint.c
+++ b/test/litest-device-alps-dualpoint.c
@@ -60,9 +60,23 @@ static struct input_event move[] = {
{ .type = -1, .code = -1 },
};
+static int
+get_axis_default(struct litest_device *d, unsigned int evcode, int32_t *value)
+{
+ switch (evcode) {
+ case ABS_PRESSURE:
+ case ABS_MT_PRESSURE:
+ *value = 30;
+ return 0;
+ }
+ return 1;
+}
+
static struct litest_device_interface interface = {
.touch_down_events = down,
.touch_move_events = move,
+
+ .get_axis_default = get_axis_default,
};
static struct input_id input_id = {
diff --git a/test/litest-device-alps-semi-mt.c
b/test/litest-device-alps-semi-mt.c
index d78adca..de0eb3a 100644
--- a/test/litest-device-alps-semi-mt.c
+++ b/test/litest-device-alps-semi-mt.c
@@ -60,9 +60,23 @@ static struct input_event move[] = {
{ .type = -1, .code = -1 },
};
+static int
+get_axis_default(struct litest_device *d, unsigned int evcode, int32_t *value)
+{
+ switch (evcode) {
+ case ABS_PRESSURE:
+ case ABS_MT_PRESSURE:
+ *value = 30;
+ return 0;
+ }
+ return 1;
+}
+
static struct litest_device_interface interface = {
.touch_down_events = down,
.touch_move_events = move,
+
+ .get_axis_default = get_axis_default,
};
static struct input_id input_id = {
diff --git a/test/litest-device-atmel-hover.c b/test/litest-device-atmel-hover.c
index c856d08..942744e 100644
--- a/test/litest-device-atmel-hover.c
+++ b/test/litest-device-atmel-hover.c
@@ -76,10 +76,24 @@ static struct input_event up[] = {
{ .type = -1, .code = -1 },
};
+static int
+get_axis_default(struct litest_device *d, unsigned int evcode, int32_t *value)
+{
+ switch (evcode) {
+ case ABS_PRESSURE:
+ case ABS_MT_PRESSURE:
+ *value = 30;
+ return 0;
+ }
+ return 1;
+}
+
static struct litest_device_interface interface = {
.touch_down_events = down,
.touch_move_events = move,
.touch_up_events = up,
+
+ .get_axis_default = get_axis_default,
};
static struct input_id input_id = {
diff --git a/test/litest-device-synaptics-hover.c
b/test/litest-device-synaptics-hover.c
index 8439f10..9c0c7bd 100644
--- a/test/litest-device-synaptics-hover.c
+++ b/test/litest-device-synaptics-hover.c
@@ -60,9 +60,23 @@ static struct input_event move[] = {
{ .type = -1, .code = -1 },
};
+static int
+get_axis_default(struct litest_device *d, unsigned int evcode, int32_t *value)
+{
+ switch (evcode) {
+ case ABS_PRESSURE:
+ case ABS_MT_PRESSURE:
+ *value = 30;
+ return 0;
+ }
+ return 1;
+}
+
static struct litest_device_interface interface = {
.touch_down_events = down,
.touch_move_events = move,
+
+ .get_axis_default = get_axis_default,
};
static struct input_id input_id = {
diff --git a/test/litest-device-synaptics-st.c
b/test/litest-device-synaptics-st.c
index 7e45d56..bbde1ef 100644
--- a/test/litest-device-synaptics-st.c
+++ b/test/litest-device-synaptics-st.c
@@ -36,7 +36,7 @@ litest_synaptics_touchpad_setup(void)
static struct input_event down[] = {
{ .type = EV_ABS, .code = ABS_X, .value = LITEST_AUTO_ASSIGN },
{ .type = EV_ABS, .code = ABS_Y, .value = LITEST_AUTO_ASSIGN },
- { .type = EV_ABS, .code = ABS_PRESSURE, .value = 30 },
+ { .type = EV_ABS, .code = ABS_PRESSURE, .value = LITEST_AUTO_ASSIGN },
{ .type = EV_ABS, .code = ABS_TOOL_WIDTH, .value = 7 },
{ .type = EV_SYN, .code = SYN_REPORT, .value = 0 },
{ .type = -1, .code = -1 },
@@ -54,10 +54,23 @@ struct input_event up[] = {
{ .type = -1, .code = -1 },
};
+static int
+get_axis_default(struct litest_device *d, unsigned int evcode, int32_t *value)
+{
+ switch (evcode) {
+ case ABS_PRESSURE:
+ *value = 30;
+ return 0;
+ }
+ return 1;
+}
+
static struct litest_device_interface interface = {
.touch_down_events = down,
.touch_move_events = move,
.touch_up_events = up,
+
+ .get_axis_default = get_axis_default,
};
static struct input_absinfo absinfo[] = {
Otherwise looks good.
Regards,
Hans
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
