We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

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

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.

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

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

Thanks!

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!

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

Created attachment 1870 [details]
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

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).

(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

Created attachment 1875 [details]
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

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!

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.

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.

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.

(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.

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.

Created attachment 2083 [details]
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

(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.

> 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.

(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.

(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 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.

(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

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.

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

Created attachment 2091 [details]
Renamed SDLGenericMotionListener back to SDLGenericMotionListener_API12

The atached patch renames SDLGenericMotionListener back to SDLGenericMotionListener_API12.

Joseba

Created attachment 2092 [details]
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

> The atached patch renames SDLGenericMotionListener back to
> SDLGenericMotionListener_API12.

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

--ryan.

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.

(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.

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).

(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.

Created attachment 2101 [details]
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.

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.

(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.

(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.

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.

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

(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.

(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).

(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.