acidanthera / bugtracker

Acidanthera Bugtracker
383 stars 44 forks source link

Future path of VoodooInput #932

Closed leo-labs closed 3 years ago

leo-labs commented 4 years ago

I am the author of VoodooSMBus, which started as an SMBus driver and an ELAN touchpad driver for touchpads that are connected via SMBus. This project is similar to VoodooI2C, just with a different bus (that has better interrupt features) as VoodooI2C does not support SMBus. It turned out that there are also some thinkpads with the combination of Synaptics touchpad + SMBus and @1Revenger1 is currently implementing the synaptics slave device driver. We are also in the process of switching to VoodooInput (when I started the project it did not exists, so we depend on an old version of VoodooI2C for multitouch right now) and some questions came up.

Thinkpads usually have additionally to the trackpad a trackpoint that is also exposed via SMBus and we have already added some code for the interaction between trackpoint and touchpad (disabling behaviour mostly) which is similar to code for interaction between keyboard and touchpad too. Right now every satellite in VoodooI2C and probably also VoodooPS2 needs to deal with such things on it's own. We would rather not want to go that path in VoodooSMBus but have one place that deals with all these input scenarios similar as it is done in Linux' libinput (see https://wayland.freedesktop.org/libinput/doc/latest/features.html). Features such as disable-while-typing (but also palm detection) should be implemented in a way that allows reuse as much as possible.

For us it seems, that VoodooInput would be the right place for those features and we would be interested in your opinion about adding these features to VoodooInput. Of course we understand that this would change quite a few things for VoodooI2C's satellites and VoodooPS2 but the future improvements might be worth it.

vit9696 commented 4 years ago

For me it sounds like a good idea, but I very well wonder how much common it is going to have mainly because different trackpads have different features. Perhaps @kprinssu and @usr-sse2 could comment more on it as I have very little knowledge on what happens on touchpad side.

Also, VoodooInput is not currently linked into VoodooPS2 or VoodooI2C, so as a result we cannot directly call functions in it. I think we can lift this limitation if necessary.

vit9696 commented 4 years ago

Other than that, you can actually reuse IOSMBUSController these days as we reverse-engineered it not so long ago: https://github.com/acidanthera/VirtualSMC/blob/master/Sensors/Private/IOSMBusController.h.

kprinssu commented 4 years ago

@leo-labs for basic mouse functionality, you will need create a mouse wrapper for trackpoint. It's best not to send input to the MT2 stack (physical buttons such as middle and right click are not supported by Apple). Rather, use the mouse wrapper to handle these inputs.

In regards to palm rejection, we can perhaps implement something similar to Rehabman's notification. The aglorithm as follows:

The above is a simple approach I can think as Apple's MT2 usually handles palm rejection but you need accurate finger sizes.

leo-labs commented 4 years ago

Other than that, you can actually reuse IOSMBUSController these days as we reverse-engineered it not so long ago: https://github.com/acidanthera/VirtualSMC/blob/master/Sensors/Private/IOSMBusController.h.

This looks interesting, on the other hand the SMBus driver that I have ported from Linux works perfectly as well. Looking at the code you have linked, how does HostNotify work in IOSMBusController?

leo-labs commented 4 years ago

@kprinssu We already have a working mouse wrapper for the trackpoint in VoodooSMBus (https://github.com/leo-labs/VoodooSMBus/blob/master/VoodooSMBus/TrackpointDevice.cpp). However there is still similar interaction needed as for the touchpad: e.g. ignoreall and disabling the touchpad when the trackpoint is used for example. Also scrolling comes to my mind.

Right now, every slave device driver still needs to publish such a device (https://github.com/leo-labs/VoodooSMBus/blob/master/VoodooSMBus/ELANTouchpadDriver.cpp#L181).

VoodooInput could have an additional message interface where we can register devices (e.g. we would register the trackpoint and touchpad (and even a touchscreen) as different devices) and their capabilities (number of fingers, relative vs. absolute, clickpad, etc.). We could then have all the filtering logic (such as disable-while-typing that is now implemented in each and every satellite and slave device driver) in one place. More sophisticated palm detection (https://wayland.freedesktop.org/libinput/doc/latest/palm-detection.html) can then also be implemented at a single place. Even if that means to simply report pressure and touch size / ellipsis correctly to MT2.

@vit9696 Since you were wondering how much touchpads have in common I mentioned to report capabilities to VoodooInput similar as it is done in libinput (https://www.mankier.com/4/libinput#Configuration_Details). Obviously not all options make sense but some of them would still reduce code duplication.

vit9696 commented 4 years ago

@leo-labs it is done via client messaging with kIOMessageSMBusAlarm type. See handleACPINotification implementation in SMCBatteryManager. We implemented emulated AppleSmartBattery over virtual SMBus there. I agree that you may not use this if you already have a solution, but perhaps it can be interesting to you.

As for VoodooInput suggestions… Well, we will most likely review pull requests if they come, but I am afraid we otherwise do not have enough manpower to drive it.

sunrez commented 4 years ago

I would like to also request some kind of optional palm rejection logic to be implemented inside VoodooInput beyond a simple disable touchpad inputs after typing ; that feature already exists but doesn't help with accidental brushes of the palm when one has not touched the keyboard.

I think borrowing from https://wayland.freedesktop.org/libinput/doc/latest/palm-detection.html if possible would be highly beneficial. The logic there does both shape and area based filtering.

Having worked on input devices in a past career, I say that Apple's touchpad firmware likely does a good job of filtering so the OS never sees the false inputs ; sadly the Elan, Synaptics and controllers from everyone else are not as nicely tuned which means some filtering between the raw touches will likely be needed.

usr-sse2 commented 4 years ago

@leo-labs for basic mouse functionality, you will need create a mouse wrapper for trackpoint. It's best not to send input to the MT2 stack (physical buttons such as middle and right click are not supported by Apple). Rather, use the mouse wrapper to handle these inputs.

In regards to palm rejection, we can perhaps implement something similar to Rehabman's notification. The aglorithm as follows:

  • keyboard kext sends notification to VoodooInput
  • VoodooInput receives notification and stops processing input for X seconds

The above is a simple approach I can think as Apple's MT2 usually handles palm rejection but you need accurate finger sizes.

The mouse wrapper should definitely be moved from VoodooPS2 to VoodooInput (for buttons and trackpoints which exist on both PS/2 and I2C trackpads). As I now have both PS/2 and I2C (and on I2C buttons currently don't work at all), I hope that I would find time to implement it.

tiger511 commented 4 years ago

Maybe this will help implementing palm rejection in Precision Trackpad:

https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-tuning-guidelines

vit9696 commented 3 years ago

VoodooInput got trackpoint support as of https://github.com/acidanthera/VoodooInput/commit/c6f297714669af2701bf8aeaec463829cd50c0bb. I believe this is now resolved. Closing.