Suggested edits for SDL android implementation

Hi all

I’ve been looking at the SDL_android.c implementation and due to the way
android deals with lifecycle events the current implementation uses static
but there are a few places where the android code leaks out of
SDL_android.c.

Example:
ANativeWindow* Android_JNI_GetNativeWindow(void)
{
ANativeWindow* anw;
jobject s;
JNIEnv *env = Android_JNI_GetEnv();
s = (*env)->CallStaticObjectMethod(env, mActivityClass,
midGetNativeSurface);
anw = ANativeWindow_fromSurface(env, s);
(*env)->DeleteLocalRef(env, s);

return anw;

}

This function is also used in another android
module:src/video/android/SDL_androidwindow.c line 66

data->native_window = Android_JNI_GetNativeWindow();

Wouldn’t it be easier to maintain if we moved this window initialization
code back into the main SDL_android.c?

Is there an android maintainer on this project?

Hi! AFAIK there is no “android maintainer”. Ryan and Sam are the most
active in the project, with occasional commits from others. I’ve been
looking at your discussions and suggestions, and I think you should open
bug reports to them, providing patches so that everyone else can test what
you’re doing.

This is usually how you get code accepted, it seems. It can take a long
time, though, which makes me guess if its not time for SDL to move to
another development model (something more open and easier to accept
patches).Em dom, 20 de set de 2015 ?s 11:21, Owen Alanzo Hogarth escreveu:

Hi all

I’ve been looking at the SDL_android.c implementation and due to the way
android deals with lifecycle events the current implementation uses static
but there are a few places where the android code leaks out of
SDL_android.c.

Example:
ANativeWindow* Android_JNI_GetNativeWindow(void)
{
ANativeWindow* anw;
jobject s;
JNIEnv *env = Android_JNI_GetEnv();
s = (*env)->CallStaticObjectMethod(env, mActivityClass,
midGetNativeSurface);
anw = ANativeWindow_fromSurface(env, s);
(*env)->DeleteLocalRef(env, s);

return anw;

}

This function is also used in another android
module:src/video/android/SDL_androidwindow.c line 66

data->native_window = Android_JNI_GetNativeWindow();

Wouldn’t it be easier to maintain if we moved this window initialization
code back into the main SDL_android.c?

Is there an android maintainer on this project?


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

I’ve been digging around in SDL mostly android for now, it seems there’s
even some code in there that’s unused, probably from the SDL 1.x days

How do I submit bug reports and submit patches? It’s something that I
wouldn’t mind doing. I don’t want to take SDL too far away from it’s core
because it does so much right already, I just think there’s maybe not a
lot of android users doing what I am trying to do with SDL.On Mon, Sep 21, 2015 at 12:12 AM, Leonardo Guilherme < leonardo.guilherme at gmail.com> wrote:

Hi! AFAIK there is no “android maintainer”. Ryan and Sam are the most
active in the project, with occasional commits from others. I’ve been
looking at your discussions and suggestions, and I think you should open
bug reports to them, providing patches so that everyone else can test what
you’re doing.

This is usually how you get code accepted, it seems. It can take a long
time, though, which makes me guess if its not time for SDL to move to
another development model (something more open and easier to accept
patches).

Em dom, 20 de set de 2015 ?s 11:21, Owen Alanzo Hogarth < @Owen_Alanzo_Hogarth> escreveu:

Hi all

I’ve been looking at the SDL_android.c implementation and due to the way
android deals with lifecycle events the current implementation uses static
but there are a few places where the android code leaks out of
SDL_android.c.

Example:
ANativeWindow* Android_JNI_GetNativeWindow(void)
{
ANativeWindow* anw;
jobject s;
JNIEnv *env = Android_JNI_GetEnv();
s = (*env)->CallStaticObjectMethod(env, mActivityClass,
midGetNativeSurface);
anw = ANativeWindow_fromSurface(env, s);
(*env)->DeleteLocalRef(env, s);

return anw;

}

This function is also used in another android
module:src/video/android/SDL_androidwindow.c line 66

data->native_window = Android_JNI_GetNativeWindow();

Wouldn’t it be easier to maintain if we moved this window initialization
code back into the main SDL_android.c?

Is there an android maintainer on this project?


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

Philipp Wiesemann does most of the Android work nowadays, I can help out as
well. If you have a recommended change, file a bug in bugzilla.libsdl.org
with a patch.
In the particular case you mention below, I doubt it will be accepted,
there’s some common structure to how all the video backends are organized
and I think this particular detail you mention is due to that.

2015-09-20 11:21 GMT-03:00 Owen Alanzo Hogarth :> Hi all

I’ve been looking at the SDL_android.c implementation and due to the way
android deals with lifecycle events the current implementation uses static
but there are a few places where the android code leaks out of
SDL_android.c.

Example:
ANativeWindow* Android_JNI_GetNativeWindow(void)
{
ANativeWindow* anw;
jobject s;
JNIEnv *env = Android_JNI_GetEnv();
s = (*env)->CallStaticObjectMethod(env, mActivityClass,
midGetNativeSurface);
anw = ANativeWindow_fromSurface(env, s);
(*env)->DeleteLocalRef(env, s);

return anw;

}

This function is also used in another android
module:src/video/android/SDL_androidwindow.c line 66

data->native_window = Android_JNI_GetNativeWindow();

Wouldn’t it be easier to maintain if we moved this window initialization
code back into the main SDL_android.c?

Is there an android maintainer on this project?


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org


Gabriel.

Hi

Thanks for the feedback, I was able to sort out a moving some of the
backend stuff by adjusting the way I deal with JNI in SDL_android.c

One question though, it seems that the function:

JNIEXPORT void JNICALL
Java_org_libsdl_app_SDLActivity_nativeFlipBuffers(JNIEnv* env, jclass jcls)

{

SDL_GL_SwapWindow(Android_Window);

}

is never actually called. I think it might be some dead code from the sdl
1.x days where you guys did the egl handling, maybe I wasn’t around back
then.

I’ve traced many applications and it’s not being called. Although looking
the source with grep:

Brother:SDL blubee$ grep -rn “SDL_GL_SwapWindow” .

./_SDL_android.c:306: SDL_GL_SwapWindow(Android_Window);

./docs/README-ios.md:147:- The drawable Renderbuffer must be bound to the
GL_RENDERBUFFER binding point when SDL_GL_SwapWindow is called.

./docs/README-ios.md:148:- The drawable Framebuffer Object must be bound
while rendering to the screen and when SDL_GL_SwapWindow is called.

./include/SDL_syswm.h:234: GLuint colorbuffer; /* The GL view’s
color Renderbuffer Object. It must be bound when SDL_GL_SwapWindow is
called. */

./include/SDL_video.h:1060:extern DECLSPEC void SDLCALL
SDL_GL_SwapWindow(SDL_Window * window);

./lwp.c:420: SDL_GL_SwapWindow(Android_Window);

./SDL_android.c:404: SDL_GL_SwapWindow(Android_Window);

./src/core/android/SDL_android.c:273: SDL_GL_SwapWindow(Android_Window);

./src/dynapi/SDL_dynapi_overrides.h:570:#define SDL_GL_SwapWindow
SDL_GL_SwapWindow_REAL

./src/dynapi/SDL_dynapi_procs.h:598:SDL_DYNAPI_PROC(void,SDL_GL_SwapWindow,(SDL_Window
*a),(a),)

./src/render/opengl/SDL_render_gl.c:1473:
SDL_GL_SwapWindow(renderer->window);

./src/render/opengles/SDL_render_gles.c:1170:
SDL_GL_SwapWindow(renderer->window);

./src/render/opengles2/SDL_render_gles2.c:1872:
SDL_GL_SwapWindow(renderer->window);

./src/video/SDL_video.c:3177:SDL_GL_SwapWindow(SDL_Window * window)

./src/video/uikit/SDL_uikitappdelegate.m:342: * displayed (e.g. if
resources are loaded before SDL_GL_SwapWindow is

./test/testgl2.c:393: SDL_GL_SwapWindow(state->windows[i]);

./test/testgles.c:308:
SDL_GL_SwapWindow(state->windows[i]);

./test/testgles.c:328: SDL_GL_SwapWindow(state->windows[i]);

./test/testgles2.c:447:
SDL_GL_SwapWindow(state->windows[i]);

./test/testgles2.c:466: SDL_GL_SwapWindow(state->windows[i]);

./test/testshader.c:413: SDL_GL_SwapWindow(window);

./Xcode-iOS/Demos/src/fireworks.c:459: SDL_GL_SwapWindow(window);

I can see it is referenced in a few .c files, what’s the deal with that
function? Is it only for ios?

Could it be safely removed from the SDL_android.c?On Mon, Sep 21, 2015 at 7:43 PM, Gabriel Jacobo wrote:

Philipp Wiesemann does most of the Android work nowadays, I can help out
as well. If you have a recommended change, file a bug in
bugzilla.libsdl.org with a patch.
In the particular case you mention below, I doubt it will be accepted,
there’s some common structure to how all the video backends are organized
and I think this particular detail you mention is due to that.

2015-09-20 11:21 GMT-03:00 Owen Alanzo Hogarth <@Owen_Alanzo_Hogarth>:

Hi all

I’ve been looking at the SDL_android.c implementation and due to the way
android deals with lifecycle events the current implementation uses static
but there are a few places where the android code leaks out of
SDL_android.c.

Example:
ANativeWindow* Android_JNI_GetNativeWindow(void)
{
ANativeWindow* anw;
jobject s;
JNIEnv *env = Android_JNI_GetEnv();
s = (*env)->CallStaticObjectMethod(env, mActivityClass,
midGetNativeSurface);
anw = ANativeWindow_fromSurface(env, s);
(*env)->DeleteLocalRef(env, s);

return anw;

}

This function is also used in another android
module:src/video/android/SDL_androidwindow.c line 66

data->native_window = Android_JNI_GetNativeWindow();

Wouldn’t it be easier to maintain if we moved this window initialization
code back into the main SDL_android.c?

Is there an android maintainer on this project?


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org


Gabriel.


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

Hi,

After a quick look, I think Owen is right:
those “flipBuffer” functions does not seem to be used (also
Android_JNI_SwapWindow is not used).

files:
android-project/src/org/libsdl/app/SDLActivity.java
src/core/android/SDL_android.c
src/core/android/SDL_android.h

Cheers,
SylvainOn 21 September 2015 at 14:09, Owen Alanzo Hogarth wrote:

Hi

Thanks for the feedback, I was able to sort out a moving some of the
backend stuff by adjusting the way I deal with JNI in SDL_android.c

One question though, it seems that the function:

JNIEXPORT void JNICALL
Java_org_libsdl_app_SDLActivity_nativeFlipBuffers(JNIEnv* env, jclass jcls)

{

SDL_GL_SwapWindow(Android_Window);

}

is never actually called. I think it might be some dead code from the sdl
1.x days where you guys did the egl handling, maybe I wasn’t around back
then.

I’ve traced many applications and it’s not being called. Although looking
the source with grep:

Brother:SDL blubee$ grep -rn “SDL_GL_SwapWindow” .

./_SDL_android.c:306: SDL_GL_SwapWindow(Android_Window);

./docs/README-ios.md:147:- The drawable Renderbuffer must be bound to the
GL_RENDERBUFFER binding point when SDL_GL_SwapWindow is called.

./docs/README-ios.md:148:- The drawable Framebuffer Object must be bound
while rendering to the screen and when SDL_GL_SwapWindow is called.

./include/SDL_syswm.h:234: GLuint colorbuffer; /* The GL view’s
color Renderbuffer Object. It must be bound when SDL_GL_SwapWindow is
called. */

./include/SDL_video.h:1060:extern DECLSPEC void SDLCALL
SDL_GL_SwapWindow(SDL_Window * window);

./lwp.c:420: SDL_GL_SwapWindow(Android_Window);

./SDL_android.c:404: SDL_GL_SwapWindow(Android_Window);

./src/core/android/SDL_android.c:273: SDL_GL_SwapWindow(Android_Window);

./src/dynapi/SDL_dynapi_overrides.h:570:#define SDL_GL_SwapWindow
SDL_GL_SwapWindow_REAL

./src/dynapi/SDL_dynapi_procs.h:598:SDL_DYNAPI_PROC(void,SDL_GL_SwapWindow,(SDL_Window
*a),(a),)

./src/render/opengl/SDL_render_gl.c:1473:
SDL_GL_SwapWindow(renderer->window);

./src/render/opengles/SDL_render_gles.c:1170:
SDL_GL_SwapWindow(renderer->window);

./src/render/opengles2/SDL_render_gles2.c:1872:
SDL_GL_SwapWindow(renderer->window);

./src/video/SDL_video.c:3177:SDL_GL_SwapWindow(SDL_Window * window)

./src/video/uikit/SDL_uikitappdelegate.m:342: * displayed (e.g. if
resources are loaded before SDL_GL_SwapWindow is

./test/testgl2.c:393: SDL_GL_SwapWindow(state->windows[i]);

./test/testgles.c:308:
SDL_GL_SwapWindow(state->windows[i]);

./test/testgles.c:328: SDL_GL_SwapWindow(state->windows[i]);

./test/testgles2.c:447:
SDL_GL_SwapWindow(state->windows[i]);

./test/testgles2.c:466: SDL_GL_SwapWindow(state->windows[i]);

./test/testshader.c:413: SDL_GL_SwapWindow(window);

./Xcode-iOS/Demos/src/fireworks.c:459: SDL_GL_SwapWindow(window);

I can see it is referenced in a few .c files, what’s the deal with that
function? Is it only for ios?

Could it be safely removed from the SDL_android.c?

On Mon, Sep 21, 2015 at 7:43 PM, Gabriel Jacobo wrote:

Philipp Wiesemann does most of the Android work nowadays, I can help out
as well. If you have a recommended change, file a bug in
bugzilla.libsdl.org with a patch.
In the particular case you mention below, I doubt it will be accepted,
there’s some common structure to how all the video backends are organized
and I think this particular detail you mention is due to that.

2015-09-20 11:21 GMT-03:00 Owen Alanzo Hogarth :

Hi all

I’ve been looking at the SDL_android.c implementation and due to the way
android deals with lifecycle events the current implementation uses static
but there are a few places where the android code leaks out of
SDL_android.c.

Example:
ANativeWindow* Android_JNI_GetNativeWindow(void)
{
ANativeWindow* anw;
jobject s;
JNIEnv *env = Android_JNI_GetEnv();
s = (*env)->CallStaticObjectMethod(env, mActivityClass,
midGetNativeSurface);
anw = ANativeWindow_fromSurface(env, s);
(*env)->DeleteLocalRef(env, s);

return anw;

}

This function is also used in another android
module:src/video/android/SDL_androidwindow.c line 66

data->native_window = Android_JNI_GetNativeWindow();

Wouldn’t it be easier to maintain if we moved this window initialization
code back into the main SDL_android.c?

Is there an android maintainer on this project?


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org


Gabriel.


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org


Sylvain Becker