Summary: | IBM Scrollpoint mouse: libinput mouse scrolling speed insanely fast | ||
---|---|---|---|
Product: | Wayland | Reporter: | peter.ganzhorn |
Component: | libinput | Assignee: | Wayland bug list <wayland-bugs> |
Status: | RESOLVED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | medium | CC: | peter.hutterer |
Version: | unspecified | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
evemu recording while scrolling.
libinput: Add IBM/Lenovo Scrollpoint mice quirk to enable smooth scrolling. |
Description
peter.ganzhorn
2018-04-13 18:39:01 UTC
oh, goodie... looks like this is just a trackpoint slapped on a mouse and the value range is the same as that of a built-in trackpoint. This explains why the hwdb doesn't work, even at click delta of 1, the values quickly accumulate to what would be a full spin of a wheel. Not too unsurprising, given that the click delta is supposed to work for wheels. This needs a model-specific quirk, in the LIBINPUT_MODEL_* category. Tag the device with that model and then go down a custom code path to set up the wheel. The scroll source should be CONTINUOUS (see the documentation) and the data should match pointer movement - given that it's a trackstick that should already be the case so no messing with click deltas and the like. You'll have to handle the REL_WHEEL/HWHEEL events accordingly, but otherwise it should be reasonably straightforward. https://wayland.freedesktop.org/libinput/doc/latest/developers.html may help as a start. Thanks for caring about this. I had a look at the libinput source ( https://github.com/wayland-project/libinput ) but quite honestly I was not able to locate the file where model-specific quirks are implemented. I would have suspected udev/libinput-model-quirks.c but this seems to be only related to some touchpads and doesn't match what kind of changes you described. Would you be so kind and tell me where the changes you mentioned should be done? 90-libinput-model-quirks.hwdb is where the quirks are applied to each device. They're converted to flags in evdev_read_model_flags() and then checked in the various bits of the code that needs that knowledge. The device uses the 'fallback' interface, wheel handling is in fallback_process_relative(), called when the event comes in, and then in fallback_flush_wheels() to actually send the events. The latter is where most of the bits would have to go, I suspect. Thanks for the details, I think I understood how it should/can be done. I added the following: 90-libinput-model-quirks.hwdb: libinput:mouse:b0003v04B3p3100* libinput:mouse:b0003v04B3p3103* libinput:mouse:b0003v04B3p3105* libinput:mouse:b0003v04B3p3108* libinput:mouse:b0003v04B3p3109* libinput:mouse:b0003v17EFp6049* LIBINPUT_MODEL_SCROLLPOINT=1 evdev.h: added EVDEV_MODEL_SCROLLPOINT = (1 << 31), to enum evdec_device_model evdec.c: added MODEL(SCROLLPOINT), to model_map in evdev_read_model_flags() which should make me able to add a quirk to fallback_process_relative(). For quick testing I just added if (device->model_flags & EVDEV_MODEL_SCROLLPOINT) { /* PGZH */ if (e->code == REL_WHEEL || e->code == REL_HWHEEL ) { if ( e->value < 16 ) e->value=1; if ( e->value < 32 && e->value >= 16 ) e->value=2; if ( e->value < 63 && e->value >= 32 ) e->value=4; if ( e->value == 63 ) e->value=8; } } before e->code is evaluated and e->value is added to the dispatch->wheel.xy values. I assumed the value range to be in between 0 and 63 since this is what I see in evemu-record. Is this assumption correct? With the outlined changes and replacment of my libinput.so (.10.13.0 and symlinks to .so.10 and .so of course) after building and putting the 90-libinput-model-quirks.hwdb into /lib/udev/hwdb I don't notice any change. Also setting e->value=1 in any case does not change anything, so something must still be wrong. Is the assumed range of e->value wrong (and how can I determine the correct range easily if required) or what could still be wrong here? yeah, 63 sounds about alright. And evdev bits look correct too. For the fallback bits: accumulate the values as-is, but in fallback_flush_wheels() change the axis source to LIBINPUT_POINTER_AXIS_SOURCE_CONTINUOUS and the discrete value to 0. This should make the trackpoint behave like touchpad smooth scrolling. That should be all that's required here. For testing I recommend: - symlink the .hwdb file from /etc/udev/hwdb.d, run udevadm hwdb --update. - run sudo udevadm test to make sure the LIBINPUT_MODEL shows up. If not, the issue is in the hwdb file somewhere, once it shows up you're ready. - build libinput and run sudo ./builddir/libinput-debug-events for testing. No need to install and restart all the time, you can do that once the debug output looks sane. There's also libinput-debug-gui where you can see the scroll motion in realtime, directly from libinput. - Once you see the effects, then you can sudo ninja install, just make sure your prefix and libdir is correct. It should set up the symlinks automatically (the .so symlink is only needed for development). I think I figured this out and have a working solution right now like this: if (dispatch->wheel.y != 0) { wheel_degrees.y = -1 * dispatch->wheel.y * device->scroll.wheel_click_angle.y; if (device->model_flags & EVDEV_MODEL_SCROLLPOINT) { discrete.y = 0; source = LIBINPUT_POINTER_AXIS_SOURCE_CONTINUOUS; } else { discrete.y = -1 * dispatch->wheel.y; source = device->scroll.is_tilt.vertical ? LIBINPUT_POINTER_AXIS_SOURCE_WHEEL_TILT: LIBINPUT_POINTER_AXIS_SOURCE_WHEEL; } evdev_notify_axis( device, time, AS_MASK(LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL), source, &wheel_degrees, &discrete); dispatch->wheel.y = 0; } I implemented the same for the x-axis as well. There is one issue left where I'd like to get your feedback: libinput-debug-events reports 15° wheel movements as smallest value possible and easily goes up to around 500° without increasing force on the scrolling device a lot. Actually hitting only 15° is almost impossible. This makes scrolling still way too fast, but with the changes I made to libinput and adding the following to /lib/udev/hwdb.d/70-mouse.hwdb I have usable results: mouse:usb:v04b3p3100:* mouse:usb:v04b3p3103:* mouse:usb:v04b3p3105:* mouse:usb:v04b3p3108:* mouse:usb:v04b3p3109:* mouse:usb:v14efp6049:* MOUSE_WHEEL_CLICK_ANGLE=2 MOUSE_WHEEL_CLICK_ANGLE_HORIZONTAL=2 This file is not part of the libinput sources, so where could I add this to be set by default? Without this part the changes I made to libinput don't really improve usability of the scrollpoint. Without the changes I made to libinput so far these options do not seem to have any noticeable effect. As soon as I know where to put this, I will create a patch against libinput to add support for the scrollpoint mice for review. oki, that diff looks good. but after digging around a bit more I think we need a few extra bits here. Luckily they're already in place. Basically, we want evdev_post_scroll() to handle the scroll events because it handles things like direction locking. Much more convenient and you don't get stray horizontal scrolls. That function takes unaccelerated coordinates (normalized), see fallback_flush_relative_motion() for how to get them. Basically call normalize_delta() on them and done. The overall goal is that you don't need the CLICK_ANGLE properties because they don't make sense on this device, you don't have a click angle because you don't have a wheel that clicks. If you need those it indicates that your EVDEV_MODEL_SCROLLPOINT flag isn't taking and you're still falling back to the old click method. A few printfs should verify that. But as long as the click angle properties affect your device it means there's a bug in the new code because it should be completely separate :) 70-mouse.hwdb is part of systemd btw but again, we don't need that change. Ok, this sounds reasonable to me. 1. I reverted all changes to /lib/udev/hwdb.d/70-mouse.hwdb. 2. I reverted all previous changes in fallback_flush_wheels() and inserted the following: if (device->model_flags & EVDEV_MODEL_SCROLLPOINT) { // Scrollpoint: Normalize scroll coords in wheel degrees and use evdev_post_scroll(). dispatch->wheel.y *= -1; normalize_delta(device, &dispatch->wheel, &wheel_degrees); evdev_post_scroll(device, time, LIBINPUT_POINTER_AXIS_SOURCE_CONTINUOUS, &wheel_degrees); dispatch->wheel.x = 0; dispatch->wheel.y = 0; return; } Probably I should add a struct normalized_coords unaccel; instead of abusing wheel_degrees which was already declared in fallback_flush_wheels() for a final version of the changes? This leads to the following output of libinput-debug-events while using the scrollpoint: event13 POINTER_AXIS +1.34s vert 10.00* horiz 0.00 (continuous) event13 POINTER_AXIS +1.40s vert 10.00* horiz 0.00 (continuous) event13 POINTER_AXIS +1.45s vert 9.00* horiz 0.00 (continuous) event13 POINTER_AXIS +1.50s vert 22.00* horiz 0.00 (continuous) event13 POINTER_AXIS +1.55s vert 17.00* horiz 0.00 (continuous) event13 POINTER_AXIS +1.60s vert 16.00* horiz 0.00 (continuous) event13 POINTER_AXIS +2.02s vert -8.00* horiz 0.00 (continuous) event13 POINTER_AXIS +2.06s vert -17.00* horiz 0.00 (continuous) event13 POINTER_AXIS +2.11s vert -18.00* horiz 0.00 (continuous) event13 POINTER_AXIS +2.17s vert -23.00* horiz 0.00 (continuous) event13 POINTER_AXIS +2.22s vert -25.00* horiz 0.00 (continuous) event13 POINTER_AXIS +2.27s vert -22.00* horiz 0.00 (continuous) event13 POINTER_AXIS +2.32s vert -17.00* horiz 0.00 (continuous) event13 POINTER_AXIS +2.38s vert -1.00* horiz 0.00 (continuous) event13 POINTER_AXIS +3.14s vert 0.00 horiz -5.00* (continuous) event13 POINTER_AXIS +3.19s vert 0.00 horiz -14.00* (continuous) event13 POINTER_AXIS +3.24s vert 0.00 horiz -19.00* (continuous) event13 POINTER_AXIS +3.30s vert 0.00 horiz -30.00* (continuous) event13 POINTER_AXIS +3.34s vert 0.00 horiz -25.00* (continuous) event13 POINTER_AXIS +3.40s vert 0.00 horiz -11.00* (continuous) event13 POINTER_AXIS +3.45s vert 0.00 horiz -6.00* (continuous) event13 POINTER_AXIS +3.81s vert -1.00* horiz 0.00 (continuous) event13 POINTER_AXIS +3.86s vert -3.00* horiz 0.00 (continuous) event13 POINTER_AXIS +3.91s vert -1.00* horiz 1.00* (continuous) event13 POINTER_AXIS +3.96s vert 0.00 horiz 6.00* (continuous) event13 POINTER_AXIS +4.01s vert 0.00 horiz 4.00* (continuous) event13 POINTER_AXIS +4.06s vert 0.00 horiz 6.00* (continuous) event13 POINTER_AXIS +4.11s vert -6.00* horiz 0.00 (continuous) evemu-record shows this: E: 1.375987 0002 0008 -003 # EV_REL / REL_WHEEL -3 E: 1.375987 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +456ms E: 1.431967 0002 0008 -034 # EV_REL / REL_WHEEL -34 E: 1.431967 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +56ms E: 1.479969 0002 0008 -047 # EV_REL / REL_WHEEL -47 E: 1.479969 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +48ms E: 1.535961 0002 0008 -054 # EV_REL / REL_WHEEL -54 E: 1.535961 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +56ms E: 1.583955 0002 0008 -056 # EV_REL / REL_WHEEL -56 E: 1.583955 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +48ms E: 1.639955 0002 0008 -051 # EV_REL / REL_WHEEL -51 E: 1.639955 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +56ms E: 1.687952 0002 0008 -022 # EV_REL / REL_WHEEL -22 E: 1.687952 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +48ms E: 1.943965 0002 0008 0005 # EV_REL / REL_WHEEL 5 E: 1.943965 0002 0006 0003 # EV_REL / REL_HWHEEL 3 E: 1.943965 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +256ms E: 1.991957 0002 0008 0006 # EV_REL / REL_WHEEL 6 E: 1.991957 0002 0006 0020 # EV_REL / REL_HWHEEL 20 E: 1.991957 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +48ms E: 2.047935 0002 0006 0024 # EV_REL / REL_HWHEEL 24 E: 2.047935 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +56ms E: 2.095925 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 2.095925 0002 0006 0026 # EV_REL / REL_HWHEEL 26 E: 2.095925 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +48ms E: 2.151923 0002 0006 0033 # EV_REL / REL_HWHEEL 33 E: 2.151923 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +56ms E: 2.199941 0002 0008 0001 # EV_REL / REL_WHEEL 1 E: 2.199941 0002 0006 0035 # EV_REL / REL_HWHEEL 35 E: 2.199941 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +48ms E: 2.247917 0002 0008 0007 # EV_REL / REL_WHEEL 7 E: 2.247917 0002 0006 0022 # EV_REL / REL_HWHEEL 22 E: 2.247917 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +48ms The scrolling speed is now quite acceptable. What exactly did you mean by "direction locking"? Should the horizontal axis be locked while scrolling vertically and vice versa? If yes, this is not working. I can scroll horizontally and vertically at the same time currently. Since the scrollpoint is quite sensitive I would prefer only one scrolling direction at a time - is this possible and what you meant? the direction locking is very mild, it's supposed to help with vertical scrolling to not introduce any undue horizontal movement. There's a built-in trigger that needs to be exceeded before we start sending events for the other direction. It's definitely not a hard lock - you can scroll in all directions at all times.
But your output above shows that it's (probably) working, you don't get any horiz movement in the vertical scroll even though, most likely, you have the odd horizontal event in the output.
> Probably I should add a
> struct normalized_coords unaccel;
> instead of abusing wheel_degrees which was already declared in
> fallback_flush_wheels() for a final version of the changes?
yeah, that sounds good. I'm always in favour of localised variables that make the code more obvious, the compiler optimises them away anyway.
Created attachment 139067 [details] [review] libinput: Add IBM/Lenovo Scrollpoint mice quirk to enable smooth scrolling. The attached patch adds support for continuous scrolling for the IBM/Lenovo scrollpoint mice. Please review it and let me know if it can be accepted into libinput. A few minor changes, renaming SCROLLPOINT to LENOVO_SCROLLPOINT and a style fix. Other than that - left as-is, thanks heaps for the patch! commit 4cc2b952a283983583781ad15a2bed0cdf5645aa Author: Peter Ganzhorn <> Date: Tue Apr 24 19:30:55 2018 +0200 fallback: Add IBM/Lenovo Scrollpoint mice quirk to enable smooth scrolling. https://wayland.freedesktop.org/libinput/doc/latest/reporting_bugs.html#fixed_bugs |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.