On Sun, Sep 28, 2014 at 01:21:06PM +0200, Hans de Goede wrote: > Before this commit the tap code deals with enabled being set to false, > by waiting for tap.state to become IDLE, and then ignoring any events from > that point on. > > This causes a problem when enabled gets set to true again while fingers are > down, because when in IDLE no release events are expected, so once a release > event for one of the fingers is send, log_bug_libinput gets called.
fwiw, the reason for the approach was that one could disable tapping at any time without any accidental consequences that can be caused by forcing a release right then and there. > This commit fixes this by making enabled use the same mechanism for enabled > state transitions as the tap suspend / resume code. > > Signed-off-by: Hans de Goede <[email protected]> > --- > src/evdev-mt-touchpad-tap.c | 39 ++++++++++++++++++++++++++++++++------- > 1 file changed, 32 insertions(+), 7 deletions(-) > > diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c > index 61c0139..b549067 100644 > --- a/src/evdev-mt-touchpad-tap.c > +++ b/src/evdev-mt-touchpad-tap.c > @@ -476,9 +476,6 @@ tp_tap_handle_event(struct tp_dispatch *tp, > > switch(tp->tap.state) { > case TAP_STATE_IDLE: > - if (!tp->tap.enabled) > - break; > - > tp_tap_idle_handle_event(tp, t, event, time); > break; > case TAP_STATE_TOUCH: > @@ -540,13 +537,19 @@ tp_tap_exceeds_motion_threshold(struct tp_dispatch *tp, > struct tp_touch *t) > return dx * dx + dy * dy > threshold * threshold; > } > > +static bool > +tp_tap_enabled(struct tp_dispatch *tp) > +{ > + return tp->tap.enabled && !tp->tap.suspended; > +} > + > int > tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time) > { > struct tp_touch *t; > int filter_motion = 0; > > - if (tp->tap.suspended) > + if (!tp_tap_enabled(tp)) > return 0; > > /* Handle queued button pressed events from clickpads. For touchpads > @@ -623,6 +626,23 @@ tp_tap_handle_timeout(uint64_t time, void *data) > } > } > > +static void > +tp_tap_enabled_updated(struct tp_dispatch *tp, bool old_enabled, uint64_t > time) this is a bit odd, both in naming and the signature. how about passing suspend and enable in from the callers and then letting this function figure out the rest. That'd be a bit more obvious I think. So the callers would be something like: tp->tap.enabled = false; tp_tap_enabled_update(tp, tp->tap.suspended, tp->tap.enabled, time); which is a lot more readable than the old_enabled dance. (btw, for the future: was_enabled is easier to grasp conceptually than old_enabled) Cheers, Peter > +{ > + bool enabled = tp_tap_enabled(tp); > + > + if (enabled == old_enabled) > + return; > + > + if (enabled) { > + /* Must restart in DEAD if fingers are down atm */ > + tp->tap.state = > + tp->nfingers_down ? TAP_STATE_DEAD : TAP_STATE_IDLE; > + } else { > + tp_release_all_taps(tp, time); > + } > +} > + > static int > tp_tap_config_count(struct libinput_device *device) > { > @@ -641,11 +661,15 @@ tp_tap_config_set_enabled(struct libinput_device > *device, > { > struct evdev_dispatch *dispatch; > struct tp_dispatch *tp = NULL; > + bool old_enabled; > > dispatch = ((struct evdev_device *) device)->dispatch; > tp = container_of(dispatch, tp, base); > > + old_enabled = tp_tap_enabled(tp); > tp->tap.enabled = (enabled == LIBINPUT_CONFIG_TAP_ENABLED); > + tp_tap_enabled_updated(tp, old_enabled, > + libinput_now(device->seat->libinput)); > > return LIBINPUT_CONFIG_STATUS_SUCCESS; > } > @@ -718,14 +742,15 @@ tp_release_all_taps(struct tp_dispatch *tp, uint64_t > now) > void > tp_tap_suspend(struct tp_dispatch *tp, uint64_t time) > { > + bool old_enabled = tp_tap_enabled(tp); > tp->tap.suspended = true; > - tp_release_all_taps(tp, time); > + tp_tap_enabled_updated(tp, old_enabled, time); > } > > void > tp_tap_resume(struct tp_dispatch *tp, uint64_t time) > { > + bool old_enabled = tp_tap_enabled(tp); > tp->tap.suspended = false; > - /* Must restart in DEAD if fingers are down atm */ > - tp->tap.state = tp->nfingers_down ? TAP_STATE_DEAD : TAP_STATE_IDLE; > + tp_tap_enabled_updated(tp, old_enabled, time); > } > -- > 2.1.0 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
