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 SDL Pinch events #9445

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

1bsyl
Copy link
Contributor

@1bsyl 1bsyl commented Apr 4, 2024

Some update:

Added SDL Pinch event (begin/update/end), with the delta scale factor.

We could add more data then the scale factor value if needed afterward (position center, diameter, rotation, global scale factor, velocity), but not sure all back-end can provide it.

I've removed the Swipe event, because it seems to be really different across platform (number fingers, swipe vs scroll definition).

Currently implemented:

  • back-ends: x11/wayland/macosx/ios/android
  • cmake detection
  • updated testgeometry to test pinch gesture (add --info event_motion in command line for verbosity)

Todo:

  • ios: is sending the global scale factor, whereas other backend are sending the delta scale ...
  • never tested on macosx (no hardware)
  • overall needs better testing

keeping the old message, because it has some info:

Add native Pinch and Swipe events for SDL.

This is DRAFT, it starts to show this can work, but clearly not functional.
Currently, I've only tried on x11 and wayland, but will also do macosx since there was some old commit over-there, and android/ios.

What can be noticed:

  • on x11, it needs xinput2 protocol 2.4 (before 2.2). that sounds ok to me.

  • on wayland, it needs "pointer-gestures-unstable-v1-client-protocol.h", since we already have added some unstable protocol, why not adding this one.

  • x11/wayland: it only detects the swipe/pinch on a touchpad (eg laptop or some device tablet). but nothing with a real touch screen. that's a restriction.

  • SDL pinch/swipe events are sent as "begin", many update, "end" sequences. seems what is done with x11 and wayland android. (at least

  • Not sure, what we should publicly expose in SDL_Event.h for the structure.
    I guess, that the big thing to determine. Also to have something coherent between desktop and smartphone/tablets.

we can just map to wayland:
https://wayland.app/protocols/pointer-gestures-unstable-v1#zwp_pointer_gesture_swipe_v1:event:begin
or x11: https://lists.x.org/archives/xorg-commit/2021-May/043325.html (see datas: GesturePinchEvent GestureSwipeEvent )

android:
https://developer.android.com/reference/android/view/ScaleGestureDetector.OnScaleGestureListener
https://developer.android.com/reference/android/view/ScaleGestureDetector

@Kontrabant
Copy link
Contributor

  • on wayland, it needs "pointer-gestures-unstable-v1-client-protocol.h", since we already have added some unstable protocol, why not adding this one.

Don't worry about this; 'unstable' is a legacy categorization at this point, as they realized that requiring backwards-incompatible changes to promote protocols to stable was a bad idea. Unstable protocols will basically be unstable forever at this point.

@1bsyl
Copy link
Contributor Author

1bsyl commented Apr 5, 2024

Original ticket was #7706

I've added some code for Macosx, but this is incomplete and untested (I have no trackpad on mac mini, and cannot get the ipad to work as a trackpad).

For MACOSX, it seems:

  • no notion of begin/end for swipe/pinch.
    • we can always encapsulate the event with a begin/end anyway.
  • pinch is actually "magnify". there is another "rotate" method.
    • should add/separate rotate to/from pinch ?
  • swipe is 3 fingers. but there is also scroll (not sure where it is), that should but a 2 fingers gesture.
    • wantsScrollEventsForSwipeTrackingOnAxis to map scroll to swipe

@1bsyl
Copy link
Contributor Author

1bsyl commented Apr 6, 2024

Android has begin/update/end callbacks for pinch events.
but for swipe, nothing yet really clear for me. if you move 1 finger, you get a swipe.
in somecase, users would prefer usual touch event than swipe event.
( https://stackoverflow.com/questions/4139288/android-how-to-handle-right-to-left-swipe-gestures )

@1bsyl
Copy link
Contributor Author

1bsyl commented Apr 10, 2024

On IOS, added callbacks to handle pinch gestures.
(tvos seems to already use the swipe gestures, and doesn't compile with pinch)

maybe, we shouldn't try to expose the swipe gesture, because, it's seems to be very different across platforms, 1 or several fingers (even the ability to report the number of fingers) and also behavior (scroll vs swipe -finger goes up at the end-)

@1bsyl 1bsyl force-pushed the br_pinch_and_swipe branch 2 times, most recently from e4ce5cf to 262b012 Compare April 11, 2024 20:56
@1bsyl 1bsyl force-pushed the br_pinch_and_swipe branch 4 times, most recently from 1c82253 to 2081211 Compare April 25, 2024 10:29
@1bsyl 1bsyl changed the title [draft] Pinch and Swipe events Add SDL Pinch events Apr 25, 2024
backend: x11/wayland/macosx/ios/Android
@1bsyl
Copy link
Contributor Author

1bsyl commented Apr 25, 2024

@libsdl-org/a-team hey, It compiles and has been cleaned up ! needs some feedback !

@ssouzawallace
Copy link

Merge This Please!

@slouken
Copy link
Collaborator

slouken commented Jun 1, 2024

@1bsyl, sorry I missed that you needed feedback on this. Let me review and get back to you.

@slouken
Copy link
Collaborator

slouken commented Jun 1, 2024

I think this is good to go as-is, it looks like your questions are about swipe gestures?

I think enabling gesture support on macOS and iOS delays touch event delivery. @icculus, what do you think about implementing pinch/zoom/rotate/swipe on top of touch events to have consistent cross-platform behavior? We could include the number of fingers in the swipe across all platforms, etc.?

@icculus
Copy link
Collaborator

icculus commented Jun 2, 2024

Making our own pinch gestures is only going to make iOS users say "this doesn't feel right for some reason." I'd say either the OS provides them or the app doesn't get those events.

@slime73
Copy link
Contributor

slime73 commented Jun 2, 2024

Would it make sense to have them opt-in with some documentation about the consequences of doing so?

@1bsyl
Copy link
Contributor Author

1bsyl commented Jun 3, 2024

@slouken no problem for the delay. actually, the PR is just a prototype to see where it goes.

yes, swipe gesture are going to be really different across platform if there are implement with different os back-end (number of finger/speed are not always provided i guess). also swipe gesture is different of scroll gesture (one expect the fingers up at the border of the screen).

but here, it's only about pinch. to simplify. but probably all back-end wont provide the number of finger for pinch, the rotation angle, the center, speed.

It makes sense that this introduce a delay for touch event. Probably that internally it starts to buffer touch event, and try to detect gestures: when the gesture is detected and sent, the touch event are discarded.

A question here is: should we send the touch event if a gesture is detected ?

I personally won´t really use the gesture things, but that sounds much easier to implement on top of touch event, than per back-end. I don't see how different ios people are used to pinch/scroll/etc that others, but maybe we could add some customization: like setting how many event before detecting a pinch/ how many finger for a scroll on ios, etc. I mean some customization so that the generic layer feels like it was a real platform back-end.

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.

None yet

6 participants