Permalink
Browse files

front: Fixed GL context restoring

  • Loading branch information...
Gillou68310 committed May 13, 2015
1 parent daa6c29 commit 1b8f7729c401f45f7ccf83e937740fd8d21a6181
@@ -23,7 +23,7 @@
#if SDL_VIDEO_DRIVER_ANDROID
/* We're going to do this by default */
#define SDL_ANDROID_BLOCK_ON_PAUSE 1
//#define SDL_ANDROID_BLOCK_ON_PAUSE 1
#include "SDL_androidevents.h"
#include "SDL_events.h"
@@ -55,6 +55,8 @@ typedef void (*pSdlSetScreen) (int width, int height, Uint32 format);
typedef void (*pVoidFunc) ();
typedef m64p_error (*pCoreDoCommand) (m64p_command, int, void *);
typedef int (*pFrontMain) (int argc, char* argv[]);
typedef void (*pNativeResume) (JNIEnv* env, jclass cls);
typedef void (*pNativePause) (JNIEnv* env, jclass cls);
// Function pointers
static pAeiInit aeiInit = NULL;
@@ -63,6 +65,8 @@ static pSdlSetScreen sdlSetScreen = NULL;
static pVoidFunc sdlMainReady = NULL;
static pCoreDoCommand coreDoCommand = NULL;
static pFrontMain frontMain = NULL;
static pNativeResume nativeResume = NULL;
static pNativePause nativePause = NULL;
void checkLibraryError(const char* message)
{
@@ -160,9 +164,11 @@ extern "C" DECLSPEC void SDLCALL Java_paulscode_android_mupen64plusae_jni_Native
sdlMainReady = (pVoidFunc) locateFunction(handleSDL, "SDL2", "SDL_SetMainReady");
coreDoCommand = (pCoreDoCommand) locateFunction(handleCore, "mupen64plus-core", "CoreDoCommand");
frontMain = (pFrontMain) locateFunction(handleFront, "mupen64plus-ui-console", "SDL_main");
nativeResume = (pNativeResume) locateFunction(handleSDL, "SDL2", "Java_org_libsdl_app_SDLActivity_nativeResume");
nativePause = (pNativePause) locateFunction(handleSDL, "SDL2", "Java_org_libsdl_app_SDLActivity_nativePause");

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 May 13, 2015

Member

These function handles will never be found.

@littleguy77

littleguy77 May 13, 2015

Member

These function handles will never be found.

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 May 13, 2015

Member

These java functions do not exist in the mupen64plus-ae repository, except in some SDL example code that currently is not built into mupen64plus-ae APK. I don't understand why this code isn't throwing error messages for you?

@littleguy77

littleguy77 May 13, 2015

Member

These java functions do not exist in the mupen64plus-ae repository, except in some SDL example code that currently is not built into mupen64plus-ae APK. I don't understand why this code isn't throwing error messages for you?

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 May 13, 2015

Member

Oh wait, Java_org_libsdl_app_SDLActivity_nativePause is the name of a native C/C++ method, not a Java method. Now I get it. Sorry for the confusion.

@littleguy77

littleguy77 May 13, 2015

Member

Oh wait, Java_org_libsdl_app_SDLActivity_nativePause is the name of a native C/C++ method, not a Java method. Now I get it. Sorry for the confusion.

// Make sure we don't have any typos
if (!aeiInit || !sdlInit || !sdlSetScreen || !sdlMainReady || !coreDoCommand || !frontMain)
if (!aeiInit || !sdlInit || !sdlSetScreen || !sdlMainReady || !coreDoCommand || !frontMain || !nativeResume || !nativePause)
{
LOGE("Could not load library functions: be sure they are named and typedef'd correctly");
}
@@ -188,6 +194,8 @@ extern "C" DECLSPEC void SDLCALL Java_paulscode_android_mupen64plusae_jni_Native
sdlMainReady = NULL;
coreDoCommand = NULL;
frontMain = NULL;
nativeResume = NULL;
nativePause = NULL;
// Close shared libraries
unloadLibrary(handleFront, "mupen64plus-ui-console");
@@ -242,12 +250,14 @@ extern "C" DECLSPEC void Java_paulscode_android_mupen64plusae_jni_NativeExports_
extern "C" DECLSPEC void Java_paulscode_android_mupen64plusae_jni_NativeExports_emuResume(JNIEnv* env, jclass cls)
{
if (nativeResume) nativeResume(env, cls);
if (coreDoCommand) coreDoCommand(M64CMD_RESUME, 0, NULL);
}
extern "C" DECLSPEC void Java_paulscode_android_mupen64plusae_jni_NativeExports_emuPause(JNIEnv* env, jclass cls)
{
if (coreDoCommand) coreDoCommand(M64CMD_PAUSE, 0, NULL);
if (nativePause) nativePause(env, cls);
}
extern "C" DECLSPEC void Java_paulscode_android_mupen64plusae_jni_NativeExports_emuAdvanceFrame(JNIEnv* env, jclass cls)
@@ -280,8 +280,10 @@ public void onWindowFocusChanged( boolean hasFocus )
Log.i( "GameLifecycleHandler", "onWindowFocusChanged: " + hasFocus );
mIsFocused = hasFocus;
if( hasFocus )
{
hideSystemBars();
tryRunning();
tryRunning();
}
}
public void onPause()
@@ -298,7 +300,8 @@ public void surfaceDestroyed( SurfaceHolder holder )
{
Log.i( "GameLifecycleHandler", "surfaceDestroyed" );
mIsSurface = false;
tryStopping();
tryPausing();
mSurface.destroyGLSurface();
}
public void onStop()
@@ -281,6 +281,8 @@ public void onOptionsItemSelected( MenuItem item )
setIme();
break;
case R.id.menuItem_exit:
CoreInterface.pauseEmulator(true);
CoreInterface.shutdownEmulator();
mActivity.finish();
break;
default:
@@ -44,9 +44,11 @@
private static final String EGL_INITIALIZE_FAIL = "Failed to initialize EGL display connection";
private static final String EGL_INITIALIZE = "Initialized EGL display connection";
private static final String EGL_INITIALIZE_NOCHANGE = "Re-used EGL display connection";
private static final String EGL_CHOOSE_CONFIG_FAIL = "Failed to find compatible EGL frame buffer configuration";
private static final String EGL_CHOOSE_CONFIG = "Found compatible EGL frame buffer configuration";
private static final String EGL_CHOOSE_CONFIG_NOCHANGE = "Re-used EGL frame buffer configuration";
private static final String EGL_CREATE_CONTEXT_FAIL = "Failed to create EGL rendering context";
private static final String EGL_CREATE_CONTEXT = "Created EGL rendering context";
@@ -220,6 +222,26 @@ public boolean destroyGLContext()
return false;
}
/**
* Unbind the previously-created OpenGL ES rendering context and destroy the window surface.
*
* @return True, if successful.
* @see GameSurface#destroyGLSurface()
*/
public boolean destroyGLSurface()
{
Log.i( TAG, "Destroying GL surface" );
if( unbindEGLContext() )
{
if( destroyEGLSurface() )
{
return true;
}
}
Log.e( TAG, "Failed to destroy GL surface" );
return false;
}
/**
* Swap the OpenGL ES framebuffers. Requires valid, bound rendering context and window surface.
*
@@ -229,7 +251,8 @@ public void flipBuffers()
{
// Uncomment the next line only for debugging; otherwise don't waste the time
// assertPrecondition( Precondition.surface );
mEgl.eglSwapBuffers( mEglDisplay, mEglSurface );
if( mEglSurface != null && mEglSurface != EGL10.EGL_NO_SURFACE )
mEgl.eglSwapBuffers( mEglDisplay, mEglSurface );
}
/**
@@ -247,59 +270,69 @@ private boolean initializeEGL( int majorVersion, int minorVersion, int[] configS
mEgl = (EGL10) EGLContext.getEGL();
// Get an EGL display connection for the native display
mEglDisplay = mEgl.eglGetDisplay( EGL10.EGL_DEFAULT_DISPLAY );
if( mEglDisplay == EGL10.EGL_NO_DISPLAY )
if ( mEglDisplay == null || mEglDisplay == EGL10.EGL_NO_DISPLAY )
{
Log.e( TAG, EGL_GET_DISPLAY_FAIL );
return false;
}
Log.v( TAG, EGL_GET_DISPLAY );
mEglDisplay = mEgl.eglGetDisplay( EGL10.EGL_DEFAULT_DISPLAY );
if( mEglDisplay == EGL10.EGL_NO_DISPLAY )
{
Log.e( TAG, EGL_GET_DISPLAY_FAIL );
return false;
}
Log.v( TAG, EGL_GET_DISPLAY );
// Initialize the EGL display connection and obtain the GLES version supported by the device
final int[] version = new int[2];
if( !mEgl.eglInitialize( mEglDisplay, version ) )
{
Log.e( TAG, EGL_INITIALIZE_FAIL );
return false;
// Initialize the EGL display connection and obtain the GLES version supported by the device
int[] version = new int[2];
if( !mEgl.eglInitialize( mEglDisplay, version ) )
{
Log.e( TAG, EGL_INITIALIZE_FAIL );
return false;
}
Log.v( TAG, EGL_INITIALIZE );
}
Log.v( TAG, EGL_INITIALIZE );
else
Log.v( TAG, EGL_INITIALIZE_NOCHANGE );
// Set the EGL frame buffer configuration and ensure that it supports the requested GLES
// version, display connection, and frame buffer configuration
// (http://stackoverflow.com/a/5930935/254218)
// Get the number of compatible EGL frame buffer configurations
final int[] numConfigOut = new int[1];
mEgl.eglChooseConfig( mEglDisplay, configSpec, null, 0, numConfigOut );
final int numConfig = numConfigOut[0];
// Get the compatible EGL frame buffer configurations
final EGLConfig[] configs = new EGLConfig[numConfig];
boolean success = mEgl.eglChooseConfig( mEglDisplay, configSpec, configs, numConfig, null );
if( !success || numConfig == 0 )
{
Log.e( TAG, EGL_CHOOSE_CONFIG_FAIL );
return false;
}
// Select the best configuration
for( int i = 0; i < numConfig; i++ )
if (mEglContext == null || mEglContext == EGL10.EGL_NO_CONTEXT)
{
// "Best" config is the first one that is fast and egl-conformant
// So we test for the "caveat" flag which would indicate slow/non-conformant
int[] value = new int[1];
mEgl.eglGetConfigAttrib( mEglDisplay, configs[i], EGL10.EGL_CONFIG_CAVEAT, value );
if( value[0] == EGL10.EGL_NONE )
// Get the number of compatible EGL frame buffer configurations
int[] numConfigOut = new int[1];
mEgl.eglChooseConfig( mEglDisplay, configSpec, null, 0, numConfigOut );
int numConfig = numConfigOut[0];
// Get the compatible EGL frame buffer configurations
EGLConfig[] configs = new EGLConfig[numConfig];
boolean success = mEgl.eglChooseConfig( mEglDisplay, configSpec, configs, numConfig, null );
if( !success || numConfig == 0 )
{
Log.e( TAG, EGL_CHOOSE_CONFIG_FAIL );
return false;
}
// Select the best configuration
for( int i = 0; i < numConfig; i++ )
{
mEglConfig = configs[i];
break;
// "Best" config is the first one that is fast and egl-conformant
// So we test for the "caveat" flag which would indicate slow/non-conformant
int[] value = new int[1];
mEgl.eglGetConfigAttrib( mEglDisplay, configs[i], EGL10.EGL_CONFIG_CAVEAT, value );
if( value[0] == EGL10.EGL_NONE )
{
mEglConfig = configs[i];
break;
}
}
Log.v( TAG, EGL_CHOOSE_CONFIG );
}
else
Log.v( TAG, EGL_CHOOSE_CONFIG_NOCHANGE );
// Record the major version
mGlMajorVersion = majorVersion;
Log.v( TAG, EGL_CHOOSE_CONFIG );
return true;
}
@@ -313,13 +346,13 @@ private boolean initializeEGL( int majorVersion, int minorVersion, int[] configS
*/
private boolean createEGLContext( boolean forceCreate )
{
assertPrecondition( Precondition.CONFIG );
// assertPrecondition( Precondition.CONFIG );
// Create EGL rendering context
if( forceCreate || mEglContext == null || mEglContext == EGL10.EGL_NO_CONTEXT )
{
final int EGL_CONTEXT_CLIENT_VERSION = 0x3098;
final int[] contextAttrs = new int[] {
int EGL_CONTEXT_CLIENT_VERSION = 0x3098;
int[] contextAttrs = new int[] {
EGL_CONTEXT_CLIENT_VERSION,
mGlMajorVersion,
EGL10.EGL_NONE };
@@ -347,7 +380,7 @@ private boolean createEGLContext( boolean forceCreate )
*/
private boolean createEGLSurface( boolean forceCreate )
{
assertPrecondition( Precondition.CONTEXT );
// assertPrecondition( Precondition.CONTEXT );
// Create window surface
if( forceCreate || mEglSurface == null || mEglSurface == EGL10.EGL_NO_SURFACE )
@@ -374,7 +407,7 @@ private boolean createEGLSurface( boolean forceCreate )
*/
private boolean bindEGLContext()
{
assertPrecondition( Precondition.SURFACE );
// assertPrecondition( Precondition.SURFACE );
// Bind the EGL rendering context to the window surface and current rendering thread
if( mEgl.eglGetCurrentContext() != mEglContext )
@@ -400,10 +433,10 @@ private boolean bindEGLContext()
*/
private boolean unbindEGLContext()
{
assertPrecondition( Precondition.DISPLAY );
// assertPrecondition( Precondition.DISPLAY );
// Unbind rendering context and window surface
if( mEglDisplay != null )
if( mEglDisplay != null && mEglDisplay != EGL10.EGL_NO_DISPLAY )
{
if( !mEgl.eglMakeCurrent( mEglDisplay, EGL10.EGL_NO_SURFACE, EGL10.EGL_NO_SURFACE,
EGL10.EGL_NO_CONTEXT ) )
@@ -427,7 +460,7 @@ private boolean unbindEGLContext()
*/
private boolean destroyEGLSurface()
{
assertPrecondition( Precondition.DISPLAY );
// assertPrecondition( Precondition.DISPLAY );
// Destroy window surface
if( mEglSurface != null && mEglSurface != EGL10.EGL_NO_SURFACE )
@@ -454,7 +487,7 @@ private boolean destroyEGLSurface()
*/
private boolean destroyEGLContext()
{
assertPrecondition( Precondition.DISPLAY );
// assertPrecondition( Precondition.DISPLAY );
// Destroy rendering context
if( mEglContext != null && mEglContext != EGL10.EGL_NO_CONTEXT )
@@ -482,7 +515,7 @@ private boolean destroyEGLContext()
*/
private boolean terminateEGL()
{
assertPrecondition( Precondition.EGL );
// assertPrecondition( Precondition.EGL );
// Terminate display connection
if( mEglDisplay != null && mEglDisplay != EGL10.EGL_NO_DISPLAY )
@@ -51,7 +51,7 @@
*/
public static boolean createGLContext( int majorVersion, int minorVersion, int[] configSpec )
{
boolean result = sSurface.createGLContext( majorVersion, minorVersion, configSpec, true );
boolean result = sSurface.createGLContext( majorVersion, minorVersion, configSpec, false );
if( !result )
{

4 comments on commit 1b8f772

@Gillou68310

This comment has been minimized.

Show comment
Hide comment
@Gillou68310

Gillou68310 May 13, 2015

Contributor

@littleguy77 could you test this branch on your devices too see if everything is ok?

I mainly rebased this commit https://github.com/Gillou68310/mupen64plus-ae-gsi/commit/733b3775349d5c4b4891227744b2aad7161ab885 using your GameSurface class ;-)

Contributor

Gillou68310 replied May 13, 2015

@littleguy77 could you test this branch on your devices too see if everything is ok?

I mainly rebased this commit https://github.com/Gillou68310/mupen64plus-ae-gsi/commit/733b3775349d5c4b4891227744b2aad7161ab885 using your GameSurface class ;-)

@littleguy77

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 May 13, 2015

Member

This seems rather intrusive at a quick glance. It seems like a lot of the GameSurface design has been short circuited. Commenting all the assertPrecondition statements suggests to me that the behavior of the whole class has changed. Can you explain the crux of the problem with the current design? i.e. step-by-step process on your particular device to reproduce the issue this solves? I know we've discussed this before but I can't seem to find the discussion.

Member

littleguy77 replied May 13, 2015

This seems rather intrusive at a quick glance. It seems like a lot of the GameSurface design has been short circuited. Commenting all the assertPrecondition statements suggests to me that the behavior of the whole class has changed. Can you explain the crux of the problem with the current design? i.e. step-by-step process on your particular device to reproduce the issue this solves? I know we've discussed this before but I can't seem to find the discussion.

@Gillou68310

This comment has been minimized.

Show comment
Hide comment
@Gillou68310

Gillou68310 May 13, 2015

Contributor

Here's the previously discussed topic ;-)
www.paulscode.com/forum/index.php?topic=1813.0

Just to be clear the current design works perfectly on my phone. This is an alternative design that doesn't shut down the core when the surface is detroyed. One advantage is that cheat codes will be retained when restoring the app from background. IMO this is the way it should work, at least this is the way other android emu works ;-)

I removed the assert precondition because of the new destroyGLSurface method which does not respect your state machine.

Concerning nativeResume + nativePause handles they work just fine for me.

Contributor

Gillou68310 replied May 13, 2015

Here's the previously discussed topic ;-)
www.paulscode.com/forum/index.php?topic=1813.0

Just to be clear the current design works perfectly on my phone. This is an alternative design that doesn't shut down the core when the surface is detroyed. One advantage is that cheat codes will be retained when restoring the app from background. IMO this is the way it should work, at least this is the way other android emu works ;-)

I removed the assert precondition because of the new destroyGLSurface method which does not respect your state machine.

Concerning nativeResume + nativePause handles they work just fine for me.

@littleguy77

This comment has been minimized.

Show comment
Hide comment
@littleguy77

littleguy77 May 13, 2015

Member

One advantage is that cheat codes will be retained when restoring the app from background.

That is indeed very important and alone is sufficient reason to fix this. Thanks for the reminder.

I will try to take a closer look this evening to understand what's going on in this commit and see what the nativePause/nativeResume stuff is all about. I don't like just commenting out code because it no longer works with the changes. That will invariably lead to confusion later when someone has to maintain it.

Member

littleguy77 replied May 13, 2015

One advantage is that cheat codes will be retained when restoring the app from background.

That is indeed very important and alone is sufficient reason to fix this. Thanks for the reminder.

I will try to take a closer look this evening to understand what's going on in this commit and see what the nativePause/nativeResume stuff is all about. I don't like just commenting out code because it no longer works with the changes. That will invariably lead to confusion later when someone has to maintain it.

Please sign in to comment.