[Touchpad KCM] New KWin Wayland version

Authored by romangg on Dec 7 2016, 3:18 PM.



In order to work under KWin Wayland the Touchpad KCM needed to be adjusted. In theory this should have meant only the creation of an additional backend. But since a libinput based touchpad configuration utility needs to be in many ways different than the legacy X version the decision was made to redesign the user interface aswell.

Feature summary:

  • Changes the configuration of each connected touchpad individually
  • Saves changes per device (through the KWin libinput backend) in a config file.
  • Quick switch between devices via drop-down list.
  • Loads and removes devices on the fly on hotplugging events and informs the user with animated messages.
  • Visual indication if a property is not supported by a device
  • Tooltips give additional informations about available options

Technical overview:

The KCModule was redesigned as a container, which loads on opening a X or Wayland plugin dependent on the active windowing system. In the X case it loads the legacy QWidget, which was untouched besides integration work in the new plugin structure. In the Wayland case, it loads the new QML based user interface, which was written from scratch.

The user interface then instantiates appropriate backend. In the X case it loads the old backend, which was also not changed. The Wayland backend was created aswell from scratch.

KWinWaylandBackend queries available devices from KWin via D-Bus and creates KWinWaylandTouchpad instances for each available touchpad.

KWinWaylandTouchpad gets via D-Bus all properties from KWin, which are relevant for a touchpad and saves them in property structs. This includes: Boolean if the property is supported by the specific touchpad. Default and reset values for the generic KCModule actions. And the D-Bus name in order to write changes back later on

When requested by the KCModule, the backend sends all changed values back to KWin, which then forwards them to libinput and saves them in its config file. The backend also connect to KWin signals over D-Bus, which emit in case a touchpad gets disconnected or another one additionally connected.

Loads the device list from the backend and all properties of the selected touchpad. If a value gets changed in the ui, it writes directly back to the KWinWaylandTouchpad instance.

If there is no touchpad available an info message is shown and the control elements get disabled. If a touchpad is then connected, it is automatically loaded and the message vanishes. A similar message is shown when disconnecting the touchpad with currently opened configuration.

Requests for help / future work:
I need in particular feedback on my QML coding. I hope it's fine in general but there is most certainly stuff to be improved. Also the option and tooltip texts need someone else to take a look on.

The touchpad in my laptop doesn't support "Scroll-on-Button-Down". Since I would've needed to test the functionality somehow while coding, I didn't implement it yet and only did some ground work.

The hot plug handling seems a bit overengineered for touchpads, which are normally permanently installed. But most functionality of this KCM in general could be transferred to a new Wayland based Mouse KCM, which also could need an overhaul. Maybe this could be a project for a just starting contributer, since it would involve different areas of Plasma/KWin while there is already a similar example with the Touchpad KCM.

This change implements T4460.

Test Plan

Manually tested with a single touchpad in my laptop. In order to test the hot plugging I changed the reply value in KWinWaylandBackend to also gather other pointer devices. This way I could test it with multiple mice over USB.

Diff Detail

R119 Plasma Desktop
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
romangg updated this revision to Diff 8848.Dec 7 2016, 3:18 PM
romangg retitled this revision from to [Touchpad KCM] New KWin Wayland version.
romangg updated this object.
romangg edited the test plan for this revision. (Show Details)
romangg added reviewers: KWin, Plasma on Wayland, Plasma.
romangg set the repository for this revision to R119 Plasma Desktop.
romangg added a project: Plasma.
romangg added a subscriber: plasma-devel.
Restricted Application edited projects, added Plasma on Wayland, KWin; removed Plasma. · View Herald TranscriptDec 7 2016, 3:18 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg added a reviewer: VDG.Dec 7 2016, 3:21 PM
romangg updated this object.Dec 7 2016, 3:26 PM

A query about the dbus calls in findTouchpads().


Just wondering whether these calls are synchronous and may cause delay in startup.

graesslin added inline comments.

check qDeleteAll


in general you can delete a nullptr, so the ifcheck is not needed




We normally don't use the name "KWin" in user facing messages. Also I don't expect users to know what a "KCM" is ;-)

Suggestion: "Querying input devices failed. Please reopen this settings module".


please don't use the Q_FOREACH in new code as Qt might deprecate it.




I doubt a restart would help here. That should run in an error again.

return std::all_of(m_devices.constBegin(), m_devices.constEnd(),
    [] (QObject *t) { return static_cast<KWinWaylandTouchpad*>(t)->applyConfig(); });

can also be simplified with algorithm


and here as well.

Given that you have three times the same method with just one smallish difference I would even suggest to use a std::function and use one method to rule them all








why is m_devices a list of QObjects and not a list of KWinWaylandTouchpad?

Also: please don't use QList any more in new code, but if possible QVector.

romangg updated this revision to Diff 9359.Dec 25 2016, 9:33 PM
romangg marked 12 inline comments as done.
Restricted Application edited projects, added Plasma; removed KWin, Plasma on Wayland. · View Herald TranscriptDec 25 2016, 9:33 PM
romangg added inline comments.Dec 25 2016, 9:35 PM

Just wondering whether these calls are synchronous and may cause delay in startup.

I assume so. Should be no problem though. We need the reply values anyway.


Are you sure? The foreach keyword is still listed in the official Qt docu. What's the best alternative? A normal for-loop? With upcounting integer or iterator?


getDevices() is a implementation independent virtual method of TouchpadBackend and therefore KWinWaylandBackend also needs to return a container of QObjects in its getDevices() function. Implicit conversion for the whole container doesn't work and I don't wanted to iterate over the container just to return QObjects, or is there another trick?

The reason for QList is, that we later in TouchpadConfigLibinput() set it as a model context property of the QML context, which only accepts QList. But I've done it anyway now as QVector and convert it there back to a QList with toList().

luebking added inline comments.

"might deprecate it" - magic eightball wisdom ;-)

The reason is that there's now

graesslin added inline comments.Dec 26 2016, 8:07 AM
romangg updated this revision to Diff 9389.Dec 27 2016, 3:45 PM
romangg marked 4 inline comments as done.

Replace foreach loops.

Restricted Application edited projects, added Plasma on Wayland, KWin; removed Plasma. · View Herald TranscriptDec 27 2016, 3:45 PM
sebas added a subscriber: sebas.Dec 27 2016, 4:28 PM
sebas added inline comments.

remove commented out bits?


indenting four more spaces aligns it with the above

Yes, OCD. :)


remove whitespace between ! and tp


Bit of a nitpick, but the (C) doesn't really add anything. I usually leave it out.


I wonder if it wouldn't be neater if the features would be an enum, and you'd have methods to check for supported and enabled features. Have you thought of this?


const & for the argument?


whitespace looks weird here ... perhaps group the properties, the new properties and the signal handlers in blocks?


Could also be hidden by using a onRunningChanged handler, that way the stop() method would 'just work'


magic values, a comment would be useful where the 4.5 and 5.5 come from


is "value changed twice" a useful condition to set initalized here?


same here, comment please


not so sure about that, it would introduce inconsistent special behaviour just in this KCM, as a slider always should react to wheel events.


full stop is missing at the end


This will fall over with high dpi displays...


problematic for high dpi

romangg updated this revision to Diff 9407.Dec 28 2016, 2:45 AM
romangg marked 12 inline comments as done.
Restricted Application edited projects, added Plasma; removed KWin, Plasma on Wayland. · View Herald TranscriptDec 28 2016, 2:45 AM
romangg added inline comments.Dec 28 2016, 2:47 AM

Haven't thought about it. The structure here is analogous to the structure in KWin's libinput counter part.


Whenever the running property is changed to false, it automatically should hide the tooltip? But what happens, when it changes to false after a normal enter or position change event and the timer triggered after its interval? Then the tooltip would get hidden again directly.


Sorry, I don't understand. What does "value changed twice" mean?


Yea, I'll remove it. I really don't like this though since you can change by accident the value, when you have the mouse cursor by chance in vertical alignment above or below the slider and then you begin scrolling with the mouse wheel and the slider moves under the cursor.

Happened to me quite often, when I was testing different parts of the ui.

Clarified your questions.

My remaining comments are all non-critical so as long as you consider them, it's up to you if you change things or not. Just wanted to make sure it's not an oversight there.


default should already be empty, so you should be able to leave this line out.


Right, that could lead to glitches. Good thinking!


What I mean is that this only prevents the rest of the codepath to run after the value has at least changed once. What if someone sets this value again before everything is properly initialized?

romangg updated this revision to Diff 9495.Dec 30 2016, 3:46 PM
romangg marked 8 inline comments as done.
Restricted Application edited projects, added Plasma on Wayland, KWin; removed Plasma. · View Herald TranscriptDec 30 2016, 3:46 PM
graesslin accepted this revision.Jan 10 2017, 4:18 PM
graesslin added a reviewer: graesslin.

code looks good to me modulo the one issue. I didn't check the qml side, though and hope the Plasma Qml experts did so.


why an include of libinput.h? If you use that you also have to find libinput in CMake. Otherwise it won't compile on all systems.

This revision is now accepted and ready to land.Jan 10 2017, 4:18 PM
This revision was automatically updated to reflect the committed changes.
romangg marked an inline comment as done.