Only pause GLSurfaceView if activity is moved to the background, and not if it just loses focus #1942

rh101 · 2024-05-30T06:28:19Z

Describe your changes

At the moment, there are several issues on Android:

1 - There are instances where applicationWillEnterForeground() is called twice. This should also not be called on initial app start-up, since the behaviour would be different to other platforms.

2 - The GLSurfaceView should not be paused if the activity has simply lost focus, but still visible to the user. According to the Android documentation, the recommendation is that it only be paused if the application enters the background, meaning in Activity.onStop.

3 - If the GLSurfaceView is paused, there is a possibility that Android will discard the EGL context under certain conditions. If the application has simply lost focus and is still visible to the user, the view should remain valid.

4 - Application::applicationDidEnterBackground() is also called from GLSurfaceView.onPause(), but this should not occur if the application simply lost focus and is still visible.

3 - Activity.onPause() should not take too much time complete, and the current implementation which calls GLSurfaceView.onPause() from Activity.onPause(), and queues a call to Application::applicationDidEnterBackground() on the GL thread. There is no guarantee that the game state can be saved before the application goes into the background if this is a time-intensive process.

The changes in this PR should resolve all the issues listed above.

1 - applicationWillEnterForeground() should not be called on initial app start-up.
2 - If the application activity loses focus, then the GLSurfaceView should not be paused, but simply put into a more efficient rendering mode, using RENDERMODE_WHEN_DIRTY instead of RENDERMODE_CONTINUOUSLY.
3 - Android will not detect the GLSurfaceView as being paused if it is not yet in the background, which means there is no reason for the EGL context to be discarded.
4 - Now that GLSurfaceView.onPause() is called from Activity.onStop(), then there are no longer any issues with time-intensive processing of saving game state in Application::applicationDidEnterBackground().

Issue ticket number and link

Related to #1211

Checklist before requesting a review

For each PR

Add Copyright if it missed:
- "Copyright (c) 2019-present Axmol Engine contributors (see AUTHORS.md)."
I have performed a self-review of my code.

Optional:
- I have checked readme and add important infos to this PR.
- I have added/adapted some tests too.

For core/new feature PR

I have checked readme and add important infos to this PR.
I have added thorough tests.

…just loses focus

smilediver · 2024-05-30T10:22:49Z

What about this:

axmol/core/platform/android/jni/Java_org_axmol_lib_AxmolRenderer.cpp

Lines 56 to 71 in 59592d3

    
           JNIEXPORT void JNICALL Java_org_axmol_lib_AxmolRenderer_nativeOnResume(JNIEnv*, jclass) 
        
           { 
        
               static bool firstTime = true; 
        
               if (Director::getInstance()->getGLView()) 
        
               { 
        
                   // don't invoke at first to keep the same logic as iOS 
        
                   // can refer to https://github.com/cocos2d/cocos2d-x/issues/14206 
        
                   if (!firstTime) 
        
                       Application::getInstance()->applicationWillEnterForeground(); 
        
                   ax::EventCustom foregroundEvent(EVENT_COME_TO_FOREGROUND); 
        
                   ax::Director::getInstance()->getEventDispatcher()->dispatchEvent(&foregroundEvent, true); 
        
                   firstTime = false; 
        
               } 
        
           }

I think firstTime hack should be removed and better be managed in Java's side, since it's responsible for managing the app's state.

Also I think EVENT_COME_TO_FOREGROUND is not supposed to be sent the first time either, or I'm wrong?

smilediver · 2024-05-30T10:29:33Z

AxmolGLSurfaceView.java has calls to setRenderMode() too. Maybe those should be removed, so that AxmolActivity.java is the only one that is calling them?

rh101 · 2024-05-30T11:09:32Z

What about this:

axmol/core/platform/android/jni/Java_org_axmol_lib_AxmolRenderer.cpp

Lines 56 to 71 in 59592d3

JNIEXPORT void JNICALL Java_org_axmol_lib_AxmolRenderer_nativeOnResume(JNIEnv*, jclass)

{

static bool firstTime = true;

if (Director::getInstance()->getGLView())

{

// don't invoke at first to keep the same logic as iOS

// can refer to https://github.com/cocos2d/cocos2d-x/issues/14206

if (!firstTime)

Application::getInstance()->applicationWillEnterForeground();

ax::EventCustom foregroundEvent(EVENT_COME_TO_FOREGROUND);

ax::Director::getInstance()->getEventDispatcher()->dispatchEvent(&foregroundEvent, true);

firstTime = false;

}

}

I think firstTime hack should be removed and better be managed in Java's side, since it's responsible for managing the app's state.

Also I think EVENT_COME_TO_FOREGROUND is not supposed to be sent the first time either, or I'm wrong?

Good point. I'll go through the code to see what that and the background event are used for, and update it accordingly.

rh101 · 2024-05-30T11:13:11Z

AxmolGLSurfaceView.java has calls to setRenderMode() too. Maybe those should be removed, so that AxmolActivity.java is the only one that is calling them?

Yes, the calls in AxmolGLSurfaceView are redundant, so I'll update that too.

…pp start Remove work-around in nativeOnResume to avoid calling applicationWillEnterForeground twice since it is no longer required Remove redundant setRenderMode calls in AxmolGLSurfaceView

rh101 · 2024-05-30T15:21:02Z

I've tested this out, and as far as I can tell, it's working well. If anyone else could possibly test it out as well then that would be great.

smilediver · 2024-05-30T16:26:52Z

I'll try to find some time for testing tomorrow.

smilediver · 2024-05-31T10:45:02Z

Java_org_axmol_lib_AxmolRenderer_nativeOnResume is not being called when GL context is lost. Repro steps:

Move app to background.
Observe Java_org_axmol_lib_AxmolRenderer_nativeOnPause is called.
Force Android to destroy GL context.
Move app to foreground.
Observe Java_org_axmol_lib_AxmolRenderer_nativeInit is called, EVENT_RENDERER_RECREATED sent.
Observe Java_org_axmol_lib_AxmolRenderer_nativeOnResume is not called.

smilediver · 2024-05-31T11:08:04Z

AxmolActivity, AxmolRenderer, AxmolGLSurfaceView - all these objects will be recreated when GL context is lost, so all bool flags like mNativeInitCompleted are lost.

rh101 · 2024-05-31T14:11:22Z

Java_org_axmol_lib_AxmolRenderer_nativeOnResume is not being called when GL context is lost. Repro steps:

* Move app to background.

* Observe `Java_org_axmol_lib_AxmolRenderer_nativeOnPause` is called.

* Force Android to destroy GL context.

* Move app to foreground.

* Observe `Java_org_axmol_lib_AxmolRenderer_nativeInit` is called, `EVENT_RENDERER_RECREATED` sent.

* Observe `Java_org_axmol_lib_AxmolRenderer_nativeOnResume` is not called.

Looking into this now.

AxmolActivity, AxmolRenderer, AxmolGLSurfaceView - all these objects will be recreated when GL context is lost, so all bool flags like mNativeInitCompleted are lost.

I don't see this at all. They are definitely not recreated, so I am a little confused as to why you're seeing that. Add a breakpoint to AxmolRenderer.onSurfaceCreated, start up the app, and you will see that mNativeInitCompleted is false, and in that method it is set to true.

Now, move the app to the background, then bring it back to the foreground, and since the context is lost, AxmolRenderer.onSurfaceCreated will be called, and you will see that mNativeInitCompleted is still true, meaning that it never recreated. Same for the mIsPaused, which is true, even though the initial value would be false if the object was recreated.

AxmolActivity is also not recreated, so I'm genuinely curious as to why that is happening in your test.

Out of curiosity, is AX_ENABLE_CACHE_TEXTURE_DATA set to 1? If not, and it is 0, then is AX_ENABLE_RESTART_APPLICATION_ON_CONTEXT_LOST set to 1 or 0?

Also, are you forcing the context loss via this.mGLSurfaceView.setPreserveEGLContextOnPause(false); or some other means?

rh101 · 2024-05-31T14:38:08Z

@smilediver I cannot reproduce what you are seeing. Using logging calls, this is the output from the steps you mentioned:

// App cold start-up

AxmolActivity           org.axmol.test                       D  AxmolActivity.onCreate
main                    org.axmol.test                       D  axmol_android_app_init
AxmolActivity           org.axmol.test                       D  AxmolActivity: SET NEW RENDERER
AxmolGLSurfaceView      org.axmol.test                       D  AxmolGLSurfaceView.setRenderer
AxmolActivity           org.axmol.test                       D  onResume()
AxmolActivity           org.axmol.test                       D  onWindowFocusChanged() hasFocus=true
AxmolRenderer           org.axmol.test                       D  AxmolRenderer.handleOnResume

// App moved to background

AxmolActivity           org.axmol.test                       D  onWindowFocusChanged() hasFocus=false
AxmolActivity           org.axmol.test                       D  onPause()
AxmolRenderer           org.axmol.test                       D  AxmolRenderer.handleOnPause
axmol                   org.axmol.test                       D  Java_org_axmol_lib_AxmolRenderer_nativeOnPause
axmol                   org.axmol.test                       D  Java_org_axmol_lib_AxmolRenderer_nativeOnPause => dispatch EVENT_COME_TO_BACKGROUND

// App brought back to foreground

AxmolActivity           org.axmol.test                       D  onResume()
AxmolActivity           org.axmol.test                       D  onWindowFocusChanged() hasFocus=true
axmol                   org.axmol.test                       I  Load GLES success, version: 30002
axmol debug info        org.axmol.test                       D  reload all texture
AxmolRenderer           org.axmol.test                       D  AxmolRenderer.handleOnResume
axmol                   org.axmol.test                       D  Java_org_axmol_lib_AxmolRenderer_nativeOnResume
axmol                   org.axmol.test                       D  Java_org_axmol_lib_AxmolRenderer_nativeOnResume => dispatch EVENT_COME_TO_FOREGROUND

To force the context loss, I've set this to false this.mGLSurfaceView.setPreserveEGLContextOnPause(false);.
Volatile texture cache is enabled: AX_ENABLE_CACHE_TEXTURE_DATA = 1
App restart is disabled: AX_ENABLE_RESTART_APPLICATION_ON_CONTEXT_LOST = 0

smilediver · 2024-05-31T14:41:18Z

AX_ENABLE_CACHE_TEXTURE_DATA is 1
AX_ENABLE_RESTART_APPLICATION_ON_CONTEXT_LOST is 0
setPreserveEGLContextOnPause is true

So with these settings moving app to background doesn't destroy GL context. To destroy the GL context I change the font size in the Android's settings. It seems this is also related to this: https://developer.android.com/guide/topics/resources/runtime-changes, where some Android events can relaunch the activity.

Maybe there are two different situations: one, where only GL context is lost; another when GL context is lost and activity is relaunched (possibly in these cases GL context is lost due to activity being relaunched).

rh101 · 2024-05-31T14:44:53Z

So with these settings moving app to background doesn't destroy GL context. To destroy the GL context I change the font size in the Android's settings. It seems this is also related to this: https://developer.android.com/guide/topics/resources/runtime-changes, where some Android events can relaunch the activity.

Yes, that's most likely the case, and since the activity is completely restarted, it isn't actually coming from the background to the foreground; it's like a cold-start. Is the entire application process killed though, or just the activity?

smilediver · 2024-05-31T14:48:12Z

it's a cold-start. Is the entire application process killed though, or just the activity?

Well, it's not a cold start, there's some warmness in there... :D No, the app is not killed, just the activity is restarted and GL context is lost. The fix probably just needs to use static variables instead of members to store the state of the native side.

rh101 · 2024-05-31T14:49:21Z

Well, it's not a cold start, there's some warmness in there... :D No, the app is not killed, just the activity is restarted and GL context is lost. The fix probably just needs to use static variables instead of members to store the state of the native side

Cocos2d-x never handled this situation either, so this is something completely new. I'll take a look at this scenario in a day or so, because it may impact more than just the EGL context related code.

For the time being, do the changes in this PR address the most common scenario, which is an EGL context loss (not an activity restart)? That's what this PR is about, so perhaps it's better to verify for this specific scenario, and deal with the other scenario in a separate PR.

smilediver · 2024-05-31T15:04:28Z

For the time being, do the changes in this PR address the most common scenario, which is an EGL context loss (not an activity restart)? That's what this PR is about, so perhaps it's better to verify for this specific scenario, and deal with the other scenario in a separate PR.

With setPreserveEGLContextOnPause(false) it works correctly, but with setPreserveEGLContextOnPause(true) I have no idea how to force GL context loss without triggering activity relaunch.

rh101 · 2024-05-31T15:32:27Z

With setPreserveEGLContextOnPause(false) it works correctly, but with setPreserveEGLContextOnPause(true) I have no idea how to force GL context loss without triggering activity relaunch.

setPreserveEGLContextOnPause(false) is the only way I could reproduce the EGL context loss quickly for testing.

With setPreserveEGLContextOnPause(true), it is not guaranteed that the EGL context will be retained. From setPreserveEGLContextOnPause:

If set to true, then the EGL context may be preserved when the GLSurfaceView is paused.

Prior to API level 11, whether the EGL context is actually preserved or not depends upon whether the Android device can support an arbitrary number of EGL contexts or not. Devices that can only support a limited number of EGL contexts must release the EGL context in order to allow multiple applications to share the GPU.

If set to false, the EGL context will be released when the GLSurfaceView is paused, and recreated when the GLSurfaceView is resumed.

So, having it as true does not guarantee that it will be preserved, and setting it to false is the way to test an EGL context loss.

smilediver · 2024-05-31T16:01:59Z

I'm aware about guarantees, just saying that I don't know how to reproduce only GL context loss with setPreserveEGLContextOnPause(true) (it would be nice to test with it set to true, since that's the final code we ship to users). In our studio we always reproduced it by changing Android's font size, which apparently also relaunches app's activity. I always assumed that GL context loss comes together with activity's destruction, but as we found out today, it's not the case.

rh101 · 2024-06-01T01:19:24Z

I'm aware about guarantees, just saying that I don't know how to reproduce only GL context loss with setPreserveEGLContextOnPause(true) (it would be nice to test with it set to true, since that's the final code we ship to users).

Setting to false to force the EGL context loss on a pause is functionally equivalent to Android forcing the context loss when the value is true (due to the app being in the background etc.). That should provide you with a reliable way to test this in your production code.

rh101 added 2 commits May 30, 2024 16:08

Only pause GLSurfaceView if activity is being stopped, and not if it …

9643172

…just loses focus

Add copyright notice

59592d3

rh101 marked this pull request as draft May 30, 2024 11:07

rh101 added 2 commits May 31, 2024 01:17

Ensure foreground and background events are not sent out on initial a…

921dca4

…pp start Remove work-around in nativeOnResume to avoid calling applicationWillEnterForeground twice since it is no longer required Remove redundant setRenderMode calls in AxmolGLSurfaceView

Add copyright notices

0e5b66b

rh101 marked this pull request as ready for review May 30, 2024 15:19

halx99 added the enhancement New feature or request label May 31, 2024

rh101 changed the title ~~Only pause GLSurfaceView if activity is being stopped, and not if it just loses focus~~ Only pause GLSurfaceView if activity is moved to the background, and not if it just loses focus Jun 1, 2024

halx99 approved these changes Jun 3, 2024

View reviewed changes

halx99 merged commit 4abc7f4 into axmolengine:dev Jun 9, 2024
28 of 29 checks passed

halx99 added this to the 2.1.4 milestone Jun 9, 2024

rh101 deleted the android-glview-state branch June 9, 2024 04:14

rh101 mentioned this pull request Sep 2, 2024

Android : Fixing rendering for all FPS (> 60 or < 60) - Thread to trigger rendering manually #2125

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only pause GLSurfaceView if activity is moved to the background, and not if it just loses focus #1942

Only pause GLSurfaceView if activity is moved to the background, and not if it just loses focus #1942

rh101 commented May 30, 2024 •

edited

Loading

smilediver commented May 30, 2024

smilediver commented May 30, 2024

rh101 commented May 30, 2024

rh101 commented May 30, 2024

rh101 commented May 30, 2024

smilediver commented May 30, 2024

smilediver commented May 31, 2024

smilediver commented May 31, 2024

rh101 commented May 31, 2024 •

edited

Loading

rh101 commented May 31, 2024 •

edited

Loading

smilediver commented May 31, 2024

rh101 commented May 31, 2024 •

edited

Loading

smilediver commented May 31, 2024

rh101 commented May 31, 2024 •

edited

Loading

smilediver commented May 31, 2024

rh101 commented May 31, 2024 •

edited

Loading

smilediver commented May 31, 2024 •

edited

Loading

rh101 commented Jun 1, 2024 •

edited

Loading

Only pause GLSurfaceView if activity is moved to the background, and not if it just loses focus #1942

Only pause GLSurfaceView if activity is moved to the background, and not if it just loses focus #1942

Conversation

rh101 commented May 30, 2024 • edited Loading

Describe your changes

Issue ticket number and link

Checklist before requesting a review

For each PR

For core/new feature PR

smilediver commented May 30, 2024

smilediver commented May 30, 2024

rh101 commented May 30, 2024

rh101 commented May 30, 2024

rh101 commented May 30, 2024

smilediver commented May 30, 2024

smilediver commented May 31, 2024

smilediver commented May 31, 2024

rh101 commented May 31, 2024 • edited Loading

rh101 commented May 31, 2024 • edited Loading

smilediver commented May 31, 2024

rh101 commented May 31, 2024 • edited Loading

smilediver commented May 31, 2024

rh101 commented May 31, 2024 • edited Loading

smilediver commented May 31, 2024

rh101 commented May 31, 2024 • edited Loading

smilediver commented May 31, 2024 • edited Loading

rh101 commented Jun 1, 2024 • edited Loading

rh101 commented May 30, 2024 •

edited

Loading

rh101 commented May 31, 2024 •

edited

Loading

rh101 commented May 31, 2024 •

edited

Loading

rh101 commented May 31, 2024 •

edited

Loading

rh101 commented May 31, 2024 •

edited

Loading

rh101 commented May 31, 2024 •

edited

Loading

smilediver commented May 31, 2024 •

edited

Loading

rh101 commented Jun 1, 2024 •

edited

Loading