Re: [PATCH libinput v3 02/17] touchpad: after a click, lock the finger to its current position
On Tue, May 20, 2014 at 04:34:50PM +0200, Hans de Goede wrote:
From: Peter Hutterer peter.hutte...@who-t.net
On clickpads, clicking the pad usually causes some motion events. To avoid
erroneous movements, lock the finger into position on the click and don't
allow for motion events until we move past a given threshold (currently 2% of
the touchpad diagonal).
HdG:
Instead of pinning the lowest touch assuming that that is the one holding
the physical button, simply pin all touches, and unpin as soon as a touch
is moved more then the threshold.
Can you merge the two commit messages together into one? if we don't have
the original patch in the tree we don't need the specific history of this
patch. if you are concerned about preserving authorship feel free to change
to yourself as author or add a Co-authored-by: tag, either you or me, I
don't mind either way. That applies to all other patches too please.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
Signed-off-by: Hans de Goede hdego...@redhat.com
Reviewed-by: Jonas Ådahl jad...@gmail.com
Reviewed-by: Hans de Goede hdego...@redhat.com
Reviewed-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad.c | 80
-
src/evdev-mt-touchpad.h | 12 +++-
2 files changed, 50 insertions(+), 42 deletions(-)
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index b4a89e3..381bb90 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -32,6 +32,7 @@
#define DEFAULT_MIN_ACCEL_FACTOR 0.16
#define DEFAULT_MAX_ACCEL_FACTOR 1.0
#define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0
+#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */
Didn't spot this but the comment should probably be changed to
/* 2% of size */
to avoid confusion about it being 2% or 0.02%.
other than that - series looks good, rev-by where missing if you want.
tested on my x220 and on the T440s and it works as expected.
Send me a pull request for the lot and I'll merge it, no need to re-send all
these patches.
Cheers,
Peter
static inline int
tp_hysteresis(int in, int center, int margin)
@@ -158,6 +159,7 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t)
tp_motion_history_reset(t);
t-dirty = true;
t-state = TOUCH_BEGIN;
+ t-pinned.is_pinned = false;
tp-nfingers_down++;
assert(tp-nfingers_down = 1);
tp-queued |= TOUCHPAD_EVENT_MOTION;
@@ -182,6 +184,7 @@ tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t)
t-dirty = true;
t-is_pointer = false;
t-state = TOUCH_END;
+ t-pinned.is_pinned = false;
assert(tp-nfingers_down = 1);
tp-nfingers_down--;
tp-queued |= TOUCHPAD_EVENT_MOTION;
@@ -346,50 +349,43 @@ tp_process_key(struct tp_dispatch *tp,
}
static void
-tp_unpin_finger(struct tp_dispatch *tp)
+tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t)
{
- struct tp_touch *t;
- tp_for_each_touch(tp, t) {
- if (t-is_pinned) {
- t-is_pinned = false;
+ unsigned int xdist, ydist;
+ struct tp_touch *tmp = NULL;
+
+ if (!t-pinned.is_pinned)
+ return;
+
+ xdist = abs(t-x - t-pinned.center_x);
+ ydist = abs(t-y - t-pinned.center_y);
- if (t-state != TOUCH_END
- tp-nfingers_down == 1)
- t-is_pointer = true;
+ if (xdist * xdist + ydist * ydist
+ tp-buttons.motion_dist * tp-buttons.motion_dist)
+ return;
+
+ t-pinned.is_pinned = false;
+
+ tp_for_each_touch(tp, tmp) {
+ if (tmp-is_pointer)
break;
- }
}
+
+ if (t-state != TOUCH_END !tmp-is_pointer)
+ t-is_pointer = true;
}
static void
-tp_pin_finger(struct tp_dispatch *tp)
+tp_pin_fingers(struct tp_dispatch *tp)
{
- struct tp_touch *t,
- *pinned = NULL;
+ struct tp_touch *t;
tp_for_each_touch(tp, t) {
- if (t-is_pinned) {
- pinned = t;
- break;
- }
- }
-
- assert(!pinned);
-
- pinned = tp_current_touch(tp);
-
- if (tp-nfingers_down != 1) {
- tp_for_each_touch(tp, t) {
- if (t == pinned)
- continue;
-
- if (t-y pinned-y)
- pinned = t;
- }
+ t-is_pointer = false;
+ t-pinned.is_pinned = true;
+ t-pinned.center_x = t-x;
+ t-pinned.center_y = t-y;
}
-
- pinned-is_pinned = true;
- pinned-is_pointer = false;
}
static void
@@ -409,16 +405,19 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time)
Re: [PATCH libinput v3 02/17] touchpad: after a click, lock the finger to its current position
Hi, On 05/22/2014 08:05 AM, Peter Hutterer wrote: On Tue, May 20, 2014 at 04:34:50PM +0200, Hans de Goede wrote: From: Peter Hutterer peter.hutte...@who-t.net On clickpads, clicking the pad usually causes some motion events. To avoid erroneous movements, lock the finger into position on the click and don't allow for motion events until we move past a given threshold (currently 2% of the touchpad diagonal). HdG: Instead of pinning the lowest touch assuming that that is the one holding the physical button, simply pin all touches, and unpin as soon as a touch is moved more then the threshold. Can you merge the two commit messages together into one? if we don't have the original patch in the tree we don't need the specific history of this patch. if you are concerned about preserving authorship feel free to change to yourself as author or add a Co-authored-by: tag, either you or me, I don't mind either way. That applies to all other patches too please. Done. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net Signed-off-by: Hans de Goede hdego...@redhat.com Reviewed-by: Jonas Ådahl jad...@gmail.com Reviewed-by: Hans de Goede hdego...@redhat.com Reviewed-by: Peter Hutterer peter.hutte...@who-t.net --- src/evdev-mt-touchpad.c | 80 - src/evdev-mt-touchpad.h | 12 +++- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index b4a89e3..381bb90 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -32,6 +32,7 @@ #define DEFAULT_MIN_ACCEL_FACTOR 0.16 #define DEFAULT_MAX_ACCEL_FACTOR 1.0 #define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0 +#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */ Didn't spot this but the comment should probably be changed to /* 2% of size */ to avoid confusion about it being 2% or 0.02%. Fixed. other than that - series looks good, rev-by where missing if you want. tested on my x220 and on the T440s and it works as expected. Send me a pull request for the lot and I'll merge it, no need to re-send all these patches. I'm going through your review comments now, I'll send a pull-req when I'm done. Regards, Hans ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput v3 02/17] touchpad: after a click, lock the finger to its current position
From: Peter Hutterer peter.hutte...@who-t.net
On clickpads, clicking the pad usually causes some motion events. To avoid
erroneous movements, lock the finger into position on the click and don't
allow for motion events until we move past a given threshold (currently 2% of
the touchpad diagonal).
HdG:
Instead of pinning the lowest touch assuming that that is the one holding
the physical button, simply pin all touches, and unpin as soon as a touch
is moved more then the threshold.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
Signed-off-by: Hans de Goede hdego...@redhat.com
Reviewed-by: Jonas Ådahl jad...@gmail.com
Reviewed-by: Hans de Goede hdego...@redhat.com
Reviewed-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad.c | 80 -
src/evdev-mt-touchpad.h | 12 +++-
2 files changed, 50 insertions(+), 42 deletions(-)
diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index b4a89e3..381bb90 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -32,6 +32,7 @@
#define DEFAULT_MIN_ACCEL_FACTOR 0.16
#define DEFAULT_MAX_ACCEL_FACTOR 1.0
#define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0
+#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* in percent of size */
static inline int
tp_hysteresis(int in, int center, int margin)
@@ -158,6 +159,7 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t)
tp_motion_history_reset(t);
t-dirty = true;
t-state = TOUCH_BEGIN;
+ t-pinned.is_pinned = false;
tp-nfingers_down++;
assert(tp-nfingers_down = 1);
tp-queued |= TOUCHPAD_EVENT_MOTION;
@@ -182,6 +184,7 @@ tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t)
t-dirty = true;
t-is_pointer = false;
t-state = TOUCH_END;
+ t-pinned.is_pinned = false;
assert(tp-nfingers_down = 1);
tp-nfingers_down--;
tp-queued |= TOUCHPAD_EVENT_MOTION;
@@ -346,50 +349,43 @@ tp_process_key(struct tp_dispatch *tp,
}
static void
-tp_unpin_finger(struct tp_dispatch *tp)
+tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t)
{
- struct tp_touch *t;
- tp_for_each_touch(tp, t) {
- if (t-is_pinned) {
- t-is_pinned = false;
+ unsigned int xdist, ydist;
+ struct tp_touch *tmp = NULL;
+
+ if (!t-pinned.is_pinned)
+ return;
+
+ xdist = abs(t-x - t-pinned.center_x);
+ ydist = abs(t-y - t-pinned.center_y);
- if (t-state != TOUCH_END
- tp-nfingers_down == 1)
- t-is_pointer = true;
+ if (xdist * xdist + ydist * ydist
+ tp-buttons.motion_dist * tp-buttons.motion_dist)
+ return;
+
+ t-pinned.is_pinned = false;
+
+ tp_for_each_touch(tp, tmp) {
+ if (tmp-is_pointer)
break;
- }
}
+
+ if (t-state != TOUCH_END !tmp-is_pointer)
+ t-is_pointer = true;
}
static void
-tp_pin_finger(struct tp_dispatch *tp)
+tp_pin_fingers(struct tp_dispatch *tp)
{
- struct tp_touch *t,
- *pinned = NULL;
+ struct tp_touch *t;
tp_for_each_touch(tp, t) {
- if (t-is_pinned) {
- pinned = t;
- break;
- }
- }
-
- assert(!pinned);
-
- pinned = tp_current_touch(tp);
-
- if (tp-nfingers_down != 1) {
- tp_for_each_touch(tp, t) {
- if (t == pinned)
- continue;
-
- if (t-y pinned-y)
- pinned = t;
- }
+ t-is_pointer = false;
+ t-pinned.is_pinned = true;
+ t-pinned.center_x = t-x;
+ t-pinned.center_y = t-y;
}
-
- pinned-is_pinned = true;
- pinned-is_pointer = false;
}
static void
@@ -409,16 +405,19 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time)
tp_motion_hysteresis(tp, t);
tp_motion_history_push(t);
+
+ tp_unpin_finger(tp, t);
}
- /* We have a physical button down event on a clickpad. For drag and
- drop, this means we try to identify which finger pressed the
- physical button and pin it, i.e. remove pointer-moving
- capabilities from it.
+ /*
+* We have a physical button down event on a clickpad. To avoid
+* spurious pointer moves by the clicking finger we pin all fingers.
+* We unpin fingers when they move more then a certain threshold to
+* to allow drag and drop.
*/
if ((tp-queued TOUCHPAD_EVENT_BUTTON_PRESS)
!tp-buttons.has_buttons)
- tp_pin_finger(tp);
+
