wayland: Add support for PinchGesture and RotationGesture #3656

linkmauve · 2024-04-27T11:45:41Z

These two events are synthesized from the same event from the same protocol, zwp_pointer_gestures_v1, and will always come together.

I’ve left over the third value, which is a dx/dy from the center of the gesture, it could be exposed as a 2D scroll but I feel like this should be a different event than the normal AxisMotion.

The documentation doesn’t indicate the unit of the rotation, so I’ve left it as degrees. I don’t have any Apple OS where I could test whether that event is also in degrees or in radians there.

The other two gestures supported by this protocol, a multi-finger swipe, and a multi-finger hold, aren’t currently matched with any event in winit, would it make sense to expose them?

I’ve only tested on GNOME, but I expect other compositors to expose the same protocol eventually.

Tested on all platforms changed
Added an entry to the changelog module if knowledge of this change could be valuable to users
Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
Created or updated an example program if it would help users understand this functionality
Updated feature matrix, if new features were added or implemented

The last two items aren’t needed, as they were already added for the other two platforms which support this feature.

madsmtm · 2024-04-27T13:58:43Z

The rotation delta is in degrees on macOS, and that's what the iOS impl tries to match as well.

(Though we should perhaps change it to radians, that's the morally superior unit).

linkmauve · 2024-04-27T15:47:06Z

Thanks! Now that this other PR is merged, I’ve also implemented PanGesture during the pinch.

These two events are synthesized from the same event from the same protocol, zwp_pointer_gestures_v1, and will always come together. I’ve left over the third value, which is a dx/dy from the center of the gesture, it could be exposed as a 2D scroll but I feel like this should be a different event than the normal AxisMotion. The documentation doesn’t indicate the unit of the rotation, so I’ve left it as degrees. I don’t have any Apple OS where I could test whether that event is also in degrees or in radians there. The other two gestures supported by this protocol, a multi-finger swipe, and a multi-finger hold, aren’t currently matched with any event in winit, would it make sense to expose them? I’ve only tested on GNOME, but I expect other compositors to expose the same protocol eventually.

This new event is exactly what was missing for the previous commit.

This has only been tested on Wayland against rust-windowing/winit#3656 and allows much nicer interactions when using a touchpad. The values have been chosen based on what felt pleasant to use on my Thinkpad x280, I haven’t given it much thought otherwise, and I expect other touchpads would feel different.

kchibisov

Besides that I have no idea what pan gesture is and that our docs are a bit confusing about it and when to use.

It seems like it's just a movement within the gesture disregarding amount of fingers, etc?

kchibisov · 2024-05-04T10:54:07Z

src/event.rs

@@ -281,7 +281,7 @@ pub enum WindowEvent {
    ///
    /// ## Platform-specific
    ///
-    /// - Only available on **macOS** and **iOS**.
+    /// - Only available on **macOS**, **iOS** and **Wayland**.


Suggested change

/// - Only available on **macOS**, **iOS** and **Wayland**.

/// - Only available on **macOS**, **iOS**, and **Wayland**.

kchibisov · 2024-05-04T10:54:17Z

src/event.rs

@@ -297,7 +297,7 @@ pub enum WindowEvent {
    ///
    /// ## Platform-specific
    ///
-    /// - Only available on **iOS**.
+    /// - Only available on **iOS** and **Wayland**.


Suggested change

/// - Only available on **iOS** and **Wayland**.

/// - Only available on **iOS**, and **Wayland**.

kchibisov · 2024-05-04T10:54:27Z

src/event.rs

@@ -333,7 +333,7 @@ pub enum WindowEvent {
    ///
    /// ## Platform-specific
    ///
-    /// - Only available on **macOS** and **iOS**.
+    /// - Only available on **macOS**, **iOS** and **Wayland**.


Suggested change

/// - Only available on **macOS**, **iOS** and **Wayland**.

/// - Only available on **macOS**, **iOS**, and **Wayland**.

kchibisov · 2024-05-04T10:54:51Z

src/changelog/unreleased.md

+
+### Added
+
+- On Wayland, add `PinchGesture`, `PanGesture` and `RotationGesture`.


https://www.grammarbook.com/blog/commas/oxford-comma/

Suggested change

- On Wayland, add `PinchGesture`, `PanGesture` and `RotationGesture`.

- On Wayland, add `PinchGesture`, `PanGesture`, and `RotationGesture`.

kchibisov · 2024-05-04T11:05:47Z

src/platform_impl/linux/wayland/seat/pointer/mod.rs

+                (TouchPhase::Started, pan_delta, 0., 0.)
+            },
+            Event::Update { time: _, dx, dy, scale, rotation } => {
+                let pan_delta = PhysicalPosition::new(dx as f32, dy as f32);


It's in surface coordinates, but PhysicalPosition is buffer, you should convert from logical to physical with the function like in wl_pointer events above.

kchibisov · 2024-05-04T11:06:03Z

src/platform_impl/linux/wayland/seat/pointer/mod.rs

+                let pan_delta = PhysicalPosition::new(0., 0.);
+                state.previous_scale = 1.;
+                (
+                    if cancelled == 0 { TouchPhase::Ended } else { TouchPhase::Cancelled },


Define it on a separate line, so it'll end up more compact.

kchibisov · 2024-05-04T11:07:54Z

src/platform_impl/linux/wayland/seat/pointer/mod.rs

+        state.events_sink.push_window_event(
+            WindowEvent::PanGesture { device_id, delta: pan_delta, phase },
+            state.window_id,
+        );


Isn't Pan basically a swipe?

@madsmtm do you know about it? Also, it's strange that we don't have amount of fingers in Pan gesture if it's so.

The terminology is probably different elsewhere, but I think a pan gesture (docs) is similar, but distinct from a swipe gesture (docs); the pan is more general (e.g. moving a map around), a swipe is usually short and only in one direction (e.g. dismissing a pop-up).

It would make sense to have the number of fingers in the event.

kchibisov · 2024-05-04T11:08:27Z

src/platform_impl/linux/wayland/seat/pointer/mod.rs

+        state.events_sink.push_window_event(
+            WindowEvent::PinchGesture { device_id, delta: scale_delta, phase },
+            state.window_id,
+        );
+        state.events_sink.push_window_event(
+            WindowEvent::RotationGesture { device_id, delta: rotation_delta, phase },
+            state.window_id,
+        );


You should only send updates if there was a change compared to previous state, so if there was not pinch, but just rotation, only rotation should be send.

kchibisov · 2024-05-04T11:09:35Z

src/platform_impl/linux/wayland/state.rs

+    /// XXX: Is this really meant to stay here?
+    pub window_id: WindowId,


No, it should be like with keyboard focus, on user data.

kchibisov · 2024-05-04T11:09:47Z

src/platform_impl/linux/wayland/state.rs

+    /// XXX: Is this really meant to stay here?
+    pub previous_scale: f64,


No, should be on user data as well.

linkmauve requested a review from kchibisov as a code owner April 27, 2024 11:45

linkmauve force-pushed the wayland-pinch-gesture branch 3 times, most recently from 01977f7 to 5b4bb44 Compare April 27, 2024 11:51

linkmauve mentioned this pull request Apr 27, 2024

Winit version 0.30.0 #3655

Merged

3 tasks

linkmauve force-pushed the wayland-pinch-gesture branch from 5b4bb44 to 50f66ed Compare April 27, 2024 12:47

linkmauve force-pushed the wayland-pinch-gesture branch 2 times, most recently from 29ee32b to 3f20162 Compare April 27, 2024 15:46

linkmauve added 2 commits April 27, 2024 19:57

wayland: Also synthesize PanGesture from a pinch

8815ac8

This new event is exactly what was missing for the previous commit.

linkmauve force-pushed the wayland-pinch-gesture branch from 3f20162 to 8815ac8 Compare April 27, 2024 17:58

linkmauve mentioned this pull request Apr 27, 2024

Update dependencies bwasty/gltf-viewer#70

Open

kchibisov requested changes May 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wayland: Add support for PinchGesture and RotationGesture #3656

wayland: Add support for PinchGesture and RotationGesture #3656

linkmauve commented Apr 27, 2024 •

edited

madsmtm commented Apr 27, 2024

linkmauve commented Apr 27, 2024

kchibisov left a comment

kchibisov May 4, 2024

kchibisov May 4, 2024

kchibisov May 4, 2024

kchibisov May 4, 2024

kchibisov May 4, 2024

kchibisov May 4, 2024

kchibisov May 4, 2024

madsmtm May 5, 2024

kchibisov May 4, 2024

kchibisov May 4, 2024

kchibisov May 4, 2024

	/// - Only available on macOS, iOS and Wayland.
	/// - Only available on macOS, iOS, and Wayland.

	/// - Only available on iOS and Wayland.
	/// - Only available on iOS, and Wayland.


		### Added

		- On Wayland, add `PinchGesture`, `PanGesture` and `RotationGesture`.

		/// XXX: Is this really meant to stay here?
		pub window_id: WindowId,

		/// XXX: Is this really meant to stay here?
		pub previous_scale: f64,

wayland: Add support for PinchGesture and RotationGesture #3656

Are you sure you want to change the base?

wayland: Add support for PinchGesture and RotationGesture #3656

Conversation

linkmauve commented Apr 27, 2024 • edited

madsmtm commented Apr 27, 2024

linkmauve commented Apr 27, 2024

kchibisov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linkmauve commented Apr 27, 2024 •

edited