Hi,
On 04-03-15 22:46, Peter Hutterer wrote:
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 <[email protected]>
---
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.
Ah thanks for the explanation. Xorg sometimes has too many levels of
indirection making things confusing...
Given the above this patch is:
Reviewed-by: Hans de Goede <[email protected]>
Regards,
Hans
p.s.
We should probably create an updated Fedora package for this, and push it as
an F-22 update, making it close:
https://bugzilla.redhat.com/show_bug.cgi?id=1198467
I can take care of that if you want me to. Please let me know either way.
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)
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
