[PATCH libinput 03/10] touchpad: hook up to the tapping configuration
Peter Hutterer
peter.hutterer at who-t.net
Tue Jun 3 15:00:17 PDT 2014
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.
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