Skip to content

Commit

Permalink
front: Fixed GL context restoring
Browse files Browse the repository at this point in the history
  • Loading branch information
Gillou68310 committed May 13, 2015
1 parent daa6c29 commit 1b8f772
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 53 deletions.
2 changes: 1 addition & 1 deletion jni/SDL2/src/video/android/SDL_androidevents.c
View file
Expand Up @@ -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"
Expand Down
12 changes: 11 additions & 1 deletion jni/ae-bridge/ae_exports.cpp
View file
Expand Up @@ -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;
Expand All @@ -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)
{
Expand Down Expand Up @@ -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.

Sign in to view

Copy link
@littleguy77

littleguy77 May 13, 2015

Member

These function handles will never be found.

This comment has been minimized.

Sign in to view

Copy link
@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.

Sign in to view

Copy link
@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");
}
Expand All @@ -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");
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions src/paulscode/android/mupen64plusae/game/GameLifecycleHandler.java
View file
Expand Up @@ -280,8 +280,10 @@ public void onWindowFocusChanged( boolean hasFocus )
Log.i( "GameLifecycleHandler", "onWindowFocusChanged: " + hasFocus );
mIsFocused = hasFocus;
if( hasFocus )
{
hideSystemBars();
tryRunning();
tryRunning();
}
}

public void onPause()
Expand All @@ -298,7 +300,8 @@ public void surfaceDestroyed( SurfaceHolder holder )
{
Log.i( "GameLifecycleHandler", "surfaceDestroyed" );
mIsSurface = false;
tryStopping();
tryPausing();
mSurface.destroyGLSurface();
}

public void onStop()
Expand Down
2 changes: 2 additions & 0 deletions src/paulscode/android/mupen64plusae/game/GameMenuHandler.java
View file
Expand Up @@ -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:
Expand Down
129 changes: 81 additions & 48 deletions src/paulscode/android/mupen64plusae/game/GameSurface.java
View file
Expand Up @@ -44,9 +44,11 @@ public class GameSurface extends SurfaceView

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";
Expand Down Expand Up @@ -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.
*
Expand All @@ -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 );
}

/**
Expand All @@ -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;
}

Expand All @@ -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 };
Expand Down Expand Up @@ -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 )
Expand All @@ -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 )
Expand All @@ -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 ) )
Expand All @@ -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 )
Expand All @@ -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 )
Expand Down Expand Up @@ -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 )
Expand Down
2 changes: 1 addition & 1 deletion src/paulscode/android/mupen64plusae/jni/NativeSDL.java
View file
Expand Up @@ -51,7 +51,7 @@ public class NativeSDL extends CoreInterface
*/
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 )
{
Expand Down

4 comments on commit 1b8f772

@Gillou68310
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.