Fix mouse wheel scrolling with libinput
ClosedPublic

Authored by ahmadsamir on Nov 26 2017, 8:34 PM.

Details

Summary

BUG: 386762

If the Libinput X server input driver is used we get a value for
pixelDelta for a physical mouse wheel scroll, so we check that the
source of the wheel event is actually a mouse, this should workaround
the bug until it's fixed upstream: https://bugreports.qt.io/browse/QTBUG-59261

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahmadsamir created this revision.Nov 26 2017, 8:34 PM
Restricted Application added a project: Konsole. · View Herald TranscriptNov 26 2017, 8:34 PM
ahmadsamir requested review of this revision.Nov 26 2017, 8:34 PM

Since this will eventually be fixed, can you conditionalize the workaround like we did in D8387?

To my knowledge, Scrollviews in Kirigami and Kickoff are also affected in the same way. Would you be able to consider preparing similar patches for them, too?

Since this will eventually be fixed, can you conditionalize the workaround like we did in D8387?

To make the fix conditional I think we'd better wait until it's actually fixed upstream, so that we have the actual Qt version with the fix. And hindenburg hasn't accepted the patch yet anyway :)

But if the code path that uses pixelDelta in ScrollState in konsole is supposed to handle high resolution touchpads/trackpads/... etc then checking that the event source is not a mouse can just stay in, even after the bug is fixed upstream in Qt. (I am not sure, but is that code supposed to handle high resolution scroll events coming from say an Apple Magic mouse?).

To my knowledge, Scrollviews in Kirigami and Kickoff are also affected in the same way. Would you be able to consider preparing similar patches for them, too?

I'll look into that, but no promises though.

I'm trying to determine how to test this - what distro/version are you using that has the newest libinput? My ubuntu/KDE system doesn't appear to have it.

Simple testing w/ my current setup and patch doesn't appear to cause any issues.

Ubuntu 17.10 comes with Libinput by default. On older versions, do this:

sudo apt install xserver-xorg-input-libinput
sudo apt remove xserver-xorg-input-evdev xserver-xorg-input-mouse xserver-xorg-input-synaptics
[reboot]

I have xorg-x11-drv-libinput-0.26.0, but I think the issue probably appeared with much older versions; as ngraham said, you need to make sure your X11 is actually using the libinput X11 driver.

OK I installed a 17.10 VM and the fix works. When this is fixed in Qt, this will need removed? What happens if Qt is fixed and this patch is used?

OK I installed a 17.10 VM and the fix works. When this is fixed in Qt, this will need removed? What happens if Qt is fixed and this patch is used?

Looking at the proposed fix upstream[1], this change can be reverted because Qt won't generate pixelDelta for a mouse wheel.

I am assuming that the pixelDelta code path is there to handle high-resolution touchpads/trackpads, so what this patch does is basically tighten the condition, i.e. the code will only use the pixelDelta path if the source of the event is not a mouse, so with Qt fixed this patch should have no side effects (hopefully :)).

[1]https://codereview.qt-project.org/#/c/212398/

ahmadsamir updated this revision to Diff 26444.Feb 3 2018, 4:11 PM

This is fixed upstream in Qt 5.9.5, so I changed the patch to make the fix/workaround conditional for QT_VERSION < 5.9.5.

Since we've established that the fix works, and it's now properly conditionalized, can we land it?

Yea let me look at it again before committing it - I can't seem to reproduce ATM

This revision was not accepted when it landed; it landed in state Needs Review.Feb 7 2018, 3:40 PM
This revision was automatically updated to reflect the committed changes.