NICHOLAS85 / gebaar-libinput

Gebaar, A Super Simple WM Independent Touchpad Gesture Daemon for libinput
GNU General Public License v3.0
7 stars 4 forks source link

Extracting device XID using libinput_device_get_sysname #16

Open cdurden opened 3 years ago

cdurden commented 3 years ago

We can't uniquely map vendor ID and product ID to an XID for our input device. This leads to possible confusion when mapping a vendor ID and product ID to an XID, particularly for touchscreen devices (See the section on libinput_device_get_device_group() at Initialization and manipulation of input devices). This pull request attempts to get the XID directly from the libinput device object using libinput_device_get_sysname.

Background

There is a 1:1 mapping between libinput devices and /dev/input/eventX device nodes. (Source)

Based on the code in the libinput-tools here, it looks like libinput_device_get_sysname returns eventX

Note: The X in /dev/input/eventX represents is here called the XID . This appears to be consistent with the use of the name XID in the xinput documentation.

NICHOLAS85 commented 3 years ago

Thanks for the links! I indeed looked into using libinput_device_get_sysname but was unable to correctly correlate it to the id we needed. Doesn't mean it's impossible of course, I'll keep on researching. It looks like after adding your changes the wrong ID is being passed to the matrix retrieving portion of determine_orientation on my system. Could you explain the following line? https://github.com/cdurden/gebaar-libinput/blob/fcd2bb88f06ed2320d0b34a11f14d6f101010bd5/src/io/input.cpp#L106

int id = atoi(sysname+5);

I'm curious as to why +5 is used

NICHOLAS85 commented 3 years ago

If I can't find a direct way to convert libinput_device_get_sysname into a device id xinput uses I could just replace my {vendorid/productid - device id} map with a {device node - device id} map which would hopefully fix the original issue. After that point I'd have to look into how to handle added/removed input devices if necessary.

cdurden commented 3 years ago

I looked at the output of the command "libinput debug-events" and I saw that the first field it prints is "eventX," where X is the XID (Correction: this is called the sysname/device node, which is not the same as the XID). So I looked in the source code for that tool and I found that this string "eventX" is the return value of libinput_device_get_sysname. So I assumed that this function can be used to get the XID, even though the documentation doesn't really say what it does.

That function returns a char pointer, so I thought I could just use pointer arithmetic to shave off the characters "event." I think sysname+5 is a pointer to the 6th character of sysname. I think atoi should use all of the remaining characters of sysname up to the null character, converting to integer. Not sure why this would not give the XID.

NICHOLAS85 commented 3 years ago

Ahh I understand better now. That line does indeed work as you're describing, it returns an integer with the devices eventX however it seems that is not what xinput expects when referring to a device's XID. For example /dev/input/event9 on my machine (Wacom HID 4833 Finger touch) has an xid of 16 at the moment. These two values don't seem to have any correlation afaik.

cdurden commented 3 years ago

I see. While it might help, I'm not convinced that the {device node - device id/XID} map will resolve the issue in all cases. For instance, it appears that my stylus and eraser have one device node and they appear as two distinct xinput devices with distinct XIDs. Furthermore, it is possible to set distinct Coordinate Transformation Matrices for these two xinput devices. I've thought of two avenues for trying to further address this issue.

Getting and setting device orientation information using libinput?

First, is it possible to set screen orientations using libinput instead of xinput? It seems that most of the setups I've seen handle orientation changes by updating the Coordinate Transformation Matrices, but libinput has the functions libinput_device_config_calibration_get_matrix() and libinput_device_config_calibration_set_matrix(). If orientation can be set in libinput, then perhaps xinput is not needed. Here are some posts that address coordinate transformations in libinput:

“Coordinate Transformation Matrix” and “libinput Calibration Matrix” - how are they related? Information for developers > Absolute axes libinput - Device configuration

Continuing with xinput implementation

Since a given libinput device can be associated with multiple xinput devices, I suggest comparing the Coordinate Transformation Matrices for all xinput devices associated with a given libinput device. If the matrices are different, then I think it would make sense to issue a warning to indicate that there is some ambiguity about the orientation for that set of devices. If the Coordinate Transformation Matrices are consistent, then update the orientation as expected.

This might not be ideal. But if libinput does not see the eraser and stylus as distinct devices, would we even be able to tell which of these inputs produced a given gesture? If not, we might as well treat them as one thing, and just make sure that everything is consistent.

NICHOLAS85 commented 3 years ago

Getting and setting device orientation information using libinput?

I actually started implementing this orientation awareness feature using the libinput calibration matrix information however stopped once I discovered, like you, that most methods of screen rotation involved manipulating a device's Coordinate transformation matrix instead. I do not know of any applications which manipulate the former instead of the latter but if you know of any please share. For now I do not have plans on adding screen rotation (and in turn device matrix manipulation) logic to this fork of Gebaar or developing an application to do so simply because I do not have the necessary expertise or time at the moment. I haven't actually tested the libinput_device_config_calibration_set_matrix() function myself either. I could look into reimplementing the orientation awareness stuff via libinput calibration matrix instead of Coordinate transformation matrix as a separate branch for use in the future though

Continuing with xinput implementation

Your suggestion sounds reasonable. My only concern is potential energy use/gesture latency. As this is currently implemented, it reads the device's Coordinate Transformation Matrix each time a gesture concludes. This is because Gebaar has no knowledge of when a screen has actually rotated and must determine this on a per-gesture basis which I understand is an inefficient process. This could be solved with the integration of screen rotation logic and matrix caching but for now I don't have plans on implementing that. The comparison you suggest can't be run just once as the matrices would most likely always match in a devices default orientation. So this means it would be run on each gestures conclusion. If you believe this additional logic wouldn't cause too much of a slowdown or isn't cause for concern let me know!

cdurden commented 3 years ago

Getting and setting device orientation information using libinput? I similarly feel that I lack the expertise to do this via calls to the library. But perhaps a higher level implementation (see below) would suffice for now. In the future, we could ask the libinput authors for guidance by posting an issue to their gitlab repository.

For now, here's a higher-level implementation that sets the device orientation using libinput: The original 2in1screen code was what I had been using to set the Coordinate Transformation Matrices based on the accelerometer. Here is a hack of the code from @aleozlx that sets the libinput Calibration Matrix instead. This works by triggering a udev rule which sets ENV{LIBINPUT_CALIBRATION_MATRIX}, as described here. I was able to get the tablet input and gestures to function with this approach. It works with gesture handling using the master branch of either touchegg or gebaar-libinput. There are a couple of issues:

  1. I have to restart touchegg or gebaard every time the orientation changes. I suppose this is okay, but there is probably a way to improve this with some reinitialization code in those projects.
  2. Setting the libinput Calibration Matrix here involves a call to udevadm trigger which requires super-user privileges.

Continuing with xinput implementation

Are you still interested in working on this implementation? I haven't noticed major slowdowns. For speed, I would suggest using conditional compilation to do the additional logic by default. Then the code would issue a warning if necessary to help users identify any issues/confusions about mappings of the devices. To increase the speed, a compile-time option could be used to override the default logic.

cdurden commented 3 years ago

I opened an issue on libinput's GitLab site to ask some questions about this. https://gitlab.freedesktop.org/libinput/libinput/-/issues/597

NICHOLAS85 commented 3 years ago

Just updated the orientationawareness branch in c96aa3a6358a2c3daf7b2e26d4ec9d5d29cbdd73 to use a device node - device id map which should be more accurate than the current map. It doesn't solve the problem of two devices sharing the same node but it is an improvement from how it was done before.