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

Hans de Goede hdegoede at redhat.com
Mon Sep 22 00:22:02 PDT 2014


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.

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.

> other than that, Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
> 
> Cheers,
>    Peter

Regards,

Hans


> 
>> +}
>> +
>> +static void
>>  tp_device_added(struct evdev_device *device,
>>  		struct evdev_device *added_device)
>>  {
>> @@ -677,6 +719,9 @@ tp_device_added(struct evdev_device *device,
>>  		/* Don't send any pending releases to the new trackpoint */
>>  		tp->buttons.active_is_top = 0;
>>  		tp->buttons.trackpoint = added_device;
>> +		libinput_device_add_eventlistener(&added_device->base,
>> +					&tp->sendevents.trackpoint_listener,
>> +					tp_trackpoint_event, tp);
>>  	}
>>  
>>  	if (tp->sendevents.current_mode !=
>> @@ -700,6 +745,8 @@ tp_device_removed(struct evdev_device *device,
>>  			tp->buttons.active = 0;
>>  			tp->buttons.active_is_top = 0;
>>  		}
>> +		libinput_device_remove_eventlistener(
>> +					&tp->sendevents.trackpoint_listener);
>>  		tp->buttons.trackpoint = NULL;
>>  	}
>>  
>> @@ -886,6 +933,16 @@ tp_init_palmdetect(struct tp_dispatch *tp,
>>  }
>>  
>>  static int
>> +tp_init_sendevents(struct tp_dispatch *tp,
>> +		   struct evdev_device *device)
>> +{
>> +	libinput_timer_init(&tp->sendevents.trackpoint_timer,
>> +			    tp->device->base.seat->libinput,
>> +			    tp_trackpoint_timeout, tp);
>> +	return 0;
>> +}
>> +
>> +static int
>>  tp_init(struct tp_dispatch *tp,
>>  	struct evdev_device *device)
>>  {
>> @@ -921,6 +978,9 @@ tp_init(struct tp_dispatch *tp,
>>  	if (tp_init_palmdetect(tp, device) != 0)
>>  		return -1;
>>  
>> +	if (tp_init_sendevents(tp, device) != 0)
>> +		return -1;
>> +
>>  	device->seat_caps |= EVDEV_DEVICE_POINTER;
>>  
>>  	return 0;
>> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
>> index e0c8c47..5224a5c 100644
>> --- a/src/evdev-mt-touchpad.h
>> +++ b/src/evdev-mt-touchpad.h
>> @@ -220,6 +220,9 @@ struct tp_dispatch {
>>  	struct {
>>  		struct libinput_device_config_send_events config;
>>  		enum libinput_config_send_events_mode current_mode;
>> +		bool trackpoint_active;
>> +		struct libinput_event_listener trackpoint_listener;
>> +		struct libinput_timer trackpoint_timer;
>>  	} sendevents;
>>  };
>>  
>> -- 
>> 2.1.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