Add more information for pinch gestures on mobile#15092
Conversation
|
This breaks binary compatibility, because the windowID field of SDL_PinchFingerEvent changes offset. An app compiled against an older SDL would get a garbage window id if dynamically linked to an SDL library with this patch. |
|
Does the newest commit fix the issue? |
No: |
|
My bad – fixed now |
|
This generally seems like a good idea. 0 could be a valid value, so we should probably use -1 if the value isn't available, and document that. The other platforms should include some data, so let's try to fill those in if we can. @Kontrabant, do you have any insight here? |
Would it make sense to send the angle of the gesture instead of the span? It would still fulfill the use case of zooming more rapidly on on one axis, and would work everywhere. Most likely, if the span were returned, anything wanting to use it for accelerating a zoom on one axis would end up deriving the angle anyway. |
I agree that would make sense. For my use case, I computed the cos/sin of the angle from the span information. So, it does not detract from the functionality if SDL returns the angle instead of the span info.
I'll try to implement that. |
…ameworks, set 'focus' and 'span' to -1 if they're unavailable
|
Some wrinkles:
On the bright side:
Cocoa/AppKit also provides finger locations, but from the comments about NULL windows in the Cocoa/AppKit file that I listed, I gather that they're locations on a trackpad. Adding the associated 'span' and 'focus' info to SDL_PinchFingerEvent would compel use of 0->1 normalization for these values because then they would have different coordinated spaces (screen or touchpad) depending on the context. Is it worth it? Or maybe it's acceptable to use pixel screen coordinates for mobile/touchscreen contexts (uikit and android) and 0->1 touchpad coordinates for desktop contexts (Cocoa/AppKit)? For now, I set the Cocoa/Appkit span/focus values to the dummy value: -1. |
| Uint32 reserved; | ||
| Uint64 timestamp; /**< In nanoseconds, populated using SDL_GetTicksNS() */ | ||
| float scale; /**< The scale change since the last SDL_EVENT_PINCH_UPDATE. Scale < 1 is "zoom out". Scale > 1 is "zoom in". */ | ||
| float span_x; /**< The average X distance between each of the pointers forming the pinch in screen pixel coordinates. Or, -1 if this information is unavailable. */ |
There was a problem hiding this comment.
These need to be added to the end of the struct, so we don't break ABI compatibility. Put them under windowID, please.
There was a problem hiding this comment.
(you mentioned this was fixed...did it not get pushed?)
There was a problem hiding this comment.
I don't know how that happened. I figure I checkout'ed an old commit without paying close enough attention. Fixed again now.
|
@brentfpage, you're planning to switch this to use angles which will enable this feature for X11 and so forth, right? @Kontrabant, am I reading that right? |
I don't think it can be done. The angle associated with libinput pinch events is a rotation relative to the previous frame. I'm guessing that only this information is available because libinput is not intended for touchscreen use – just trackpad use. It may make sense to have separate pinch events for trackpads and touchscreens. It's not as though the current pinch API is written in stone – it was added something like last october if memory serves. |
|
We should document that these values are only sent on mobile devices. Once that's done, @Kontrabant, do you have any more feedback? Do you think this is a good addition? |
|
It's unfortunate that libinput and desktop Macs work differently here, but this otherwise seems useful on mobile devices and looks good to me. |
|
I made it clear now in the documentation for |
Description
SDL_PinchFingerEventstruct defined in include/SDL3/SDL_events.h has been expanded to include the midpoint of the finger positions comprising the pinch gesture (focus) as well as the x and y displacements between the finger positions (span). Including this information allows a programmer to implement a zoom in response to the gesture that a.) keepsfocuscentered on the screen and that b.) zooms the y axis more rapidly if the user's fingers are more vertically aligned.SDL_SendPinchin src/events/(SDL_touch_c.h and SDL_touch.c) accommodate this newfocusandspaninformationfocusandspaninfo is forwarded on to downstream functions in addition toscale, while previously onlyscalewas forwarded. This forwarding function is specificallyonNativePinchUpdateonNativePinchUpdatetoSDL_SendPinchhas been expanded to includefocusandspanSDL_SendPinchin non-Android user interface frameworkshave been adjusted so that
event.pinch.focus_x,event.pinch.focus_y,event.pinch.span_x, andevent.pinch.span_yare set to zero bySDL_SendPinch. Existing uses of pinch events will only involveevent.pinch.scale, so these changes are non-breaking. On the other hand, it may be surprising to a programmer thatfocusandspanare zero considering that they exist. I am unfamiliar with these UI frameworks, so won't attempt to addfocusandspaninfo to theirSDL_SendPinchcalls, but some glances at these frameworks' APIs suggests this would be possible to some extent.Coordinate system transformations:
In SDLSurface.java, the Android gesture detector returns
spanandfocusinfo in screen pixel coordinates. For consistency with what is done in that same file for simple touch events, I convertspanandfocusto normalized screen coordinates before passing them on toSDL_SendPinch/onNativePinchUpdate. But, I convert them back to pixel screen coordinates inSDL_SendPinch, again to be consistent with what is done for touch events (inSDL_SendTouch)To be precise, the pinch info transformations are very nearly inverses of one another, but for reasons I'm unfamiliar with, the original conversion to normalized screen coordinates divides by (pixel_count - 1) instead of (pixel_count), while the subsequent conversion back to screen pixel coordinates multiplies by pixel_count. The final change in coordinates by a multiplicative factor pixel_count/(pixel_count - 1) is minuscule for all devices.