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

Handle mouse events in Android #720

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

Handle mouse events in Android #720

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 10, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: Android (All), Other

Comments on the original bug report:

On 2012-12-16 09:48:51 +0000, Joseba García Echebarria wrote:

Created attachment 999
Android mouse handling

The attached patch changes how SDL handles Touch events in Android to make a difference between SDL mouse presses and real touchpad/touchscreen touches.

It does not disable mouse emulation when the user touches the device.

On 2013-02-16 02:04:28 +0000, Philipp Wiesemann wrote:

The new mouse source from patch calls SDL_Log() if mouse button is pressed. Maybe this is unwanted overhead.

On 2014-09-06 20:15:52 +0000, Luiggi Reffatti wrote:

Just tested my game with SDL rev 9117, had the same problem.
I connected a mouse in my android device (Galaxy S4 Zoom) with an OTG cable, and I got a cursor and a working mouse.

Problem is that events genereted by the mouse are received as touch events, and not mouse events as expected. As my game handle both events in different ways, mouse events just don't work properly.
This issue is specially problematic in Android devices that are meant to be used with touchpad/mouse, as Asus Transformer.

SideNote (this is another issue): the functions that change mouse cursor on Windows and Linux doesn't seem to work on Android.

On 2014-09-11 22:55:37 +0000, Joseba García Echebarria wrote:

The attached patch is no longer valid but should be easily replaceable by a newer version that does pretty much the same. If there is interest, I can to update it for the latest version of SDL HG.

I strongly believe this is something SDL should do in Android (or any other platform) and there is a clear use case for the feature.
I could maybe add some SDL_HINT that controls if use and touch events should be treated together or separately.

Kind regards,
Joseba

On 2014-09-13 09:16:28 +0000, Sam Lantinga wrote:

Yes, please update the patch. I'd like to get it in for the 2.0.4 release.

Thanks!

On 2014-09-14 09:50:49 +0000, Joseba García Echebarria wrote:

All right, I'll try to upload it on Monday.

Kind regards,
Joseba

(In reply to Sam Lantinga from comment # 4)

Yes, please update the patch. I'd like to get it in for the 2.0.4 release.

Thanks!

On 2014-09-16 02:44:36 +0000, Joseba García Echebarria wrote:

Working on getting this working ASAP.

I'm working on giving the user as much info as possible in their OS level. So

  • In Android >= 9 the user can now that some button was pressed, as well as the mouse position when it's being clicked
  • In Android >= 12 the user can know the mouse position even if it's not being pressed
  • In Android >= 14 the user can know which button was pressed (but not which mouse button was released...)

Sorry for the delay

On 2014-09-17 03:16:36 +0000, Joseba García Echebarria wrote:

Created attachment 1870
New Android mouse handling patch

The attached patch adds the mentioned functionality.
Some notes:

  • Android >= 14 adds a way of knowing which mouse button press originated the associated event, but I cannot seem to find a way to know which mouse button release originated the associated event. Right now the code assumes that The mouse you pressed is the one being released.
  • All mouse events are assigned to mouse 0, which will not always be true.
  • I've added a new hint (SDL_ANDROID_SEPARATE_MOUSE_AND_TOUCH) which controls if mouse/touch events are to be handled just like until now or separately. By default the behaviour is not changed when compared to SDL 2.0.3. This means that all mouse button presses (either real or faked) are always mapped to the left mouse button. If you set the hint to 1, you get the new features.
  • I've compiled with Android SDK 12 and tested in Android 19 (4.4.2) with an Apple magic mouse connected through bluetooth to a NExus 5 phone. USB should work just as well.

Please let me know if any more changes are required.

Kind regards,
Joseba

On 2014-09-18 08:56:10 +0000, Philipp Wiesemann wrote:

Thank you for updating the patch.

In AndroidManifest.xml and the properties files the target needs to be changed from 12 to 14 or the Java part will not compile (because of getButtonState()). If this was changed also README-android.md would need to be updated.

In SDL_Activity.java and onTouch() the return value of nativeGetHint() is only needed if a mouse is used. Therefore it does not need to be called if the source of the MotionEvent is not a mouse which would prevent unnecessary overhead (assuming unnecessary JNI calls would slow the application down noticeably).

On 2014-09-18 19:25:20 +0000, Joseba García Echebarria wrote:

(In reply to Philipp Wiesemann from comment # 8)

Thank you for updating the patch.

In AndroidManifest.xml and the properties files the target needs to be
changed from 12 to 14 or the Java part will not compile (because of
getButtonState()). If this was changed also README-android.md would need to
be updated.

In SDL_Activity.java and onTouch() the return value of nativeGetHint() is
only needed if a mouse is used. Therefore it does not need to be called if
the source of the MotionEvent is not a mouse which would prevent unnecessary
overhead (assuming unnecessary JNI calls would slow the application down
noticeably).

Good evening,

Thanks for taking the time to review the patch.
About the SDK version: the compiler cache probably hid that. I'll update the patch.

I am on holidays and out of home until Sunday; I'll update the patch with your corrections then.

Cheers,
Joseba

On 2014-09-21 22:03:46 +0000, Joseba García Echebarria wrote:

Created attachment 1875
Updated patch with Philipp's suggestions

The attached patch addresses the issues Philipp mentioned.

I'm not very good with HG so I let SourceTree fo the job here, I hope everything is fine.

Cheers,
Joseba

PS: There does not seem to be an

On 2015-02-19 05:22:16 +0000, Ryan C. Gordon wrote:

Marking a large number of bugs with the "triage-2.0.4" keyword at once. Sorry if you got a lot of email from this. This is to help me sort through some bugs in regards to a 2.0.4 release. We may or may not fix this bug for 2.0.4, though!

On 2015-03-22 05:18:58 +0000, Ryan C. Gordon wrote:

Gabriel and Philipp, is this patch acceptable, or does it need more work? I'd like to close this out for 2.0.4 if possible.

Thanks,
--ryan.

On 2015-03-22 21:38:13 +0000, Philipp Wiesemann wrote:

I have not tested or compiled this. From just reading, it looks useful.

SDL_androidmouse.c includes "SDL_Log.h" but it should be "SDL_log.h" (or not at all because there seems to be no logging).

The project.properties file maybe needs to be changed because it still contains "target=android-12".

There are some unrelated changes (whitespace, replacing constants, renaming a class) which I think are not needed. But it looks like it does not break anything.

On 2015-03-23 07:16:44 +0000, Philipp Wiesemann wrote:

Changing the targetSdkVersion version in the AndroidManifest.xml always silently changes some of Android's compatibility settings. [1] These settings could be implicitly required for some applications to work.

Maybe the API level 12 parts from the patch could be added before SDL 2.0.4 and the API level 14 parts [2] could be added afterwards. This would prevent breaking some things (in case there are) which may not be obvious with only short tests on few devices (and the basic AndroidManifest.xml which is included in SDL).

API level 12 was used since SDL 2.0.2/2.0.3.

[1] http://developer.android.com/guide/topics/manifest/uses-sdk-element.html#target
[2] This seems to be the support for more than one button only.

On 2015-03-24 06:28:15 +0000, Ryan C. Gordon wrote:

(In reply to Philipp Wiesemann from comment # 14)

Maybe the API level 12 parts from the patch could be added before SDL 2.0.4
and the API level 14 parts [2] could be added afterwards.

That's not a bad idea. If someone (Joseba? Gabriel? Philipp?) splits up the patch as such, we'll apply the level 12 parts now and the level 14 parts right after 2.0.4 ships. Otherwise, let's leave this patch alone until we start on 2.0.5.

--ryan.

On 2015-03-24 07:04:40 +0000, Joseba García Echebarria wrote:

Good morning,

Thanks for having a look at the patch. Your idea seems reasonable.
I can split it this evening and upload the resulting patches here.

Joseba

(In reply to Ryan Gordon from comment # 15)

(In reply to Philipp Wiesemann from comment # 14)

Maybe the API level 12 parts from the patch could be added before SDL 2.0.4
and the API level 14 parts [2] could be added afterwards.

That's not a bad idea. If someone (Joseba? Gabriel? Philipp?) splits up the
patch as such, we'll apply the level 12 parts now and the level 14 parts
right after 2.0.4 ships. Otherwise, let's leave this patch alone until we
start on 2.0.5.

--ryan.

On 2015-03-24 20:14:18 +0000, Joseba García Echebarria wrote:

Created attachment 2083
Pacth for Android API 12

Good evening.

The new patch should include the changes in an Android API 12 compatible way. Android API 14 is needed only to determine which mouse button was pressed but that code is no longer present in the Java code.
I believe I hadn't mentioned that this code also adds support for hover events: now you can now where the mouse is even if the user is not clicking.

-- Joseba

On 2015-03-25 14:48:07 +0000, Ryan C. Gordon wrote:

(In reply to Joseba García Echebarria from comment # 17)

The new patch should include the changes in an Android API 12 compatible
way. Android API 14 is needed only to determine which mouse button was
pressed but that code is no longer present in the Java code.
I believe I hadn't mentioned that this code also adds support for hover
events: now you can now where the mouse is even if the user is not clicking.

These are in revision control now, as https://hg.libsdl.org/SDL/rev/fff5af5de6dd and https://hg.libsdl.org/SDL/rev/0d01b53cdbea, thanks!

I'd like to add a hint watcher instead of looking it up for literally every mouse event, so I'll leave this bug open while I do that, but I wanted to make sure this patch made it in for 2.0.4 in any case.

Philipp and Gabriel, if you have any other concerns, feel free to push changes directly. :)

--ryan.

On 2015-03-25 15:01:59 +0000, Ryan C. Gordon wrote:

I'd like to add a hint watcher instead of looking it up for literally every
mouse event, so I'll leave this bug open while I do that, but I wanted to
make sure this patch made it in for 2.0.4 in any case.

This uses a hint watcher callback now, as of https://hg.libsdl.org/SDL/rev/fc7522030729 ...

Is there anything else that needs to be done? The API 14 parts became unnecessary, if I understand correctly? (I'll close the bug if that's the case, otherwise, we'll leave it open until after 2.0.4 ships).

--ryan.

On 2015-03-25 21:43:53 +0000, Philipp Wiesemann wrote:

(In reply to Ryan C. Gordon from comment # 19)

Is there anything else that needs to be done? The API 14 parts became
unnecessary, if I understand correctly?

With API level 14 it is possible to check which mouse button was actually pressed.

On 2015-03-25 21:48:17 +0000, Joseba García Echebarria wrote:

(In reply to Ryan C. Gordon from comment # 19)

I'd like to add a hint watcher instead of looking it up for literally every
mouse event, so I'll leave this bug open while I do that, but I wanted to
make sure this patch made it in for 2.0.4 in any case.

This uses a hint watcher callback now, as of
https://hg.libsdl.org/SDL/rev/fc7522030729 ...

Is there anything else that needs to be done? The API 14 parts became
unnecessary, if I understand correctly? (I'll close the bug if that's the
case, otherwise, we'll leave it open until after 2.0.4 ships).

--ryan.

Thank you very much for integrating the patch. I think it can be useful for others, too.
On API14 you can know which moose button was pressed, as Philipp mentions.
I'd say that is the main feature required from android API 14.

Joseba

On 2015-03-25 22:36:04 +0000, Ryan C. Gordon wrote:

On API14 you can know which moose button was pressed, as Philipp mentions.
I'd say that is the main feature required from android API 14.

This might be a silly question, but how hard is it to do some sort of introspection thing, so this can be used on newer Android devices but fall back to the API 12 behavior on older units?

(if that's impossible or just obnoxious, don't worry about it.)

--ryan.

On 2015-03-25 23:14:46 +0000, Joseba García Echebarria wrote:

(In reply to Ryan C. Gordon from comment # 22)

On API14 you can know which moose button was pressed, as Philipp mentions.
I'd say that is the main feature required from android API 14.

This might be a silly question, but how hard is it to do some sort of
introspection thing, so this can be used on newer Android devices but fall
back to the API 12 behavior on older units?

(if that's impossible or just obnoxious, don't worry about it.)

--ryan.

Hadn't really thought about it. I've done some quick research and it seems to not be too hard.
The following code is wrong, but I think the final one should look something like this:
Some more info here:
https://developer.android.com/reference/java/lang/Class.html#getMethod(java.lang.String, java.lang.Class<?>...)


public boolean onTouch(View v, MotionEvent event) {
    Method getButtonState;
    [...]

    if (event.getSource() == InputDevice.SOURCE_MOUSE &&
        SDLActivity.nativeGetHint("SDL_ANDROID_SEPARATE_MOUSE_AND_TOUCH").equals("1")) {
            mouseButton = 1;    // For Android==12 all mouse buttons are the left button
            try{
                getButtonState = event.getClass().getMethod("getButtonState", null);
                // Not sure if I can invoke the method this way or I must do
                // getButtonState.invoke(null, null)
                mouseButton = getButtonState();
            } catch(Exception e) {
                // It's fine if the method is not available
            }
            SDLActivity.onNativeMouse(mouseButton, action, event.getX(0), event.getY(0));
     [...]

I'll try to look into it tomorrow.
Joseba

On 2015-03-25 23:26:46 +0000, Gabriel Jacobo wrote:

The SDLACtivity.java code uses this mechanism in several places. Look for "Build.VERSION.SDK_INT"

BTW, you renamed SDLGenericMotionListener_API12 which made obvious the API level it was designed for. It should be probably be put back as it was and then add SDLGenericMotionListener_API14 including the code that detects which button was pressed.

On 2015-03-26 08:28:45 +0000, Joseba García Echebarria wrote:

Good morning,

(In reply to Gabriel Jacobo from comment # 24)

The SDLACtivity.java code uses this mechanism in several places. Look for
"Build.VERSION.SDK_INT"
I believe that's not the case. By checking the VERSION.SDK_INT you don't avoid needing to compile with Android 14. By using introspection, you can use features from android 14 and still only compile with SDK 12 at the cost of having "uglier" code.
The main potential advantage is that newer versions of android will not try to emulate bugs in SDK12.
While I agree that it's a bad idea to bump the compilation SDK version so close to releasing 2.0.4, IMHO, the idea of switching to SDK 14 after release makes more sense; potential behavior changes in Android should arise during the next development cycle and there should be time to fix them.

That said, I'm going to try to give introspection a try this evening, even if it's just for curiosity.

BTW, you renamed SDLGenericMotionListener_API12 which made obvious the API
level it was designed for. It should be probably be put back as it was and
then add SDLGenericMotionListener_API14 including the code that detects
which button was pressed.
Yes, I did change that. No real reason there, I was just comparing that code with some other branch of mine that doesn't include version numbers.

Regards
Joseba

On 2015-03-26 22:59:53 +0000, Joseba García Echebarria wrote:

Created attachment 2091
Renamed SDLGenericMotionListener back to SDLGenericMotionListener_API12

The atached patch renames SDLGenericMotionListener back to SDLGenericMotionListener_API12.

Joseba

On 2015-03-26 23:28:23 +0000, Joseba García Echebarria wrote:

Created attachment 2092
Introspection support for getButtonState

The attached pathc should add the proposed introspection for determining which mouse button originated a mouse press event.

Please note that I haven't been able to test the code, since the program I usually test this stuff on is horribly broken and not compiling right now, so sorry in advance if some other minor change is needed.

Still, as I said earlier, I believe this should only be regarded as a temporary meassure.
Android version 3.x (API 12) is an Android version with a tiny user base (not even listed in Google's Android usage stats*) and compatibility with older Android versions can still be achieved even after bumping the compile-time SDK version. That's only my opinion, of course, and I haven't thoroughly tested what the effects of changing the build SDK version are.

Joseba

https://developer.android.com/about/dashboards/index.html

On 2015-03-27 22:11:28 +0000, Ryan C. Gordon wrote:

The atached patch renames SDLGenericMotionListener back to
SDLGenericMotionListener_API12.

This is now https://hg.libsdl.org/SDL/rev/17095a4130ce

--ryan.

On 2015-03-31 00:16:18 +0000, Ryan C. Gordon wrote:

Gabriel/Philipp:

We've got this in the touch event handler in SDLActivity.java...
SDLActivity.nativeGetHint("SDL_ANDROID_SEPARATE_MOUSE_AND_TOUCH").equals("1")) {

I've got a callback that fires in the native code whenever this value changes, and I'd like to set a global variable or call a Java method from that callback, so the touch handler just have to check a boolean instead of looking up the hint through JNI each time. What's the proper way to do this?

--ryan.

On 2015-03-31 00:16:50 +0000, Ryan C. Gordon wrote:

(In reply to Joseba García Echebarria from comment # 27)

Created attachment 2092 [details]
Introspection support for getButtonState

(I'll get this patch in there soon, too.)

--ryan.

On 2015-03-31 12:31:55 +0000, Gabriel Jacobo wrote:

Let's hold off on getting this last patch in after 2.0.4, I don't think it's the right way to do it (at least it doesn't match the style of other introspection thingies we do in there).

On 2015-03-31 20:14:22 +0000, Philipp Wiesemann wrote:

(In reply to Ryan C. Gordon from comment # 29)

I've got a callback that fires in the native code whenever this value
changes, and I'd like to set a global variable or call a Java method from
that callback, so the touch handler just have to check a boolean instead of
looking up the hint through JNI each time. What's the proper way to do this?

I do not think there is a proper way to do this. :(

The hint callback could use Android_JNI_SendMessage() from SDL_android.c to sent something to the Android UI thread. The message could be handled in handleMessage() of SDLCommandHandler and SDLActivity.java. A boolean member variable of SDLActivity could be set there and later be checked in onTouch().
Both handleMessage() and onTouch() run on the UI thread which prevents race conditions (hint callback being in a different thread).

The nativeGetIntHint() is only called in onTouch() if the device has a mouse attached. Assuming that most Android devices do not, maybe implementing something complicated is currently not worth the effort.

Currently a Java String is created (in SDL_android.c) for every call to nativeGetHint() (from SDLActivity.java). The String (containing a number) is then only passed to Java to be compared to a constant String containing a number. This overhead could be removed by adding a "public static native int nativeGetIntHint()" (or similar). It could return the hint converted to an int.
This method could also be used in openAPKExtensionInputStream(). Although it may not matter there.

On 2015-04-01 03:37:26 +0000, Ryan C. Gordon wrote:

Created attachment 2101
Rough idea on the hint callback vs Java...

The nativeGetIntHint() is only called in onTouch() if the device has a mouse
attached. Assuming that most Android devices do not, maybe implementing
something complicated is currently not worth the effort.

I've attached the rough idea of what I'd like to do here; I haven't compiled this or anything. Is this not a reasonable way to get this information passed to Java?

After this, the call to the nativeGetHint() in onTouch() just becomes a check of a static boolean in SDLActivity.

I'm not deeply concerned if this variable races; it would likely only set once by most apps, at startup, and even if it changes mid-event, the risk is getting one errant touch/mouse event added to the queue.

I haven't really thought this through, and don't know the specifics of the system in any case, so let me know if this is just a really bad idea.

--ryan.

On 2015-04-01 14:18:29 +0000, Gabriel Jacobo wrote:

Looks good to me Ryan. The only nitpick would be to label the variable as "externally" settable with a hint in a comment, just in case someone is looking at the Java code and wondering how they are supposed to change that value.

On 2015-04-01 18:51:19 +0000, Ryan C. Gordon wrote:

(In reply to Gabriel Jacobo from comment # 34)

Looks good to me Ryan. The only nitpick would be to label the variable as
"externally" settable with a hint in a comment, just in case someone is
looking at the Java code and wondering how they are supposed to change that
value.

I put this in revision control...

https://hg.libsdl.org/SDL/rev/a65088766e35

...plus one to fix my screwups...

https://hg.libsdl.org/SDL/rev/bcb16ffce95b

I just punted and put the API14 try/catch in there. Unless it's a serious problem, let's rip it out after 2.0.4 and bump the minimum API to 14.

I haven't tested any of this at all, so I'd appreciate if someone could try this out. If any of this is really a terrible idea, I won't be offended if someone pushes something to revert it.

--ryan.

On 2015-04-01 18:52:13 +0000, Ryan C. Gordon wrote:

(In reply to Ryan C. Gordon from comment # 35)

I haven't tested any of this at all, so I'd appreciate if someone could
try this out. If any of this is really a terrible idea, I won't be offended
if someone pushes something to revert it.

(But if this is okay, we can officially resolve this bug.)

--ryan.

On 2015-04-01 23:02:54 +0000, Joseba García Echebarria wrote:

Created attachment 2105
Cast Object to Integer for getButtonState

I've just tested the code in HG and it crashes when the hint is active. The fix is simple: I wasn't casting the Object returned by getMethod().invoke() to an integer. The attached patch should fix this.

I've created a small test APK, in case you want to see it in action. Nothing spectacular, really.
https://www.dropbox.com/s/mt1m5flyojz5tlx/MyGame-SDL2Mouse.apk?dl=0

Thanks for having a look and for improving my implementation,
Joseba

On 2015-04-02 01:48:22 +0000, Ryan C. Gordon wrote:

(In reply to Joseba García Echebarria from comment # 37)

Created attachment 2105 [details]
Cast Object to Integer for getButtonState

I've just tested the code in HG and it crashes when the hint is active. The
fix is simple: I wasn't casting the Object returned by getMethod().invoke()
to an integer. The attached patch should fix this.

This is now https://hg.libsdl.org/SDL/rev/49a34578d9e7, thanks!

So I think we're done here now, right? Gabriel, Philipp?

--ryan.

On 2015-04-02 20:21:36 +0000, Philipp Wiesemann wrote:

(In reply to Ryan C. Gordon from comment # 38)

So I think we're done here now, right? Gabriel, Philipp?

I think it was implemented and the bug could be resolved. I can not test real mouse events myself. But it did not crash without a mouse. :)

For SDL_ShowCursor(), SDL_SetCursor() and similar functions a new bug could be created (see comment from 2014-09-06).

On 2015-04-06 23:30:31 +0000, Ryan C. Gordon wrote:

(In reply to Philipp Wiesemann from comment # 39)

(In reply to Ryan C. Gordon from comment # 38)

So I think we're done here now, right? Gabriel, Philipp?

I think it was implemented and the bug could be resolved. I can not test
real mouse events myself. But it did not crash without a mouse. :)

For SDL_ShowCursor(), SDL_SetCursor() and similar functions a new bug could
be created (see comment from 2014-09-06).

Okay, I've opened Bug # 2930 to handle the overflow (API bump, missing cursor, etc), and closing this bug.

Thanks everyone!

--ryan.

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

No branches or pull requests

1 participant