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.

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

Add client touch capability support #499

Merged
merged 7 commits into from
Mar 1, 2022
Merged

Conversation

chrisduerr
Copy link
Contributor

@chrisduerr chrisduerr commented Feb 22, 2022

While Smithay already supports libinput's touch events, it did not
support forwarding touch events to clients yet.

This patch implements a new TouchHandle, which can be acquired from
Seat::get_touch.

pub struct TouchSlot {
id: u64,
id: Option<u32>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've decided to change the TouchSlot here because it already was creating some friction inside my compositor when handling touch events and just created more problems when it came to converting the Option<TouchSlot> back to a i32 to send it to the client.

So since I'd assume nobody is using touch yet (since it doesn't work for clients), changing it now will prevent headaches in the future.

/// # "seat-0".into(),
/// # None
/// # );
/// let touch_handle = seat.add_touch();
Copy link
Member

Choose a reason for hiding this comment

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

This example doesn't really explain anything more than the function docs itself.

If the seat creation are made visible it would be more jusitifed to have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, but I'd say the same applies for the existing code examples for keyboard/pointer?

If the seat creation are made visible it would be more jusitifed to have.

What do you mean by this? Removing the # before the seat stuff?

@chrisduerr
Copy link
Contributor Author

chrisduerr commented Feb 23, 2022

I've uploaded some videos for the people unable to test with touchscreens (these will be gone in 24h, unfortunately the videos are too big for GH):

Touch Input

Multi-touch

Copy link
Member

@PolyMeilex PolyMeilex left a comment

Choose a reason for hiding this comment

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

I have no way to test it, but overall this looks really good 👍
Just a few minor nitpicks.

Copy link
Member

@PolyMeilex PolyMeilex left a comment

Choose a reason for hiding this comment

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

Ok, looks good to me now.

@PolyMeilex
Copy link
Member

Actually, could we get a short changelog entry for this? Quick note about the addition and maybe mention the braking change of touch slot (although I doubt anyone was using it)

@chrisduerr
Copy link
Contributor Author

Actually, could we get a short changelog entry for this? Quick note about the addition and maybe mention the braking change of touch slot (although I doubt anyone was using it)

Yep, of course. Was just waiting for feedback on the general design first.

CHANGELOG.md Outdated
@@ -14,6 +14,8 @@
- `MouseButton` is now non-exhaustive.
- Remove `Other` and add `Forward` and `Back` variants to `MouseButton`. Use the new `PointerButtonEvent::button_code` in place of `Other`.
- `GrabStartData` has been renamed to `PointerGrabStartData`
- Added `TouchHandle` for Wayland client touch support (see `Seat::get_touch`)
Copy link
Member

Choose a reason for hiding this comment

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

Technically, this should be under Additions > Clients & Protocols as this is not "Braking Change" per se

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

I agree with @PolyMeilex's remarks, but other than that, this is good for merge.

@Drakulix
Copy link
Member

Drakulix commented Mar 1, 2022

@chrisduerr Could you rebase on master? I pushed some fixes to make CI on master green again and would like to verify CI works on this branch as well before merging it.

While Smithay already supports libinput's touch events, it did not
support forwarding touch events to clients yet.

This patch implements a new `TouchHandle`, which can be acquired from
`Seat::get_touch`.
@chrisduerr
Copy link
Contributor Author

Done.

@PolyMeilex PolyMeilex merged commit 4c03819 into Smithay:master Mar 1, 2022
@PolyMeilex
Copy link
Member

Thank you!

@chrisduerr chrisduerr deleted the touchy branch March 1, 2022 21:35
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.

4 participants