[PATCH libinput 03/10] touchpad: hook up to the tapping configuration
Jonas Ådahl
jadahl at gmail.com
Wed Jun 4 00:51:36 PDT 2014
On Wed, Jun 04, 2014 at 08:00:17AM +1000, Peter Hutterer wrote:
> On Tue, Jun 03, 2014 at 10:42:50PM +0200, Jonas Ådahl wrote:
> > On Tue, Jun 03, 2014 at 03:34:56PM +1000, Peter Hutterer wrote:
> > > Now that we have run-time changes of the tap.enabled state move the check
> > > to the IDLE state only. Otherwise the tap machine may hang if tapping is
> > > disabled while a gesture is in progress.
> > >
> > > Two basic tests are added to check for the tap default setting - which is now
> > > "tap disabled by default", because a bike shed's correct color is green.
> >
> > Say what? I might have missed some joke reference, but why would you
> > want to disable tapping by default?
>
> dates back a couple of years where we had some amusing back/forth in the
> synaptics driver in regards to tapping enabled or disabled by default.
> long story short, we ended up disabling it by default for two reasons:
> * if you don't know that tapping is a thing (or enabled by default), you get
> spurious mouse events that make the desktop feel buggy.
> * if you do know what tapping is and you want it, you usually know where to
> enable it, or at least you can search for it.
>
> Of course, there is merry disagreement on this but I still think those two
> reasons above justify disabling tapping by default.
Fair enough. I won't start such a thread again, even though it means one
of my touchpads will not work by default (as the physical buttons are
broken, although reported existing :P).
I suppose however that certain models are designed to be used for
tapping having it enabled in the preinstalled system, but I wouldn't
know how to get that information.
Jonas
>
> Cheers,
> Peter
>
> >
> > >
> > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > ---
> > > src/evdev-mt-touchpad-tap.c | 56 +++++++++++++++++++++++++++++++++++++++------
> > > src/evdev-mt-touchpad.h | 1 +
> > > test/touchpad.c | 32 ++++++++++++++++++++++++++
> > > 3 files changed, 82 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
> > > index eee334f..24ea8c3 100644
> > > --- a/src/evdev-mt-touchpad-tap.c
> > > +++ b/src/evdev-mt-touchpad-tap.c
> > > @@ -438,13 +438,13 @@ static void
> > > tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time)
> > > {
> > > enum tp_tap_state current;
> > > - if (!tp->tap.enabled)
> > > - return;
> > >
> > > current = tp->tap.state;
> > >
> > > switch(tp->tap.state) {
> > > case TAP_STATE_IDLE:
> > > + if (!tp->tap.enabled)
> > > + break;
> > > tp_tap_idle_handle_event(tp, event, time);
> > > break;
> > > case TAP_STATE_TOUCH:
> > > @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
> > > unsigned int
> > > tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
> > > {
> > > - if (!tp->tap.enabled)
> > > - return 0;
> > > -
> > > if (tp->tap.timeout && tp->tap.timeout <= time) {
> > > tp_tap_clear_timer(tp);
> > > tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
> > > @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
> > > return tp->tap.timeout;
> > > }
> > >
> > > +static int
> > > +tp_tap_config_count(struct libinput_device *device)
> > > +{
> > > + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)->dispatch;
> > > + struct tp_dispatch *tp = container_of(dispatch, tp, base);
> > > +
> > > + return min(tp->ntouches, 3); /* we only do up to 3 finger tap */
> > > +}
> > > +
> > > +static enum libinput_config_status
> > > +tp_tap_config_enable(struct libinput_device *device, int enabled)
> > > +{
> > > + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)->dispatch;
> >
> > nit: (struct bla *) var, and 80 chars line limit (here and below, and in
> > patch 5).
> >
> > > + struct tp_dispatch *tp = container_of(dispatch, tp, base);
> > > +
> > > + if (tp_tap_config_count(device) == 0)
> > > + return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
> > > +
> > > + tp->tap.enabled = enabled;
> > > +
> > > + return LIBINPUT_CONFIG_STATUS_SUCCESS;
> > > +}
> > > +
> > > +static int
> > > +tp_tap_config_is_enabled(struct libinput_device *device)
> > > +{
> > > + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)->dispatch;
> > > + struct tp_dispatch *tp = container_of(dispatch, tp, base);
> > > +
> > > + return tp->tap.enabled;
> > > +}
> > > +
> > > +static void
> > > +tp_tap_config_reset(struct libinput_device *device)
> > > +{
> > > + struct evdev_dispatch *dispatch = ((struct evdev_device *)device)->dispatch;
> > > + struct tp_dispatch *tp = container_of(dispatch, tp, base);
> > > +
> > > + tp->tap.enabled = false;
> > > +}
> > > +
> > > int
> > > tp_init_tap(struct tp_dispatch *tp)
> > > {
> > > + tp->tap.config.count = tp_tap_config_count;
> > > + tp->tap.config.enable = tp_tap_config_enable;
> > > + tp->tap.config.is_enabled = tp_tap_config_is_enabled;
> > > + tp->tap.config.reset = tp_tap_config_reset;
> > > + tp->device->base.config.tap = &tp->tap.config;
> > > +
> > > tp->tap.state = TAP_STATE_IDLE;
> > > tp->tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
> > >
> > > @@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp)
> > > return -1;
> > > }
> > >
> > > - tp->tap.enabled = 1; /* FIXME */
> > > -
> > > return 0;
> > > }
> > >
> > > diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> > > index d514ed6..0b31e9a 100644
> > > --- a/src/evdev-mt-touchpad.h
> > > +++ b/src/evdev-mt-touchpad.h
> > > @@ -203,6 +203,7 @@ struct tp_dispatch {
> > >
> > > struct {
> > > bool enabled;
> > > + struct libinput_device_config_tap config;
> > > int timer_fd;
> > > struct libinput_source *source;
> > > unsigned int timeout;
> > > diff --git a/test/touchpad.c b/test/touchpad.c
> > > index 35781c3..060b529 100644
> > > --- a/test/touchpad.c
> > > +++ b/test/touchpad.c
> > > @@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap)
> > > struct libinput *li = dev->libinput;
> > > struct libinput_event *event;
> > >
> > > + libinput_device_config_tap_enable(dev->libinput_device, 1);
> > > +
> > > litest_drain_events(li);
> > >
> > > litest_touch_down(dev, 0, 50, 50);
> > > @@ -141,6 +143,8 @@ START_TEST(touchpad_1fg_tap_n_drag)
> > > struct libinput *li = dev->libinput;
> > > struct libinput_event *event;
> > >
> > > + libinput_device_config_tap_enable(dev->libinput_device, 1);
> > > +
> > > litest_drain_events(li);
> > >
> > > litest_touch_down(dev, 0, 50, 50);
> > > @@ -194,6 +198,8 @@ START_TEST(touchpad_2fg_tap)
> > > struct libinput *li = dev->libinput;
> > > struct libinput_event *event;
> > >
> > > + libinput_device_config_tap_enable(dev->libinput_device, 1);
> > > +
> > > litest_drain_events(dev->libinput);
> > >
> > > litest_touch_down(dev, 0, 50, 50);
> > > @@ -349,6 +355,30 @@ START_TEST(clickpad_click_n_drag)
> > > }
> > > END_TEST
> > >
> > > +START_TEST(touchpad_tap_is_available)
> > > +{
> > > + struct litest_device *dev = litest_current_device();
> > > +
> > > + ck_assert_int_ge(libinput_device_config_tap_get_finger_count(dev->libinput_device), 1);
> > > + ck_assert_int_eq(libinput_device_config_tap_is_enabled(dev->libinput_device), 0);
> > > +}
> > > +END_TEST
> > > +
> > > +START_TEST(touchpad_tap_is_not_available)
> > > +{
> > > + struct litest_device *dev = litest_current_device();
> > > +
> > > + ck_assert_int_eq(libinput_device_config_tap_get_finger_count(dev->libinput_device), 0);
> > > + ck_assert_int_eq(libinput_device_config_tap_is_enabled(dev->libinput_device), 0);
> > > + ck_assert_int_eq(libinput_device_config_tap_enable(dev->libinput_device, 1),
> > > + LIBINPUT_CONFIG_STATUS_UNSUPPORTED);
> > > +
> > > + /* doesn't do anything but tests for null-pointer derefernce */
> > > + libinput_device_config_tap_reset(dev->libinput_device);
> > > +}
> > > +END_TEST
> > > +
> > > +
> > > int main(int argc, char **argv) {
> > >
> > > litest_add("touchpad:motion", touchpad_1fg_motion, LITEST_TOUCHPAD, LITEST_ANY);
> > > @@ -357,6 +387,8 @@ int main(int argc, char **argv) {
> > > litest_add("touchpad:tap", touchpad_1fg_tap, LITEST_TOUCHPAD, LITEST_ANY);
> > > litest_add("touchpad:tap", touchpad_1fg_tap_n_drag, LITEST_TOUCHPAD, LITEST_ANY);
> > > litest_add("touchpad:tap", touchpad_2fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
> > > + litest_add("touchpad:tap", touchpad_tap_is_available, LITEST_TOUCHPAD, LITEST_ANY);
> > > + litest_add("touchpad:tap", touchpad_tap_is_not_available, LITEST_ANY, LITEST_TOUCHPAD);
> > >
> > > litest_add_no_device("touchpad:clickfinger", touchpad_1fg_clickfinger);
> > > litest_add_no_device("touchpad:clickfinger", touchpad_2fg_clickfinger);
> > > --
> > > 1.9.0
> > >
> > > _______________________________________________
> > > wayland-devel mailing list
> > > wayland-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list