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

Patch for Android: protecting threads and multitouch #382

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

Patch for Android: protecting threads and multitouch #382

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Labels
duplicate This issue or pull request already exists

Comments

@SDLBugzilla
Copy link
Collaborator

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), ARM

Comments on the original bug report:

On 2011-03-02 09:22:30 +0000, Gueniffey wrote:

Attached is a patch that enables multitouch support for Android and fixes issues when the JNI environment is not accessible anymore(when calling from other threads or if it has been released)

On 2011-03-02 09:23:24 +0000, Gueniffey wrote:

Created attachment 593
Android multitouch + thread protection

On 2011-03-03 00:34:10 +0000, Gueniffey wrote:

Created attachment 594
Android multitouch + thread protection

The previous patch was messed out, sorry. Here is the right one.

On 2011-03-03 03:28:11 +0000, Gueniffey wrote:

Created attachment 595
Patch for Android multitouch + thread protection

New clean patch with everything working (sorry for the two first, which were messed up)

On 2011-03-05 09:58:27 +0000, Sam Lantinga wrote:

Thank you very much for your patch for SDL 1.3!

Do you give me permission to release your code with SDL 1.3 and future
versions of SDL under both the LGPL and a closed-source commercial
license?

On 2011-03-05 10:00:33 +0000, Gueniffey wrote:

(In reply to comment # 4)

Thank you very much for your patch for SDL 1.3!

Do you give me permission to release your code with SDL 1.3 and future
versions of SDL under both the LGPL and a closed-source commercial
license?

Yes of course, if you give me the permission to use your library under LGPL for our commercial Android application

On 2011-03-05 10:05:24 +0000, Sam Lantinga wrote:

Of course, as long as you comply with the LGPL license by dynamically linking, etc.

On 2011-03-05 10:07:24 +0000, Sam Lantinga wrote:

A few comments:

I noticed you updated the android target from android-4 (1.6) to android-7. This breaks compatibility with older Android phones. Was this intentional?

Your patch to attach and detach threads from the VM adds some overhead to the SDL calls. What was the problem you were fixing by doing this?

Thanks!

On 2011-03-07 00:21:48 +0000, Gueniffey wrote:

The multitouch APIs are only available since API level 5 (Android 2.1). You can downgrade the target to this (I should have done it). For older versions, the old code with mouse pointers will still work and is still available, developers will need to make a few changes in the java code if they need to support Android 1.6.

The thread attachment code doesn't add any overhead, since it's only called for when the calling thread is different from the main SDL thread. With the current code, it would crash, as the JNI environmemnt cannot be accessed from other threads. Moreover, it could be released from the main thread and this would also lead to a crash.
For more info, check this out:
http://www.google.fr/url?sa=t&source=web&cd=1&ved=0CBgQFjAA&url=http%3A%2F%2Fandroid.wooyd.org%2FJNIExample%2Ffiles%2FJNIExample.pdf&rct=j&q=android%20jni%20attachcurrentthread&ei=ZZR0TZyiCIaMswb2qZSEDg&usg=AFQjCNHwjbm09fLt2PAgFYDRjSKlL3mQTA&sig2=N7PRNFcRbEuoqhSWOdmNfA&cad=rja

On 2011-03-11 05:44:44 +0000, Gueniffey wrote:

API level 5 is android 2.0, forgive my mistake.

On 2011-03-11 15:58:31 +0000, Sam Lantinga wrote:

In the multi-touch code, any touch up will send touch up for all current touches. Is that intentional?

"The thread attachment code doesn't add any overhead, since it's only called for
when the calling thread is different from the main SDL thread."

Can any number of threads be attached to the VM? If so, is there any reason to call this after each time you attach?
mJVM->DetachCurrentThread();

Thanks!

On 2011-03-11 22:47:38 +0000, Gueniffey wrote:

(In reply to comment # 10)

In the multi-touch code, any touch up will send touch up for all current
touches. Is that intentional?

Nope. Maybe there's a bug in my code.

"The thread attachment code doesn't add any overhead, since it's only called
for
when the calling thread is different from the main SDL thread."

Can any number of threads be attached to the VM? If so, is there any reason to
call this after each time you attach?
mJVM->DetachCurrentThread();

You need to detach any attached thread before it ends. How can you detach it anywhere else ?

On 2011-03-11 23:50:56 +0000, Sam Lantinga wrote:

You could keep track of attached threads add maybe add code to the SDL thread cleanup to take care of it.

What is the original case that you were trying to solve?

On 2011-03-11 23:56:13 +0000, Gueniffey wrote:

Well, that's possible, but you can use threads that are not SDL threads, right? In that case the app would crash when the thread ends.
I need to call a SDL function that triggered Android code from a separate thread, and my app crashed, since the JNI environment is not available for other threads.

On 2011-03-19 14:14:55 +0000, Gueniffey wrote:

I don't think there's a need to modify the patch (except if there's a bug in the multitouch handling code). So I suggest that you add it to the trunk, as there is still a crash that can occur in the current android code. Maybe somebody could make changes later if the fix is faulty.

On 2012-06-21 07:55:40 +0000, Gabriel Jacobo wrote:

Multitouch for Android is now fully functional, there's a threading issue with RWops that's being dealt with in bug # 1422

*** This bug has been marked as a duplicate of bug 1422 ***

@SDLBugzilla SDLBugzilla added bug duplicate This issue or pull request already exists labels Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

1 participant