Re: [PATCH] touchpad: reset motion history when nfingers changes on semi-mt pads

2014-07-23 Thread Hans de Goede 
Hi,

On 07/23/2014 07:58 AM, Peter Hutterer wrote:
 On Tue, Jul 22, 2014 at 09:18:26AM +0200, Hans de Goede wrote:
 Hi,

 On 07/22/2014 01:34 AM, Peter Hutterer wrote:
 On Mon, Jul 21, 2014 at 03:25:47PM +0200, Hans de Goede wrote:
 On semi-mt touchpads the reported position of the first finger down may
 jump when the pad switches from st to mt mode. When this happens a large
 delta gets seen on the first finger at the same time the second fingers
 is first seen down, causing a spurious 2 finger scroll event.

 Reset the motion history when nfingers changes on semi-mt pads to avoid 
 this.

 Signed-off-by: Hans de Goede hdego...@redhat.com

 I don't seem to have any good recordings here that show the SEMI_MT jumps.
 Any chance you can get one and add a test device for this?

 Attached is a recording.

 The jump starts at line 315, there slot 0 x / y are aprox. 640 / 1300, then
 at line 326 they jump to 1400 / 1272. This jump happens because in 2 finger
 mode the touchpad no longer reports exact locations, instead we get
 a bounding box, where the fingers occupy 2 opposing corners, and in this case
 the fingers are on 2 different corners then the ones the kernel chooses to
 always report.

 I guess the kernel could / should be made smarter here, and could decide 
 which
 2 corners to pick based on the last single touch coordinate, instead of 
 always
 picking the upper left and bottom right corners. I'll take a look at that,
 resetting the motion history is still the right thing to do though, since
 the 2 finger coordinates basically use a different coordinate system which
 gets scaled to the single touch coordinate system.
 
 if the kernel is no option (as you said in the other email), can we add a
 semi-mt specific touch filtering system so that we fix the touchpoints
 _before_ we get to the main touchpad processing code? That way we don't have
 to care at all about semi-mt once the touchpoints are fixed. It won't be
 perfect, but it's better than having to worry about semi-mt everywhere.

For the record, Peter and I discussed this on the phone, and we decided
that just leaving things as is + restting motion history when the number of
fingers changes is the best way forward with this.

  
 Adding a test device for this based on the recording should be easy.

 ACK to the patch, but this is really something I'd like to see a specific
 test case for so we know what we're fixing here. plus, adding a semi-mt
 device to the test suite means we can figure out if everything else works
 fine with it.

 I already expected you would want a test-case, the problem is that currently
 in litest we cannot send a motion event with it automatically getting a sync
 added at the end of it, making it impossible to reproduce the behavior from
 the recording in litest. I've been thinking a bit about this and I think
 that the best way to deal with this is a suppress_syn flag which will 
 basically
 suppress all syn sending when set, then special cases like this one can
 just set that flag, queue up events, clear the flag and do an explicit syn.
 
 How about an API that takes multiple touch down events and
 strips the SYN_REPORTS out automatically. So e.g.
 
 litest_multi_touch_down(0,  10, 10,
 1,  50, 50,
 -1);
 
 and the matching move/up calls. That won't cater for this particular case
 though, I think for this use-case you really need to just litest_write the
 specific event sequence. Or, even better, overwrite touch_down in the
 litest-device interface to mangle the touchpoints per slot to replicate the
 kernel behaviour, which would make the device applicable for all tests then.

We discussed this too, Peter has some plans to create a special test device
which accurately emulates semi-mt devices. Peter has put doing this on his
TODO list.

Regards,

Hans
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] touchpad: reset motion history when nfingers changes on semi-mt pads

2014-07-22 Thread Hans de Goede 
Hi,

On 07/22/2014 01:34 AM, Peter Hutterer wrote:
 On Mon, Jul 21, 2014 at 03:25:47PM +0200, Hans de Goede wrote:
 On semi-mt touchpads the reported position of the first finger down may
 jump when the pad switches from st to mt mode. When this happens a large
 delta gets seen on the first finger at the same time the second fingers
 is first seen down, causing a spurious 2 finger scroll event.

 Reset the motion history when nfingers changes on semi-mt pads to avoid this.

 Signed-off-by: Hans de Goede hdego...@redhat.com
 
 I don't seem to have any good recordings here that show the SEMI_MT jumps.
 Any chance you can get one and add a test device for this?

Attached is a recording.

The jump starts at line 315, there slot 0 x / y are aprox. 640 / 1300, then
at line 326 they jump to 1400 / 1272. This jump happens because in 2 finger
mode the touchpad no longer reports exact locations, instead we get
a bounding box, where the fingers occupy 2 opposing corners, and in this case
the fingers are on 2 different corners then the ones the kernel chooses to
always report.

I guess the kernel could / should be made smarter here, and could decide which
2 corners to pick based on the last single touch coordinate, instead of always
picking the upper left and bottom right corners. I'll take a look at that,
resetting the motion history is still the right thing to do though, since
the 2 finger coordinates basically use a different coordinate system which
gets scaled to the single touch coordinate system.

Adding a test device for this based on the recording should be easy.

 ACK to the patch, but this is really something I'd like to see a specific
 test case for so we know what we're fixing here. plus, adding a semi-mt
 device to the test suite means we can figure out if everything else works
 fine with it.

I already expected you would want a test-case, the problem is that currently
in litest we cannot send a motion event with it automatically getting a sync
added at the end of it, making it impossible to reproduce the behavior from
the recording in litest. I've been thinking a bit about this and I think
that the best way to deal with this is a suppress_syn flag which will basically
suppress all syn sending when set, then special cases like this one can
just set that flag, queue up events, clear the flag and do an explicit syn.

Regards,

Hans


 
 Cheers,
Peter
 
 ---
  src/evdev-mt-touchpad.c | 8 
  src/evdev-mt-touchpad.h | 2 ++
  2 files changed, 10 insertions(+)

 diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
 index b0520c7..315c531 100644
 --- a/src/evdev-mt-touchpad.c
 +++ b/src/evdev-mt-touchpad.c
 @@ -406,6 +406,11 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
  
  for (i = 0; i  tp-ntouches; i++) {
  t = tp_get_touch(tp, i);
 +
 +/* semi-mt finger postions may jump when nfingers changes */
 +if (tp-semi_mt  tp-nfingers_down != tp-old_nfingers_down)
 +tp_motion_history_reset(t);
 +
  if (i = tp-real_touches  t-state != TOUCH_NONE) {
  t-x = first-x;
  t-y = first-y;
 @@ -454,6 +459,7 @@ tp_post_process_state(struct tp_dispatch *tp, uint64_t 
 time)
  t-dirty = false;
  }
  
 +tp-old_nfingers_down = tp-nfingers_down;
  tp-buttons.old_state = tp-buttons.state;
  
  tp-queued = TOUCHPAD_EVENT_NONE;
 @@ -668,6 +674,8 @@ tp_init_slots(struct tp_dispatch *tp,
  tp-has_mt = false;
  }
  
 +tp-semi_mt = libevdev_has_property(device-evdev, INPUT_PROP_SEMI_MT);
 +
  ARRAY_FOR_EACH(max_touches, m) {
  if (libevdev_has_event_code(device-evdev,
  EV_KEY,
 diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
 index af6a3a3..83edf4f 100644
 --- a/src/evdev-mt-touchpad.h
 +++ b/src/evdev-mt-touchpad.h
 @@ -152,8 +152,10 @@ struct tp_dispatch {
  struct evdev_dispatch base;
  struct evdev_device *device;
  unsigned int nfingers_down; /* number of fingers down */
 +unsigned int old_nfingers_down; /* previous no fingers down */
  unsigned int slot;  /* current slot */
  bool has_mt;
 +bool semi_mt;
  
  unsigned int real_touches;  /* number of slots */
  unsigned int ntouches;  /* no slots inc. fakes */
 -- 
 2.0.1

 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel

# EVEMU 1.2
# Input device name: AlpsPS/2 ALPS GlidePoint
# Input device ID: bus 0x11 vendor 0x02 product 0x08 version 0x300
# Supported events:
#   Event type 0 (EV_SYN)
# Event code 0 (SYN_REPORT)
# Event code 1 (SYN_CONFIG)
# Event code 2 (SYN_MT_REPORT)
# Event code 3 (SYN_DROPPED)
# Event code 4 ((null))
# Event code 5 ((null))
# Event code 6

Re: [PATCH] touchpad: reset motion history when nfingers changes on semi-mt pads

2014-07-22 Thread Peter Hutterer 
On Tue, Jul 22, 2014 at 09:18:26AM +0200, Hans de Goede wrote:
 Hi,
 
 On 07/22/2014 01:34 AM, Peter Hutterer wrote:
  On Mon, Jul 21, 2014 at 03:25:47PM +0200, Hans de Goede wrote:
  On semi-mt touchpads the reported position of the first finger down may
  jump when the pad switches from st to mt mode. When this happens a large
  delta gets seen on the first finger at the same time the second fingers
  is first seen down, causing a spurious 2 finger scroll event.
 
  Reset the motion history when nfingers changes on semi-mt pads to avoid 
  this.
 
  Signed-off-by: Hans de Goede hdego...@redhat.com
  
  I don't seem to have any good recordings here that show the SEMI_MT jumps.
  Any chance you can get one and add a test device for this?
 
 Attached is a recording.
 
 The jump starts at line 315, there slot 0 x / y are aprox. 640 / 1300, then
 at line 326 they jump to 1400 / 1272. This jump happens because in 2 finger
 mode the touchpad no longer reports exact locations, instead we get
 a bounding box, where the fingers occupy 2 opposing corners, and in this case
 the fingers are on 2 different corners then the ones the kernel chooses to
 always report.
 
 I guess the kernel could / should be made smarter here, and could decide which
 2 corners to pick based on the last single touch coordinate, instead of always
 picking the upper left and bottom right corners. I'll take a look at that,
 resetting the motion history is still the right thing to do though, since
 the 2 finger coordinates basically use a different coordinate system which
 gets scaled to the single touch coordinate system.

if the kernel is no option (as you said in the other email), can we add a
semi-mt specific touch filtering system so that we fix the touchpoints
_before_ we get to the main touchpad processing code? That way we don't have
to care at all about semi-mt once the touchpoints are fixed. It won't be
perfect, but it's better than having to worry about semi-mt everywhere.
 
 Adding a test device for this based on the recording should be easy.
 
  ACK to the patch, but this is really something I'd like to see a specific
  test case for so we know what we're fixing here. plus, adding a semi-mt
  device to the test suite means we can figure out if everything else works
  fine with it.
 
 I already expected you would want a test-case, the problem is that currently
 in litest we cannot send a motion event with it automatically getting a sync
 added at the end of it, making it impossible to reproduce the behavior from
 the recording in litest. I've been thinking a bit about this and I think
 that the best way to deal with this is a suppress_syn flag which will 
 basically
 suppress all syn sending when set, then special cases like this one can
 just set that flag, queue up events, clear the flag and do an explicit syn.

How about an API that takes multiple touch down events and
strips the SYN_REPORTS out automatically. So e.g.

litest_multi_touch_down(0,  10, 10,
1,  50, 50,
-1);

and the matching move/up calls. That won't cater for this particular case
though, I think for this use-case you really need to just litest_write the
specific event sequence. Or, even better, overwrite touch_down in the
litest-device interface to mangle the touchpoints per slot to replicate the
kernel behaviour, which would make the device applicable for all tests then.

Cheers,
   Peter

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH] touchpad: reset motion history when nfingers changes on semi-mt pads

2014-07-21 Thread Peter Hutterer 
On Mon, Jul 21, 2014 at 03:25:47PM +0200, Hans de Goede wrote:
 On semi-mt touchpads the reported position of the first finger down may
 jump when the pad switches from st to mt mode. When this happens a large
 delta gets seen on the first finger at the same time the second fingers
 is first seen down, causing a spurious 2 finger scroll event.
 
 Reset the motion history when nfingers changes on semi-mt pads to avoid this.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com

I don't seem to have any good recordings here that show the SEMI_MT jumps.
Any chance you can get one and add a test device for this?

ACK to the patch, but this is really something I'd like to see a specific
test case for so we know what we're fixing here. plus, adding a semi-mt
device to the test suite means we can figure out if everything else works
fine with it.

Cheers,
   Peter

 ---
  src/evdev-mt-touchpad.c | 8 
  src/evdev-mt-touchpad.h | 2 ++
  2 files changed, 10 insertions(+)
 
 diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
 index b0520c7..315c531 100644
 --- a/src/evdev-mt-touchpad.c
 +++ b/src/evdev-mt-touchpad.c
 @@ -406,6 +406,11 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
  
   for (i = 0; i  tp-ntouches; i++) {
   t = tp_get_touch(tp, i);
 +
 + /* semi-mt finger postions may jump when nfingers changes */
 + if (tp-semi_mt  tp-nfingers_down != tp-old_nfingers_down)
 + tp_motion_history_reset(t);
 +
   if (i = tp-real_touches  t-state != TOUCH_NONE) {
   t-x = first-x;
   t-y = first-y;
 @@ -454,6 +459,7 @@ tp_post_process_state(struct tp_dispatch *tp, uint64_t 
 time)
   t-dirty = false;
   }
  
 + tp-old_nfingers_down = tp-nfingers_down;
   tp-buttons.old_state = tp-buttons.state;
  
   tp-queued = TOUCHPAD_EVENT_NONE;
 @@ -668,6 +674,8 @@ tp_init_slots(struct tp_dispatch *tp,
   tp-has_mt = false;
   }
  
 + tp-semi_mt = libevdev_has_property(device-evdev, INPUT_PROP_SEMI_MT);
 +
   ARRAY_FOR_EACH(max_touches, m) {
   if (libevdev_has_event_code(device-evdev,
   EV_KEY,
 diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
 index af6a3a3..83edf4f 100644
 --- a/src/evdev-mt-touchpad.h
 +++ b/src/evdev-mt-touchpad.h
 @@ -152,8 +152,10 @@ struct tp_dispatch {
   struct evdev_dispatch base;
   struct evdev_device *device;
   unsigned int nfingers_down; /* number of fingers down */
 + unsigned int old_nfingers_down; /* previous no fingers down */
   unsigned int slot;  /* current slot */
   bool has_mt;
 + bool semi_mt;
  
   unsigned int real_touches;  /* number of slots */
   unsigned int ntouches;  /* no slots inc. fakes */
 -- 
 2.0.1
 
 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel