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

Trackpad-specific input event support #482

Merged
merged 7 commits into from
Nov 11, 2024

Conversation

thatcomputerguy0101
Copy link
Contributor

This adds support for pinch and rotation gesture events, as exposed from winit on MacOS platforms. Additionally, this associates ctrl-scrolling with the pinch gesture unless actual gesture events are detected since some Windows trackpads expose pinch information as scrolling with the ctrl key held. This somewhat breaks any custom control systems that expect pinch to zoom until they are updated with the new pinch event. However, since key modifiers are not made accessible to the camera control interface, any old handlers are still accessible by scrolling vertically.

Copy link
Owner

@asny asny left a comment

Choose a reason for hiding this comment

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

That's a nice feature to add 👌

src/window/winit_window/frame_input_generator.rs Outdated Show resolved Hide resolved
@thatcomputerguy0101
Copy link
Contributor Author

Instead of remapping events, I added the capability to execute different camera actions depending on the state of the modifier keys. This removed the Copy trait from CameraAction and CameraControl due to Box now being involved, but standard uses of those two types outside of the three-d crate should not be affected.

@thatcomputerguy0101
Copy link
Contributor Author

On further consideration, I'm not sure I'm happy with the Box-based modifier map, so I'm going to separate that from this PR to allow for further design revision. In the meantime, custom control interfaces could handle modifier keys before passing the event list to CameraControl::handle_events.

@asny
Copy link
Owner

asny commented Nov 11, 2024

I think it looks really good. Since I was so slow responding, I resolved the merge conflict and merged the changes. If you think there's more changes needed, feel free to open another PR 🙂 Thanks for the contribution, yet again 🙏

@asny asny merged commit bf25a5e into asny:master Nov 11, 2024
5 checks passed
@asny
Copy link
Owner

asny commented Nov 11, 2024

@thatcomputerguy0101 While looking at your changes, I realised that the whole control setup is a bit over engineered. Therefore, I tried to remove the CameraControl and the CameraAction in #507. It simply applies the incoming events directly to the camera. This actually made the whole setup much simpler, so I'm going to merge that. Feedback to this approach is very welcome 🙏

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.

2 participants