On Thu, Jul 30, 2015 at 04:45:04PM +0200, Hans de Goede wrote: > Hi, > > On 30-07-15 14:33, Hans de Goede wrote: > >Hi, > > > >On 30-07-15 08:11, Peter Hutterer wrote: > >>The first finger is accurate, it's just the second finger that is imprecise, > >>so we can't handle it as a true touch. Instead, revert the device back to > >>being a single-touch touchpad and use the fake touch bits for second finger > >>handling. > >> > >>Two-finger scrolling thus becomes usable though we will lose out on > >>other features like thumb detection. Useful scrolling trumps that though. > > > >I think we should wait with doing this until the scroll situation is more > >clear, see my last comment here: > > > >https://bugzilla.redhat.com/show_bug.cgi?id=1235175 > > Ok so Benjamin responded in the bug report, these touchpads > report accurate single touch data in the old-style single-touch > events, and inaccurate data in the mt events when 2 fingers are down. > > So this patch which switches things over to using st coordinates > is the right thing todo. Review inline. > > > >>Signed-off-by: Peter Hutterer <[email protected]> > >>--- > >> src/evdev-mt-touchpad.c | 19 ++++++++++++++----- > >> test/touchpad-tap.c | 3 +++ > >> test/touchpad.c | 9 +++------ > >> 3 files changed, 20 insertions(+), 11 deletions(-) > >> > >>diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c > >>index af1cd47..54ea819 100644 > >>--- a/src/evdev-mt-touchpad.c > >>+++ b/src/evdev-mt-touchpad.c > >>@@ -1465,6 +1465,19 @@ tp_init_slots(struct tp_dispatch *tp, > >> > >> tp->semi_mt = libevdev_has_property(device->evdev, > >> INPUT_PROP_SEMI_MT); > >> > >>+ /* This device has a terrible 2fg resolution, but only for the > >>+ * second finger, causing scroll jumps when we use the touch points > >>+ * properly. The first finger resolution is accurate though, so > >>+ * we simply pretend it's a single touch touchpad with the BTN_TOOL > >>+ * bits. > >>+ */ > > This comment is wrong, the old-style st coordinates report accurate > data where as the mt data reports inaccurate data for both fingers > when 2 fingers are down. What is happening is that when a single finger > is down the st data is copied over to the mt data slot 0, so as to > have something to report to user-space which only listens to mt events, > and as soon as a second finger comes down the driver starts reporting > the bounding box limits in the mt data, making *both* slots inaccurate. > > So it is not "only for the second finger" nor is "The first finger > resolution is accurate" really accurate to say :) The first finger as > reported in the st events is accurate, the first finger reported in the > mt events is still no good, otherwise we would not need this commit at > all.
ok, I've amended the comment to: + /* This device has a terrible resolution when two fingers are down, + * causing scroll jumps. The single-touch emulation ABS_X/Y is + * accurate but the ABS_MT_POSITION touchpoints report the bounding + * box and that causes jumps. So we simply pretend it's a single + * touch touchpad with the BTN_TOOL bits. + * See https://bugzilla.redhat.com/show_bug.cgi?id=1235175 for an + * explanation. I hope that's close enough now, let me know if you want me to amend this further. Worst case, the bugzilla link has the more detailed comments :) Cheers, Peter > > The rest of the patch looks good and is: > > Reviewed-by: Hans de Goede <[email protected]> > > Regards, > > Hans > > > >>+ if (tp->semi_mt && > >>+ (device->model_flags & EVDEV_MODEL_JUMPING_SEMI_MT)) { > >>+ tp->num_slots = 1; > >>+ tp->slot = 0; > >>+ tp->has_mt = false; > >>+ } > >>+ > >> ARRAY_FOR_EACH(max_touches, m) { > >> if (libevdev_has_event_code(device->evdev, > >> EV_KEY, > >>@@ -1526,11 +1539,7 @@ tp_scroll_get_methods(struct tp_dispatch *tp) > >> { > >> uint32_t methods = LIBINPUT_CONFIG_SCROLL_EDGE; > >> > >>- /* some Synaptics semi-mt touchpads have a terrible 2fg resolution, > >>- * causing scroll jumps. For all other 2fg touchpads, we enable 2fg > >>- * scrolling */ > >>- if (tp->ntouches >= 2 && > >>- (tp->device->model_flags & EVDEV_MODEL_JUMPING_SEMI_MT) == 0) > >>+ if (tp->ntouches >= 2) > >> methods |= LIBINPUT_CONFIG_SCROLL_2FG; > >> > >> return methods; > >>diff --git a/test/touchpad-tap.c b/test/touchpad-tap.c > >>index 00afcdb..62c7a5c 100644 > >>--- a/test/touchpad-tap.c > >>+++ b/test/touchpad-tap.c > >>@@ -241,6 +241,9 @@ START_TEST(touchpad_1fg_multitap_n_drag_2fg) > >> int range = _i, > >> ntaps; > >> > >>+ if (litest_is_synaptics_semi_mt(dev)) > >>+ return; > >>+ > >> litest_enable_tap(dev->libinput_device); > >> > >> litest_drain_events(li); > >>diff --git a/test/touchpad.c b/test/touchpad.c > >>index a6989e7..77c1d2d 100644 > >>--- a/test/touchpad.c > >>+++ b/test/touchpad.c > >>@@ -402,14 +402,12 @@ START_TEST(touchpad_scroll_defaults) > >> > >> method = libinput_device_config_scroll_get_methods(device); > >> ck_assert(method & LIBINPUT_CONFIG_SCROLL_EDGE); > >>- if (libevdev_get_num_slots(evdev) > 1 && > >>- !litest_is_synaptics_semi_mt(dev)) > >>+ if (libevdev_get_num_slots(evdev) > 1) > >> ck_assert(method & LIBINPUT_CONFIG_SCROLL_2FG); > >> else > >> ck_assert((method & LIBINPUT_CONFIG_SCROLL_2FG) == 0); > >> > >>- if (libevdev_get_num_slots(evdev) > 1 && > >>- !litest_is_synaptics_semi_mt(dev)) > >>+ if (libevdev_get_num_slots(evdev) > 1) > >> expected = LIBINPUT_CONFIG_SCROLL_2FG; > >> else > >> expected = LIBINPUT_CONFIG_SCROLL_EDGE; > >>@@ -425,8 +423,7 @@ START_TEST(touchpad_scroll_defaults) > >> status = libinput_device_config_scroll_set_method(device, > >> LIBINPUT_CONFIG_SCROLL_2FG); > >> > >>- if (libevdev_get_num_slots(evdev) > 1 && > >>- !litest_is_synaptics_semi_mt(dev)) > >>+ if (libevdev_get_num_slots(evdev) > 1) > >> ck_assert_int_eq(status, LIBINPUT_CONFIG_STATUS_SUCCESS); > >> else > >> ck_assert_int_eq(status, LIBINPUT_CONFIG_STATUS_UNSUPPORTED); > >> _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
