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.

Bug 3917

Summary: Android, issues with getManifestEnvironmentVariable
Product: SDL Reporter: Sylvain <sylvain.becker>
Component: *don't know*Assignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2    
Version: don't know   
Hardware: All   
OS: Android (All)   
Attachments: patch
patch
patch
patch

Description Sylvain 2017-10-24 11:01:25 UTC 
Some issue with this commit:
https://hg.libsdl.org/SDL/rev/4130b92b6be4

There is a memory allocation missing.
And the example is wrong, it should be within "applications" markups.

I also noticed, that at each texture creation, there is call to get SDL_HINT_RENDER_SCALE_QUALITY and so a call this java function ...




diff -r 4130b92b6be4 android-project/app/src/main/AndroidManifest.xml
--- a/android-project/app/src/main/AndroidManifest.xml	Tue Oct 24 00:17:07 2017 -0700
+++ b/android-project/app/src/main/AndroidManifest.xml	Tue Oct 24 13:00:44 2017 +0200
@@ -8,10 +8,6 @@
     android:versionName="1.0"
     android:installLocation="auto">
 
-    <!-- Example of setting SDL hints from AndroidManifest.xml:
-    <meta-data android:value="0" android:name="SDL_ENV.SDL_ACCELEROMETER_AS_JOYSTICK"/>
-     -->
-     
     <!-- Android 4.0.1 -->
     <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="16" />
 
@@ -39,6 +35,11 @@
         android:allowBackup="true"
         android:theme="@android:style/Theme.NoTitleBar.Fullscreen"
         android:hardwareAccelerated="true" >
+
+        <!-- Example of setting SDL hints from AndroidManifest.xml:
+        <meta-data android:value="0" android:name="SDL_ENV.SDL_ACCELEROMETER_AS_JOYSTICK"/>
+        -->
+
         <activity android:name="SDLActivity"
             android:label="@string/app_name"
             android:configChanges="keyboardHidden|orientation|screenSize"
diff -r 4130b92b6be4 src/core/android/SDL_android.c
--- a/src/core/android/SDL_android.c	Tue Oct 24 00:17:07 2017 -0700
+++ b/src/core/android/SDL_android.c	Tue Oct 24 13:00:44 2017 +0200
@@ -2054,6 +2069,7 @@
 
     jstring jVariableName = (*env)->NewStringUTF(env, variableName);
     jstring jResult = (jstring)((*env)->CallStaticObjectMethod(env, mActivityClass, midGetManifestEnvironmentVariable, jVariableName));
+    (*env)->DeleteLocalRef(env, jVariableName);
 
     if (jResult == NULL) {
         return NULL;
Comment 1 Sam Lantinga 2017-11-02 02:21:00 UTC 
Your patches are in.

Regarding the environment variable handling, can they be pushed to native SDL_setenv() before running SDL_main()? Will they persist across activities if we do that?
Comment 2 Sylvain 2017-11-02 09:27:46 UTC 
Yes, we can read the Manifest Environment Variables in the onCreate() method and push them with SDL_setenv(). It's a much better option.

Especially because currently SDL_getenv() is broken: it doesn't get the real variable environments anymore (e.g. SDL_getenv("PATH"), for quick test, would return null).

Yes, if you start a second activity, you can get the environment variables you have set from the first activity.


Though, I am not sure that setting environment in AndroidManifest.xml is good option. It tends to make platform different.
Comment 3 Sam Lantinga 2017-11-02 15:54:03 UTC 
It's needed to fix race conditions with joystick initialization.

Can you implement a patch which pushes the variables with setenv()? It should be a static function on the SDL class and be called from SDL_JAVA_INTERFACE(nativeSetupJNI), since there are use cases where SDL is used without instantiating SDLActivity.

Thanks!
Comment 4 Sylvain 2017-11-02 16:41:32 UTC 
I just give a try, that does not seems to work because it requires a "Context", which mean an activity started !
Comment 5 Sylvain 2017-11-02 17:24:18 UTC 
Created attachment 3056 [details]
patch

This is a quick patch, it requires the context to be set before calling setupJNI(). That probably needs to be modified !
Comment 6 Sylvain 2017-11-03 08:36:38 UTC 
patch is also problematic because Os.setenv appears in API_21
Comment 7 Sylvain 2017-11-04 10:07:35 UTC 
Can't a race condition for joystick initialization be solved simply by adding a mutex ?
Comment 8 Sylvain 2017-11-04 13:56:30 UTC 
Created attachment 3065 [details]
patch

Updated patch so that it doesn't require java Os.setenv of API_21, but uses SDL_setenv() and native setenv() instead.
Comment 9 Sam Lantinga 2017-11-04 16:38:11 UTC 
Can you try this patch and see if that works?
https://hg.libsdl.org/SDL/rev/1f10a52295e3

It should allow querying all the environment variables at once, after the activity is set.
Comment 10 Sylvain 2017-11-04 18:47:39 UTC 
Created attachment 3067 [details]
patch

Yes this is working! Some "tabs" in the SDLActivity though.

What about getting some return code instead of creating another native function. But maybe this is to use from other classes. see patch.
Comment 11 Sylvain 2017-11-04 18:49:26 UTC 
Created attachment 3068 [details]
patch

patch updated - typo.


maybe also, write an explicit initialization : 

static SDL_bool bHasEnvironmentVariables = SDL_FALSE;
Comment 12 Sam Lantinga 2017-11-05 05:03:47 UTC 
This looks good, thanks!
https://hg.libsdl.org/SDL/rev/e4d90d54cb01