Add SDL Pinch events#9445
Conversation
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. |
|
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:
|
|
Android has begin/update/end callbacks for pinch events. |
|
On IOS, added callbacks to handle pinch gestures. 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-) |
e4ce5cf to
262b012
Compare
1c82253 to
2081211
Compare
d9e4658 to
023cf24
Compare
|
@libsdl-org/a-team hey, It compiles and has been cleaned up ! needs some feedback ! |
|
Merge This Please! |
|
@1bsyl, sorry I missed that you needed feedback on this. Let me review and get back to you. |
|
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.? |
|
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. |
|
Would it make sense to have them opt-in with some documentation about the consequences of doing so? |
|
@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. |
|
Can I have write access to this repository can merge pull request? |
Uh, no, but we should probably merge this soon. Where I stand on this:
|
|
Also, this needs to be updated to the latest in main, with an extremely minor conflict resolved. |
| SDL_EVENT_FINGER_UP, | ||
| SDL_EVENT_FINGER_MOTION, | ||
|
|
||
| /* Pinch events */ |
There was a problem hiding this comment.
Zoom events? It wasn't intuitive for me that a pinch event would also mean the opposite of pinch. :)
There was a problem hiding this comment.
Although looking through the implementations, at least half of them refer to this as pinch, so this is probably fine. The event is well documented.
There was a problem hiding this comment.
hum not sure I got it, but yes pinch and zoom are the same thing.
and this can be "pinch in" or "pinch out" for "zoom in/out"
There was a problem hiding this comment.
I have documented more the events enum started/updated/end
| SDL_EventType type; /**< ::SDL_EVENT_PINCH_BEGIN or ::SDL_EVENT_PINCH_UPDATE or ::SDL_EVENT_PINCH_END */ | ||
| Uint32 reserved; | ||
| Uint64 timestamp; /**< In nanoseconds, populated using SDL_GetTicksNS() */ | ||
| float scale; /**< The scale factor provided with SDL_EVENT_PINCH_UPDATE. Scale < 1 is "zoom out". Scale > 1 is "zoom in". */ |
There was a problem hiding this comment.
We could potentially add a rotation here, as some of the implementations support it.
There was a problem hiding this comment.
I think, we could have more, but all back-end won't have them:
number of finger user for pinch,
the rotation angle,
the center of fingers used,
speed
14ea90d to
04f5aac
Compare
backend: x11/wayland/macosx/ios/Android
410e10d to
317d2c5
Compare
|
I've sync'ed the branch... not sure if it still compiles ... |
|
@madebr Hey, I try to compile (quickly) a single sdl test apk (testgeometry). but that fails after it builds, any idea ? not sure but:
Thanks! here's my commands: cd build
cmake .. -DCMAKE_TOOLCHAIN_FILE=$ANDROID_NDK_HOME/build/cmake/android.toolchain.cmake -DANDROID_ABI=arm64-v8a -DSDL_ANDROID_HOME=$ANDROID_HOME -DANDROID_PLATFORM=25 -DSDL_TESTS=ON
cmake --build . --target SDL3-jar
cmake --build . --target testgeometry-apk
adb install -r ./test/testgeometry.apk
edit: it's working now thanks @madebr |
|
hey @madebr ! sorry to bother, any idea to this error ? |
That code is still present and mostly unchanged, and exercised daily on ci. Lines 1563 to 1616 in a1672f2 The SDL3-jar target should create a SDL3 Java archive.
Clean you build directory and try again? |
|
ok, this is working now, I forget to call the SDL3-jar target |
|
though, I am still not sure why it tries to bulid ulicm, direct3d, vita, ngage, etc. |
80a353f to
172a5c4
Compare
|
This is merged. Thanks @1bsyl, for all your work on this! |
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:
--info event_motionin command line for verbosity)Todo:
keeping the old message, because it has some info: