Skip to content

Wip/libinput support#131

Merged
clefebvre merged 14 commits into
linuxmint:masterfrom
whot:wip/libinput-support
Sep 26, 2016
Merged

Wip/libinput support#131
clefebvre merged 14 commits into
linuxmint:masterfrom
whot:wip/libinput-support

Conversation

@whot

@whot whot commented Sep 13, 2016

Copy link
Copy Markdown
Contributor

version 2 of the pull request.

Fixes #78

Add libinput support. Done cooperatively with other drivers, so most of the bits toggle the bit that exists on the device. This should in theory even support two touchpads, one with synaptics and one with libinput, and just DTRT.

The keys don't all map into libinput config options, so I took a best guess where needed.

I wouldn't mind if someone could do a bit more testing though, happy to merge/squash any patches.

whot added 14 commits September 13, 2016 14:59
libinput doesn't use device types, what is considered a touchpad is up to the
caller. We consider anything with tapping a touchpad.
Easier to read and removes the current bug where some checks for a property
would create it, negating the check to begin with.
Move it up too so we can re-use it from elsewhere and don't have to move it
later. And while we're moving this code anyway, fix the indentation to match
the rest of the code.

No functional changes.
Use one with the extra device_is_touchpad check, one for generic boolean
property
Note that libinput does not allow for both edge and twofinger scrolling to be
enabled simultaneously. We prefer twofinger scrolling. The same goes for
horizontal scrolling, it picks the setting for whatever scroll method we
applied.
libinput does not allow for configuration of the click actions, the options
are just "software buttons" or "clickfinger". If any clickfinger actions are
set, we enable clickfinger, otherwise software buttons.
We ignore the "sensitivity" setting in libinput, we only have an accel setting
here
Excluding touchpad devices since the old code only applied to evdev devices.
@whot

whot commented Sep 13, 2016

Copy link
Copy Markdown
Contributor Author

For testing: xinput watch-props "" and then toggle the various UI elements. If the property on the driver changes in the correct manner, then the bits work correctly. Doesn't work for all options though, e.g. disable-while-typing, left-handed and accel settings aren't properties on the legacy drivers

@clefebvre

Copy link
Copy Markdown
Member

First of all, many thanks for this. I really like the way you supported both synaptics and libinput here.

I tested as much as I could and I couldn't find regressions.

In Linux Mint, libinput takes over from synaptics for all touchpad devices as soon as it is installed.

When libinput isn't installed, all options in Cinnamon work properly and affect the synaptics driver. This is still true after the changes brought by this PR.

When libinput is installed, the touchpad continues to work but the options in Cinnamon no longer work and the touchpad is no longer configurable. This was due to the lack of libinput support which this PR addresses. With the changes brought by this PR, most options work. Those which don't are probably failing due to limitations inherent to the libinput driver itself.

This is therefore ready to merge. I hope we'll be able to improve the mouse module in cinnamon settings so that it can show/hide relevant configuration options based on the driver being used.

@monsta

monsta commented Dec 22, 2016

Copy link
Copy Markdown
Contributor

@whot: I started adapting this to mate-settings-daemon, and there are some things I have some questions about... can you please clarify these things?

  • 8551023: setting tap to click for libinput doesn't take "left handed" setting into account... is that ok?

  • c35f952: well, this is related to previous question... setting "left handed" property on a Synaptics touchpad also includes calling set_tap_to_click for it to swap left and right buttons if necessary, but it's not done for libinput.

  • c35f952 and 1e7d504: here property_exists_on_device is used instead of property_from_name to check if a device is managed by libinput - wouldn't using property_from_name be enough?

@whot

whot commented Dec 22, 2016

Copy link
Copy Markdown
Contributor Author

sure.

  • in libinput tapping is handled differently so you don't have to worry about left-handed, there is no crazy button mapping needed like in synaptics, libinput will handle this for you, same applies to your second comment
  • properties names are global and device independent, once initialised by any device it exists until the end of the session. so if you have (had) one device with libinput, property_from_name always returns a valid property, even if there is no device currently present that actually uses that property. that's why you need to check for the property on the device itself

@monsta

monsta commented Dec 23, 2016

Copy link
Copy Markdown
Contributor

Ok, got it. But then... shouldn't property_exists_on_device be used in other functions instead of property_from_name as well?

@monsta

monsta commented Dec 23, 2016

Copy link
Copy Markdown
Contributor

Side note: I've noticed that property_exists_on_device doesn't close the device in the end...

@whot

whot commented Dec 30, 2016

Copy link
Copy Markdown
Contributor Author

oops, yeah, the missing xdevice_close() is a bug that needs to be fixed.

A quick check of the property_from_name() usage: yes, ideally they'll all use property_exists_on_device() everywhere including in the old code. right now, I think the only issues that could arise is when you have evdev + libinput simultaneously active or synaptics + libinput. But because changing properties is always "get property -> change property" the "get" part will fail and we bail, so no harm done. So right now, it's a cosmetic issue.

monsta added a commit that referenced this pull request Jan 2, 2017
@monsta

monsta commented Jan 2, 2017

Copy link
Copy Markdown
Contributor

Ok, thanks.
I've submitted a fix for device closing in #147.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants