[libinput] Add support for clickfinger and button areas click method
ClosedPublic

Authored by atulbi on Dec 14 2018, 2:19 PM.

Details

Summary

Added support for libinput clickfinger , areas method for touchpad KCM
Also added mock methods.

Included commits in branch libinputClickMethod:

Added libinput mock methods
Added : 1. setScrollMethod 2. added key value pair in QMap 3. added methods for Q_Props
Added Q_Props for click method, signal
Added variables and initialize in constructor

Diff Detail

Repository
R108 KWin
Branch
libinputClickMethod
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6017
Build 6035: arc lint + arc unit
atulbi created this revision.Dec 14 2018, 2:19 PM
Restricted Application added a project: KWin. · View Herald TranscriptDec 14 2018, 2:19 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
atulbi requested review of this revision.Dec 14 2018, 2:19 PM
atulbi edited the summary of this revision. (Show Details)Dec 14 2018, 2:24 PM
atulbi added a reviewer: ngraham.
zzag edited reviewers, added: KWin; removed: ngraham.Dec 14 2018, 3:03 PM

Good start. Thanks.

libinput/device.cpp
364

Current we switch modes, but there's also a:

LIBINPUT_CONFIG_CLICK_METHOD_NONE

Is there a need for the KCM to ever set libinput to this?


If so as a general rule exposing a 3 state enum states as 2 bools can lead to things becoming more complicated. Especially if we have async connections to the config.

366
libinput/device.h
405

The default and current methods are a single enum value not flags.

i.e we can just compare here.

graesslin requested changes to this revision.Dec 14 2018, 4:22 PM
graesslin added a subscriber: graesslin.

Please also extend the unit test to verify the new functionality.

This revision now requires changes to proceed.Dec 14 2018, 4:22 PM
atulbi added a comment.EditedDec 15 2018, 3:34 PM

Yes Next I'm going to implement unit test once this get finalized

libinput/device.cpp
364

If the user device already has physical buttons

We need not to change anything
default setting will be applicable and no ui will be exposed .

else if it doesn't has physical buttons

then user needs to select either areas or clickfinger mode

So i don't think we ever need to set method as LIBINPUT_CONFIG_CLICK_METHOD_NONE

atulbi updated this revision to Diff 47642.EditedDec 15 2018, 10:51 PM

Added unit test

Changes made :

  • Replace & with == when comparing two enums
  • Improve whitespaces
  • Added unit test
atulbi marked an inline comment as done.Dec 15 2018, 10:53 PM

Recently I extended our test to verify dbus properties, see ba0cf19286bcbe731786e33bc96f62830e4ebcfc. Would be cool, if your test could also include these tests.

atulbi updated this revision to Diff 47929.Dec 21 2018, 7:02 AM
  • Added dbus test
atulbi marked an inline comment as done.Dec 21 2018, 7:05 AM
atulbi updated this revision to Diff 47959.Dec 21 2018, 2:15 PM
  • Added dbus test for default values
atulbi updated this revision to Diff 47960.Dec 21 2018, 2:29 PM
This comment was removed by atulbi.
atulbi updated this revision to Diff 47961.Dec 21 2018, 2:42 PM
  • added test for defaults
graesslin accepted this revision.Dec 21 2018, 4:49 PM

Do you have commit rights?

This revision is now accepted and ready to land.Dec 21 2018, 4:49 PM

No , I don't have commit rights

zzag added a subscriber: zzag.Dec 21 2018, 6:54 PM
zzag added inline comments.
autotests/libinput/device_test.cpp
1734 ↗(On Diff #47961)

Please use static_cast instead of c-style casts.

autotests/libinput/mock_libinput.cpp
297

missing whitespace before "!="

zzag retitled this revision from Support for libinput Clickfinger and areas method to [libinput] Add support for clickfinger and button area click method.Dec 21 2018, 7:01 PM
zzag retitled this revision from [libinput] Add support for clickfinger and button area click method to [libinput] Add support for clickfinger and button areas click method.Dec 21 2018, 7:15 PM
atulbi updated this revision to Diff 47979.Dec 21 2018, 7:27 PM
  • Changed C-style cast to static cast
  • improve whitespace
atulbi marked 2 inline comments as done.Dec 21 2018, 7:30 PM
zzag accepted this revision.Dec 21 2018, 8:29 PM

Thanks. :-)

The code generates warnings:

/home/martin/src/kde/workspace/kwin/libinput/device.h:571:16: warning: ‘KWin::LibInput::Device::m_defaultCalibrationMatrix’ will be initialized after [-Wreorder]
     QMatrix4x4 m_defaultCalibrationMatrix;
                ^~~~~~~~~~~~~~~~~~~~~~~~~~
/home/martin/src/kde/workspace/kwin/libinput/device.h:562:13: warning:   ‘quint32 KWin::LibInput::Device::m_supportedClickMethods’ [-Wreorder]
     quint32 m_supportedClickMethods;
             ^~~~~~~~~~~~~~~~~~~~~~~
/home/martin/src/kde/workspace/kwin/libinput/device.cpp:154:1: warning:   when initialized here [-Wreorder]
 Device::Device(libinput_device *device, QObject *parent)
atulbi updated this revision to Diff 48011.Dec 22 2018, 4:10 PM
  • fixed warning
This revision was automatically updated to reflect the committed changes.