Refactor kcm_input to enable having multiple backends.
ClosedPublic

Authored by xuetianweng on Oct 25 2017, 4:16 AM.

Details

Summary

Split patch for D8168. This change only refactor the X11-related code into
its own places. KCM itself will only use the backend interface.

Test Plan

Manually test all options.

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
xuetianweng created this revision.Oct 25 2017, 4:16 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 25 2017, 4:16 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

A kapplymousetheme binary is for sure only needed in the context of X. I would therefore not write it by the tools of the backend plugin system, but leave it as it is and put it in the X11 backend directory (and put its cmake code into the X11 cmake file). That will reduce the necessary linking for the binary.

kcms/input/mouse.cpp
82–83

Break into two lines.

kcms/input/mousebackend.cpp
33

Until there is a full backend plugin for Wayland the X backend needs to be used on Wayland as well for Xwayland stuff (see the extern "C" method).

Update accroding to subdiff's comment.

Also, I doubt if our X11 backend work on Xwayland. AFAIK Xwayland will only
get a virtual cursor. Thus I just move the condition to check the name of qpa
platform for now.

romangg added inline comments.Oct 29 2017, 9:06 PM
kcms/input/backends/x11/x11mousebackend.cpp
27

This include is not needed.

43

These 3 are never used.

51

Never used.

140

Below you do it with 256 for reasons explained there.

169

You set m_numButtons here and in load. This creates some ambiguity. Shouldn't be it enough to set it in the load() function?

171

nitpick: Spaces

207

remap only used here -> Don't use local variable. Just check the condition here.

208

Spaces

216

Capture "this" pointer and settings reference directly.

kcms/input/main.cpp
79–80

I know it's copy paste. But so we can directly clean it up: Coding style -> always use braces.

kcms/input/mousebackend.h
27

Put this enum class in a separate file, such that you don't have to include the full mousebackend.h in other header files like mouse.h

kcms/input/mousesettings.cpp
97

coding style

kcms/input/mousesettings.h
28

class -> struct

And remove the public keywords.

37

-> handedNeedsApply

like the others.

47

-> currentAccelProfile

Also, I doubt if our X11 backend work on Xwayland. AFAIK Xwayland will only
get a virtual cursor. Thus I just move the condition to check the name of qpa
platform for now.

Can you explain this more? And why is checking the qpa platform name now different to the previous check? But I think you're right that the backend is not used in the Wayland session. The workaround in KWin reads in the X mouse acceleration directly from the config file.

xuetianweng updated this revision to Diff 21674.Nov 1 2017, 3:06 AM
xuetianweng marked 11 inline comments as done.

update based on comments.

kcms/input/mousebackend.h
27

I guess I can put it in mousesettings.h ..

kcms/input/mousesettings.h
47

Actually this should be removed for now since I haven't add code to use this prop.

xuetianweng updated this revision to Diff 21675.Nov 1 2017, 3:27 AM
xuetianweng marked 8 inline comments as done.

Remove backend check so xwayland settings may be changed.

In D8460#161613, @subdiff wrote:

Also, I doubt if our X11 backend work on Xwayland. AFAIK Xwayland will only
get a virtual cursor. Thus I just move the condition to check the name of qpa
platform for now.

Can you explain this more? And why is checking the qpa platform name now different to the previous check? But I think you're right that the backend is not used in the Wayland session. The workaround in KWin reads in the X mouse acceleration directly from the config file.

I tested under weston Xwayland's cursor and key mapping can be configured. So it does make some sense to use X11 backend on all window system for now. Remove the check.

xuetianweng abandoned this revision.Nov 3 2017, 4:04 PM

To be split into small patches.

xuetianweng reclaimed this revision.Nov 3 2017, 4:05 PM

.. Sorry click on the wrong one.

ngraham accepted this revision.Nov 8 2017, 2:14 PM

Works great for me. Nice work, I can't wait to see this whole thing finished and get proper libinput support.

This revision is now accepted and ready to land.Nov 8 2017, 2:14 PM
This revision was automatically updated to reflect the committed changes.