[PATCH xf86-input-libinput] Up the scroll dist value for touchpads

[Peter Hutterer]

On Wed, Mar 04, 2015 at 01:15:31PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 04-03-15 06:00, Peter Hutterer wrote:
> >For source FINGER and CONTINUOUS, the axis value is the same as relative
> >motion - but scrolling in X usually doesn't have the same speed as finger
> >movement, it's a lot coarser.
> >
> >We don't know ahead of time where we'll get the scroll events from. Set a
> >default scroll distance of 15 and multiply any wheel clicks we get by this
> >value.
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >15 is more-or-less a magic value, it feels just right on my box here
> 
> Hmm, this is somewhat different from what we discussed in our mail conversation.

indeed, I had that first, then compared it to the evdev driver and decided
that this one is the better approach after all, explanation below.

> First of all the problem most users are reporting and I'm seeing myself is not
> that mouse wheel scrolling is too slow (which this patch seems to imply), but
> rather that finger scrolling is much too fast, see e.g.:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1198467
> 
> What I understood from our discussion on this is that the idea for
> mouse wheel scrolling was to emulate discrete-value number of button
> 4 / 5 clicks and let X do the translation into scrolling axis, giving
> us the exact same scroll wheel speed as the evdev driver.
> 
> That and for finger scroll events actually make things slower by applying
> a multiplication factor < 1.0 .

X has two ways for a driver to submit scroll events: buttons 4-7 or data in
a scroll valuator. Because clients may rely on either of those methods
exclusively, the server emulates the other method.

If set up, a driver defines a scroll distance. A valuator movement of that
scroll distance is equivalent to one mouse wheel click, and vice versa.
So if the driver sends a button 5 click, the server emulates a 
<distance> motion event. If the driver sends a <distance> motion event, the
server emulates a button 4 click. The distance accumulates in the server, so
if you send <distance/2> twice, the server will only emulate the click event
on the second event.

This is what the scroll distance does here - a movement of 15 on the
touchpad is now equivalent to a mouse wheel click. So this patch does slow
down the touchpad scrolling while leaving the mouse wheel as-is.

This is a better approach (and a smaller diff) than the button click
approach I suggested initially because it gives us some smooth scrolling on
the wheel as well. A REL_WHEEL value of 2 will now result in a single smooth
scroll event that can be used for speed calculation. Posting the button
events directly would prevent that.

Ideally we could just leave the scroll distance at 1 for devices that only
have mouse wheels but we don't know this at device init time. Hence the
default distance optimized for touchpads, then we multiply by that distance
for wheel clicks. The actual value of the scroll distance is meaningless,
it's just a reference to know how many "scroll units" a motion event
represents. IIRC synaptics currently uses a default of 100 and that's in
device coordinates.

So in short, this patch does what we want, it slows down touchpad scrolling
while leaving wheel scrolling as-is.

Cheers,
   Peter


> >
> >  src/libinput.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> >
> >diff --git a/src/libinput.c b/src/libinput.c
> >index 049c15b..5e616c8 100644
> >--- a/src/libinput.c
> >+++ b/src/libinput.c
> >@@ -756,18 +756,22 @@ xf86libinput_handle_axis(InputInfoPtr pInfo, struct libinput_event_pointer *even
> >
> >  	axis = LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL;
> >  	if (libinput_event_pointer_has_axis(event, axis)) {
> >-		if (source == LIBINPUT_POINTER_AXIS_SOURCE_WHEEL)
> >+		if (source == LIBINPUT_POINTER_AXIS_SOURCE_WHEEL) {
> >  			value = libinput_event_pointer_get_axis_value_discrete(event, axis);
> >-		else
> >+			value *=  driver_data->scroll_vdist;
> >+		} else {
> >  			value = libinput_event_pointer_get_axis_value(event, axis);
> >+		}
> >  		valuator_mask_set_double(mask, 3, value);
> >  	}
> >  	axis = LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL;
> >  	if (libinput_event_pointer_has_axis(event, axis)) {
> >-		if (source == LIBINPUT_POINTER_AXIS_SOURCE_WHEEL)
> >+		if (source == LIBINPUT_POINTER_AXIS_SOURCE_WHEEL) {
> >  			value = libinput_event_pointer_get_axis_value_discrete(event, axis);
> >-		else
> >+			value *=  driver_data->scroll_hdist;
> >+		} else {
> >  			value = libinput_event_pointer_get_axis_value(event, axis);
> >+		}
> >  		valuator_mask_set_double(mask, 2, value);
> >  	}
> >
> >@@ -1189,8 +1193,8 @@ xf86libinput_pre_init(InputDriverPtr drv,
> >  	if (!driver_data->valuators)
> >  		goto fail;
> >
> >-	driver_data->scroll_vdist = 1;
> >-	driver_data->scroll_hdist = 1;
> >+	driver_data->scroll_vdist = 15;
> >+	driver_data->scroll_hdist = 15;
> >
> >  	path = xf86SetStrOption(pInfo->options, "Device", NULL);
> >  	if (!path)
> >