Wayland: Allow to set a scroll Factor for input devices
ClosedPublic

Authored by meven on Mar 26 2020, 3:33 PM.

Details

Summary

Mouse and touchpad wheel events are concerned.

CCBUG: 403843

KCM patch: D28331

Test Plan

build, ctest, manual set of scrollfactor value via qdbusviewer

Diff Detail

Repository
R108 KWin
Branch
scrollFactor
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24285
Build 24303: arc lint + arc unit
meven created this revision.Mar 26 2020, 3:33 PM
Restricted Application added a project: KWin. · View Herald TranscriptMar 26 2020, 3:33 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
meven requested review of this revision.Mar 26 2020, 3:33 PM
meven added a comment.EditedMar 26 2020, 3:49 PM

We follow libinput maintainer opinion https://gitlab.freedesktop.org/libinput/libinput/issues/185 that the platform (or the toolkit) should implement this.
(sway compositor implements this https://github.com/swaywm/sway/pull/3018/files#diff-40846134db734aabaf9eb9e98bccab5eL1005)

It is an open issue for gnome https://gitlab.gnome.org/GNOME/gtk/issues/1308

We might want to let some apps the real "untouched" events, we could allow a "X-KWin-No-ScrollFactor" in their service file for instance.

apol accepted this revision.Mar 26 2020, 3:55 PM
apol added a subscriber: apol.

Makes sense, thanks!

This revision is now accepted and ready to land.Mar 26 2020, 3:55 PM
davidedmundson requested changes to this revision.Mar 26 2020, 3:55 PM
davidedmundson added inline comments.
libinput/events.cpp
217

Why not this one?

This revision now requires changes to proceed.Mar 26 2020, 3:55 PM
meven updated this revision to Diff 78576.Mar 26 2020, 4:49 PM

Apply scroll factor to axisValue, properly load setting on kwin start

meven marked an inline comment as done.Mar 26 2020, 4:50 PM
meven edited the summary of this revision. (Show Details)Mar 26 2020, 4:53 PM
meven added a comment.Mar 27 2020, 8:32 AM

Should I add a "supportScrollFactor" bool to make it easier of the UI side ?

Should I add a "supportScrollFactor" bool to make it easier of the UI side ?

By UI side you mean the KCM?

meven added a comment.Mar 27 2020, 9:32 AM

Should I add a "supportScrollFactor" bool to make it easier of the UI side ?

By UI side you mean the KCM?

Yes, but nevermind, those KCM Uis are already split X/Wayland so I don't need it for this use case actually.

meven edited the summary of this revision. (Show Details)Mar 27 2020, 9:34 AM

Seems fine to me it's analogous to a touchpad input matrix, which are also done server side.

I believe Vlad had a concern, I do want to hear what that is before approving.

meven edited the summary of this revision. (Show Details)Mar 27 2020, 9:37 AM
zzag added a comment.Mar 27 2020, 9:59 AM

If sway folks do a similar thing, then it's fine with me.

davidedmundson accepted this revision.Mar 27 2020, 10:00 AM
This revision is now accepted and ready to land.Mar 27 2020, 10:00 AM
ervin requested changes to this revision.Mar 27 2020, 5:39 PM
ervin added inline comments.
libinput/device.cpp
315

There is type erasure and implicit conversion involved so I guess it ends up working properly in practice, still I'd advise using "1.0" here which would be of the right type. Just a question of making intent obvious.

libinput/device.h
335

s/ScrollFactorDefault/scrollFactorDefault/

This revision now requires changes to proceed.Mar 27 2020, 5:39 PM
meven updated this revision to Diff 78690.Mar 27 2020, 6:14 PM

Formatting, use a more explicit default value

ervin accepted this revision.Mar 27 2020, 9:13 PM

LGTM

This revision is now accepted and ready to land.Mar 27 2020, 9:13 PM
This revision was automatically updated to reflect the committed changes.
meven marked an inline comment as done.
clel added a subscriber: clel.Apr 30 2020, 8:46 PM

We follow libinput maintainer opinion https://gitlab.freedesktop.org/libinput/libinput/issues/185 that the platform (or the toolkit) should implement this.

(sway compositor implements this https://github.com/swaywm/sway/pull/3018/files#diff-40846134db734aabaf9eb9e98bccab5eL1005)

It is an open issue for gnome https://gitlab.gnome.org/GNOME/gtk/issues/1308

We might want to let some apps the real "untouched" events, we could allow a "X-KWin-No-ScrollFactor" in their service file for instance.

First of all, thanks for integrating this into KDE, can't wait to test when it ships with a more stable build.

Just a short note, there is also another, more specific issue for this on GNOME: https://gitlab.gnome.org/GNOME/gnome-control-center/issues/379