Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix dropped motion events in mtdev provider. #8207

Merged
merged 3 commits into from
Apr 21, 2024
Merged

Fix dropped motion events in mtdev provider. #8207

merged 3 commits into from
Apr 21, 2024

Conversation

rnixx
Copy link
Member

@rnixx rnixx commented Apr 13, 2023 •
edited by misl6
Loading

Maintainer merge checklist

  • Title is descriptive/clear for inclusion in release notes.
  • Applied a Component: xxx label.
  • Applied the api-deprecation or api-break label.
  • Applied the release-highlight label to be highlighted in release notes.
  • Added to the milestone version it was merged into.
  • Unittests are included in PR.
  • Properly documented, including versionadded, versionchanged as needed.

with a current setup on a 6.1.0 linux kernel and a multi touch display i encountered motion events getting dropped.

The mtdev input provider processes values if MTDEV_CODE_TRACKING_ID event is received and it's value denotes an unused slot. However, this not happens in my case, so i added processing on position changes as well to ensure on_touch_move events get fired properly.

Actually i have not tried with another kernel, so i have no idea if there are changes in libmtdev or if the multitouch display i use not follows standards with sending data.

Also i have not digged in detail into the mtdev docs (https://docs.kernel.org/6.1/input/multi-touch-protocol.html) nor did i studied the input provider implementation in depth, maybe there's a better place to fix this issue.

@rnixx rnixx added the Component: input kivy/input label Apr 13, 2023
@misl6 misl6 added this to the 2.3.0 milestone Apr 15, 2023
@misl6 misl6 modified the milestones: 2.3.0, 3.0.0 Dec 17, 2023
@rnixx
Copy link
Member Author

rnixx commented Mar 6, 2024

According to the Multi-touch (MT) Protocol docs (https://www.kernel.org/doc/html/v4.19/input/multi-touch-protocol.html)

Creation, replacement and destruction of contacts is achieved by modifying the ABS_MT_TRACKING_ID of the associated slot. A non-negative tracking id is interpreted as a contact, and the value -1 denotes an unused slot.

In the current implementation, changes are applied by interpreting ABS_MT_TRACKING_ID -1 as sync, which is not the way it is supposed to be done.

When taking a look at the protocol B example (https://www.kernel.org/doc/html/v4.19/input/multi-touch-protocol.html#protocol-example-b)

ABS_MT_SLOT 0
ABS_MT_TRACKING_ID 45
ABS_MT_POSITION_X x[0]
ABS_MT_POSITION_Y y[0]
ABS_MT_SLOT 1
ABS_MT_TRACKING_ID 46
ABS_MT_POSITION_X x[1]
ABS_MT_POSITION_Y y[1]
SYN_REPORT

there can be changes for several slots followed by a EV_SYN/SYN_REPORT which should be used to process changes.

All drivers mark the end of a multi-touch transfer by calling the usual input_sync() function. This instructs the receiver to act upon events accumulated since last EV_SYN/SYN_REPORT and prepare to receive a new set of events/packets.

So i changed the implementation/PR to process events every time a sync is received and there are changes of interest. i also did some minor polishing with the changes set to reduce noise.

IMO this PR is now ready to be merged. If someone has objections please let me know

@rnixx rnixx requested review from tshirtman, tito and misl6 March 6, 2024 14:03
@rnixx
Copy link
Member Author

rnixx commented Apr 21, 2024

anyone?

misl6
misl6 approved these changes Apr 21, 2024
View reviewed changes
Copy link
Member

@misl6 misl6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyone?

I'm unable to test it on runtime as I do not have such a setup ready, but:

  • Code looks good
  • You digged into the MTDev code and made a perfect explanation on "why we should change it", and you made changes to the PR accordingly.

So, that's an LGTM.
Please just consider taking care of this specific provider if you're using it quite extensively in the future.

@misl6 misl6 merged commit 677071e into kivy:master Apr 21, 2024
29 checks passed
@rnixx rnixx deleted the rnixx-mtdev-move-events branch April 22, 2024 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@misl6 misl6 misl6 approved these changes

@tshirtman tshirtman Awaiting requested review from tshirtman

@tito tito Awaiting requested review from tito

Assignees
No one assigned
Labels
Component: input kivy/input
Projects
Kivy's Control Tower
Status: Done
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rnixx @misl6