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

sokol_app.h: add a tool_type field to the sapp_touchpoint struct (valid on Android) #717

Merged
merged 2 commits into from Oct 8, 2022

Conversation

Wertzui123
Copy link
Contributor

This makes it possible to differentiate between different types of touch input.
See: https://developer.android.com/ndk/reference/group/input#amotionevent_gettooltype for the documentation of this function
I've tested it and it successfully showed 1 for finger input and 2 for S-Pen input.

I'm unsure if adding a new field to the event struct is the way to go; maybe we should provide the user directly with the AMotionEvent and the pointer_index (e and idx variables) instead so that they can then use arbitrary NDK functions in their callbacks.

@billzez
Copy link
Contributor

billzez commented Sep 19, 2022

I think it might make more sense to emit mouse events so that Android matches other platforms, and we don't have to use two different APIs to handle mouse input.

@Wertzui123
Copy link
Contributor Author

No, I don't think so. Afaik, mouses on Android already emit a mouse event, but touch should remain touch.

@Wertzui123
Copy link
Contributor Author

I've noticed my previous implementation (a field in the event struct) didn't make much sense in the multi-touch architecture of sokol_app.h.
Instead, I have changed it to a field of the sapp_touchpoint struct now, which should also be a smaller bc break for wrappers.

@billzez
Copy link
Contributor

billzez commented Sep 19, 2022

Actually, android has a single API for both mouse and touch, and calls them "pointers". Currently sokol creates touch events, but not mouse events for android.

@Wertzui123
Copy link
Contributor Author

Oh, okay, I didn't know that. Well, on one hand, it makes sense, but on the other hand, you normally have only 1 mouse, but multiple fingers, so it maybe makes sense to separate it.

@Wertzui123 Wertzui123 changed the title sokol_app.h: add a touch_tool_type field (valid on Android) sokol_app.h: add a tool_type field to the sapp_touchpoint struct (valid on Android) Sep 19, 2022
Wertzui123 added a commit to Wertzui123/v that referenced this pull request Sep 19, 2022
@floooh
Copy link
Owner

floooh commented Oct 8, 2022

I'll start looking into this PR now.

@billzez: I'm not a big fan of unified mouse and and touch events, for instance UWP also only has this sort of "pointer events", and it's quite a mess. It's better if the application decides to explicitely support touch vs mouse input, and if it wants to unify those two.

@floooh
Copy link
Owner

floooh commented Oct 8, 2022

@Wertzui123: Do you know if there's a documentation somewhere what the integer return value from AMotionEvent_getToolType() actually means? I have a hard time finding any useful documentation.

Also, I'll rename the field to android_tooltype, to make clear that it is only populated on Android.

PS: nevermind, found it: https://developer.android.com/reference/android/view/MotionEvent#TOOL_TYPE_FINGER

I'll add an sapp_android_tooltype enum with those values, and replace the plain integer with the enum.

@Wertzui123
Copy link
Contributor Author

Do you know if there's a documentation somewhere what the integer return value from AMotionEvent_getToolType() actually means? I have a hard time finding any useful documentation.

I think you are looking for this: https://developer.android.com/ndk/reference/group/input#anonymous-enum-38

Also, I'll rename the field to android_tooltype, to make clear that it is only populated on Android.

I used a generic name since we may want to add iOS support in the future too (if that's possible), but I'm fine with adding an android prefix for now.

I'll add an sapp_android_tooltype enum with those values, and replace the plain integer with the enum.

Good idea.

@floooh
Copy link
Owner

floooh commented Oct 8, 2022

Please ignore the Github Actions error, this seems to be a regression in Nim.

@floooh
Copy link
Owner

floooh commented Oct 8, 2022

since we may want to add iOS support in the future too

I did some quick googling too but haven't found anything (neither iOS nor web APIs). If iOS provides that sort of information then we should go back from the android prefixes to generic names, but as long as it's not a multi-platform feature it's better to keep the prefix.

@floooh floooh merged commit f94f444 into floooh:master Oct 8, 2022
@floooh
Copy link
Owner

floooh commented Oct 8, 2022

Ok, merged, I did the following changes on top:

2a1efce

(argh, extra whitespace line, I'll remove that too...)
PS: and a comment typo...

@Wertzui123
Copy link
Contributor Author

Thanks for merging!

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

3 participants