[PATCH libinput 3/3] touchpad: Disable touchpads on trackpoint activity

[Peter Hutterer]

On Mon, Sep 22, 2014 at 09:22:02AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 09/22/2014 07:18 AM, Peter Hutterer wrote:
> > On Thu, Sep 18, 2014 at 01:11:05PM +0200, Hans de Goede wrote:
> >> Some laptops with both a clickpad and a trackpoint have such a large touchpad,
> >> that parts of the users hands will touch the pad when using the trackpoint.
> >> Examples of this are the Lenovo T440s and the Toshiba Tecra Z40-A.
> >>
> >> This commit makes libinput automatically disable the touchpad while using
> >> the trackpoint on these devices, except for the buttons, as people may want
> >> to use the touchpad (hardware or soft) buttons while using the trackpoint.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> >> ---
> >>  src/evdev-mt-touchpad.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++--
> >>  src/evdev-mt-touchpad.h |  3 +++
> >>  2 files changed, 65 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> >> index 1c32cc6..31e2e83 100644
> >> --- a/src/evdev-mt-touchpad.c
> >> +++ b/src/evdev-mt-touchpad.c
> >> @@ -31,6 +31,7 @@
> >>  
> >>  #define DEFAULT_ACCEL_NUMERATOR 1200.0
> >>  #define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0
> >> +#define DEFAULT_TRACKPOINT_ACTIVITY_TIMEOUT 500 /* ms */
> >>  
> >>  static inline int
> >>  tp_hysteresis(int in, int center, int margin)
> >> @@ -523,8 +524,8 @@ tp_post_events(struct tp_dispatch *tp, uint64_t time)
> >>  	double dx, dy;
> >>  	int consumed = 0;
> >>  
> >> -	/* Only post (top) button events while suspended */
> >> -	if (tp->device->suspended) {
> >> +	/* Only post (top) button events? */
> >> +	if (tp->device->suspended || tp->sendevents.trackpoint_active) {
> >>  		tp_post_button_events(tp, time);
> >>  		return;
> >>  	}
> >> @@ -594,6 +595,16 @@ tp_process(struct evdev_dispatch *dispatch,
> >>  }
> >>  
> >>  static void
> >> +tp_destroy_sendevents(struct tp_dispatch *tp)
> >> +{
> >> +	libinput_timer_cancel(&tp->sendevents.trackpoint_timer);
> >> +
> >> +	if (tp->buttons.trackpoint)
> >> +		libinput_device_remove_eventlistener(
> >> +					&tp->sendevents.trackpoint_listener);
> >> +}
> >> +
> >> +static void
> >>  tp_destroy(struct evdev_dispatch *dispatch)
> >>  {
> >>  	struct tp_dispatch *tp =
> >> @@ -601,6 +612,7 @@ tp_destroy(struct evdev_dispatch *dispatch)
> >>  
> >>  	tp_destroy_tap(tp);
> >>  	tp_destroy_buttons(tp);
> >> +	tp_destroy_sendevents(tp);
> >>  
> >>  	filter_destroy(tp->filter);
> >>  	free(tp->touches);
> >> @@ -667,6 +679,36 @@ tp_resume(struct tp_dispatch *tp, struct evdev_device *device)
> >>  }
> >>  
> >>  static void
> >> +tp_trackpoint_timeout(uint64_t now, void *data)
> >> +{
> >> +	struct tp_dispatch *tp = data;
> >> +
> >> +	tp_clear_state(tp, tp->device);
> >> +	tp->sendevents.trackpoint_active = false;
> >> +}
> >> +
> >> +static void
> >> +tp_trackpoint_event(struct libinput_event *event, void *data)
> >> +{
> >> +	struct tp_dispatch *tp = data;
> >> +
> >> +	/* Only movement counts as trackpad activity, as people may use
> >> +	   the trackpoint buttons in combination with the touchpad. */
> >> +	if (event->type != LIBINPUT_EVENT_POINTER_MOTION)
> >> +		return;
> > 
> > the wheel emulation code posts axis events, you'll need to include those
> > here as well. 
> 
> Ah good one will fix.
> 
> > 
> >> +
> >> +	if (!tp->sendevents.trackpoint_active) {
> >> +		tp_clear_state(tp, tp->device);
> >> +		tp->sendevents.trackpoint_active = true;
> >> +	}
> >> +
> >> +	/* We don't use the event time as that may wrap */
> >> +	libinput_timer_set(&tp->sendevents.trackpoint_timer,
> >> +			   libinput_now(tp->device->base.seat->libinput) +
> >> +				DEFAULT_TRACKPOINT_ACTIVITY_TIMEOUT);
> > 
> > Can you expand on this?
> > it seems like an odd comment given that we use the event time elsewhere.
> 
> Elsewhere we use evdev_event times. Here we've a libinput_event which uses
> a 32 bit unsigned integer holding ms, which will wrap aprox. every 50 days,
> and since this is clock_monotonic, that is every 50 days since system boot,
> not since last login or some such. Since uptimes of 50 days are not out
> of the ordinary, we need to deal with this.
> 
> I've fixed this for evdev a while back by changing evdev internal timestamps
> to uint64 everywhere. Currently these get truncated to 32 bit timestamps
> the moment one of the (e.g.) pointer_notify_foo functions get called.
> 
> I guess a better fix might be to push the 64 bit timestamps down into
> libinput.c itself, doing the truncating to 32 bit when (e.g.)
> libinput_event_pointer_get_time gets called.

that seems to be the best option here, should slot in nicely enough before
the patchset without much rebasing pain.

> Note that one could argue we should make libinput_event_pointer_get_time
> return a 64 bit value (or add a 64 bit variant), but the wayland wire
> protocol only uses 32 bit time stamps, so there is not much use in that.
 
> > it also provides inconsistent behaviour, if libinput is slow to read the
> > event the timeout is set from when it gets processes as opposed to when the
> > physical interaction happened. That's something we should avoid.
> 
> Under normal circumstances I would expect libinput to get behind say
> 50 ms maximum, which is not all that much on a 500ms timeout.

Normal circumstances are not what we have to worry about :)

Cheers,
   Peter