From 2291855a1f6ced85ec1122f36b3ce550ebac383d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 24 Sep 2023 21:41:09 +0200 Subject: [PATCH 1/6] Remove dead code path --- android/jni/app-android.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 1d667d72ebcb..5f9d6ea031cf 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -872,6 +872,7 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_pause(JNIEnv *, jclass) { } extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) { + INFO_LOG(SYSTEM, "NativeApp.shutdown() -- begin"); if (renderer_inited && useCPUThread && graphicsContext) { // Only used in Java EGL path. EmuThreadStop("shutdown"); @@ -891,8 +892,7 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) { EmuThreadJoin(); } - INFO_LOG(SYSTEM, "NativeApp.shutdown() -- begin"); - if (renderer_inited) { + if (graphicsContext) { INFO_LOG(G3D, "Shutting down renderer"); graphicsContext->Shutdown(); delete graphicsContext; @@ -1135,6 +1135,9 @@ void UpdateRunLoopAndroid(JNIEnv *env) { } extern "C" void Java_org_ppsspp_ppsspp_NativeRenderer_displayRender(JNIEnv *env, jobject obj) { + // This doesn't get called on the Vulkan path. + _assert_(useCPUThread); + static bool hasSetThreadName = false; if (!hasSetThreadName) { hasSetThreadName = true; @@ -1144,13 +1147,9 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeRenderer_displayRender(JNIEnv *env, if (IsVREnabled() && !StartVRRender()) return; - if (useCPUThread) { - // This is the "GPU thread". Call ThreadFrame. - if (!graphicsContext || !graphicsContext->ThreadFrame()) { - return; - } - } else { - UpdateRunLoopAndroid(env); + // This is the "GPU thread". Call ThreadFrame. + if (!graphicsContext || !graphicsContext->ThreadFrame()) { + return; } if (IsVREnabled()) { @@ -1457,6 +1456,7 @@ static void ProcessFrameCommands(JNIEnv *env) { } // This runs in Vulkan mode only. +// This handles the entire lifecycle of the Vulkan context, init and exit. extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runVulkanRenderLoop(JNIEnv *env, jobject obj, jobject _surf) { _assert_(!useCPUThread); @@ -1507,11 +1507,11 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runVulkanRenderLoo hasSetThreadName = true; SetCurrentThreadName("AndroidRender"); } - } - while (!exitRenderLoop) { - LockedNativeUpdateRender(); - ProcessFrameCommands(env); + while (!exitRenderLoop) { + LockedNativeUpdateRender(); + ProcessFrameCommands(env); + } } INFO_LOG(G3D, "Leaving EGL/Vulkan render loop."); From 2b0bbb1e0c4a23c80ef278d9b0185954c3d225c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 24 Sep 2023 22:07:48 +0200 Subject: [PATCH 2/6] Remove isFinishing check in onDestroy - not relevant. --- android/src/org/ppsspp/ppsspp/NativeActivity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/src/org/ppsspp/ppsspp/NativeActivity.java b/android/src/org/ppsspp/ppsspp/NativeActivity.java index db4a4c1c931d..368748763abe 100644 --- a/android/src/org/ppsspp/ppsspp/NativeActivity.java +++ b/android/src/org/ppsspp/ppsspp/NativeActivity.java @@ -785,7 +785,7 @@ protected void onDestroy() { // TODO: Can we ensure that the GL thread has stopped rendering here? // I've seen crashes that seem to indicate that sometimes it hasn't... NativeApp.audioShutdown(); - if (shuttingDown || isFinishing()) { + if (shuttingDown) { NativeApp.shutdown(); unregisterCallbacks(); initialized = false; From 0013c6fedec51e3ca06f43edf4f389a435bf47f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 24 Sep 2023 22:09:00 +0200 Subject: [PATCH 3/6] Rename ensureRenderLoop -> startRenderLoopThread --- android/src/org/ppsspp/ppsspp/NativeActivity.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/android/src/org/ppsspp/ppsspp/NativeActivity.java b/android/src/org/ppsspp/ppsspp/NativeActivity.java index 368748763abe..1f4d09178dc9 100644 --- a/android/src/org/ppsspp/ppsspp/NativeActivity.java +++ b/android/src/org/ppsspp/ppsspp/NativeActivity.java @@ -662,7 +662,7 @@ public void onCreate(Bundle savedInstanceState) { Log.i(TAG, "setcontentview before"); setContentView(mSurfaceView); Log.i(TAG, "setcontentview after"); - ensureRenderLoop(); + startRenderLoopThread(); } } @@ -682,7 +682,7 @@ public void notifySurface(Surface surface) { if (mSurface == null) { joinRenderLoopThread(); } else { - ensureRenderLoop(); + startRenderLoopThread(); } } updateSustainedPerformanceMode(); @@ -690,7 +690,7 @@ public void notifySurface(Surface surface) { // Invariants: After this, mRenderLoopThread will be set, and the thread will be running, // if in Vulkan mode. - protected synchronized void ensureRenderLoop() { + protected synchronized void startRenderLoopThread() { if (javaGL) { Log.e(TAG, "JavaGL mode - should not get into ensureRenderLoop."); return; @@ -866,7 +866,7 @@ protected void onResume() { if (!javaGL) { // Restart the render loop. - ensureRenderLoop(); + startRenderLoopThread(); } } From 1b8b441cfd51d29481672be299e077e17e99dc5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 25 Sep 2023 00:01:54 +0200 Subject: [PATCH 4/6] Tighten up some renderloop logic in app-android.cpp --- android/jni/app-android.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 5f9d6ea031cf..2160536fac87 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -688,8 +688,9 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_init EARLY_LOG("NativeApp.init() -- begin"); PROFILE_INIT(); - std::lock_guard guard(renderLock); // Note: This is held for the rest of this function - intended? + std::lock_guard guard(renderLock); // Note: This is held for the rest of this function - intended? and even necessary? renderer_inited = false; + exitRenderLoop = false; androidVersion = jAndroidVersion; deviceType = jdeviceType; @@ -1462,10 +1463,18 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runVulkanRenderLoo if (!graphicsContext) { ERROR_LOG(G3D, "runVulkanRenderLoop: Tried to enter without a created graphics context."); + renderLoopRunning = false; + exitRenderLoop = false; return false; } - exitRenderLoop = false; + if (exitRenderLoop) { + WARN_LOG(G3D, "runVulkanRenderLoop: ExitRenderLoop requested at start, skipping the whole thing."); + renderLoopRunning = false; + exitRenderLoop = false; + return true; + } + // This is up here to prevent race conditions, in case we pause during init. renderLoopRunning = true; @@ -1525,6 +1534,7 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runVulkanRenderLoo INFO_LOG(G3D, "Shutting down graphics context from render thread..."); graphicsContext->ShutdownFromRenderThread(); renderLoopRunning = false; + exitRenderLoop = false; WARN_LOG(G3D, "Render loop function exited."); return true; From 8b9836afd3a4e15f6fae8147bf5a74fc0cca8005 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 24 Sep 2023 21:51:58 +0200 Subject: [PATCH 5/6] SizeManager: Don't send notifySurface if paused. Cleaner exits / task switches in the log. --- android/src/org/ppsspp/ppsspp/NativeActivity.java | 14 ++++++++------ android/src/org/ppsspp/ppsspp/SizeManager.java | 14 +++++++++++++- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/android/src/org/ppsspp/ppsspp/NativeActivity.java b/android/src/org/ppsspp/ppsspp/NativeActivity.java index 1f4d09178dc9..831cc16d7660 100644 --- a/android/src/org/ppsspp/ppsspp/NativeActivity.java +++ b/android/src/org/ppsspp/ppsspp/NativeActivity.java @@ -677,6 +677,12 @@ public void onWindowFocusChanged(boolean hasFocus) { public void notifySurface(Surface surface) { mSurface = surface; + + if (!initialized) { + Log.e(TAG, "Can't deal with surfaces while not initialized"); + return; + } + if (!javaGL) { // If we got a surface, this starts the thread. If not, it doesn't. if (mSurface == null) { @@ -740,12 +746,6 @@ void setupSystemUiCallback() { navigationCallbackView = decorView; } - @Override - protected void onStop() { - super.onStop(); - Log.i(TAG, "onStop - do nothing special"); - } - @Override protected void onDestroy() { super.onDestroy(); @@ -803,6 +803,7 @@ protected void onPause() { super.onPause(); Log.i(TAG, "onPause"); loseAudioFocus(this.audioManager, this.audioFocusChangeListener); + sizeManager.setPaused(true); NativeApp.pause(); if (!javaGL) { mSurfaceView.onPause(); @@ -838,6 +839,7 @@ private boolean detectOpenGLES30() { protected void onResume() { super.onResume(); updateSustainedPerformanceMode(); + sizeManager.setPaused(false); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.ICE_CREAM_SANDWICH) { updateSystemUiVisibility(); } diff --git a/android/src/org/ppsspp/ppsspp/SizeManager.java b/android/src/org/ppsspp/ppsspp/SizeManager.java index 25509769eb4d..af81f640ddc3 100644 --- a/android/src/org/ppsspp/ppsspp/SizeManager.java +++ b/android/src/org/ppsspp/ppsspp/SizeManager.java @@ -39,10 +39,18 @@ public class SizeManager implements SurfaceHolder.Callback { private Point desiredSize = new Point(); private int badOrientationCount = 0; + + private boolean paused = false; + public SizeManager(final NativeActivity a) { activity = a; } + + public void setPaused(boolean p) { + paused = p; + } + @TargetApi(Build.VERSION_CODES.P) public void setSurfaceView(SurfaceView view) { surfaceView = view; @@ -107,7 +115,11 @@ public void surfaceChanged(SurfaceHolder holder, int format, int width, int heig NativeApp.backbufferResize(width, height, format); updateDisplayMeasurements(); - activity.notifySurface(holder.getSurface()); + if (!paused) { + activity.notifySurface(holder.getSurface()); + } else { + Log.i(TAG, "Skipping notifySurface while paused"); + } } @Override From 0fd22ea3bbccb4b62d3da4eb7513cbe45b505abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 25 Sep 2023 09:40:14 +0200 Subject: [PATCH 6/6] Comment clarifications. Slightly extend renderlock use in shutdown. --- android/jni/app-android.cpp | 28 +++++++++++-------- .../src/org/ppsspp/ppsspp/NativeActivity.java | 25 ++++++++++------- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 2160536fac87..ffe7e411b61b 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -688,7 +688,7 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_init EARLY_LOG("NativeApp.init() -- begin"); PROFILE_INIT(); - std::lock_guard guard(renderLock); // Note: This is held for the rest of this function - intended? and even necessary? + std::lock_guard guard(renderLock); renderer_inited = false; exitRenderLoop = false; androidVersion = jAndroidVersion; @@ -874,8 +874,13 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_pause(JNIEnv *, jclass) { extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) { INFO_LOG(SYSTEM, "NativeApp.shutdown() -- begin"); + if (renderer_inited && useCPUThread && graphicsContext) { // Only used in Java EGL path. + + // We can't lock renderLock here because the emu thread will be in NativeFrame + // which locks renderLock already, and only gets out once we call ThreadFrame() + // in a loop before, to empty the queue. EmuThreadStop("shutdown"); INFO_LOG(SYSTEM, "BeginAndroidShutdown"); graphicsContext->BeginAndroidShutdown(); @@ -893,18 +898,19 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) { EmuThreadJoin(); } - if (graphicsContext) { - INFO_LOG(G3D, "Shutting down renderer"); - graphicsContext->Shutdown(); - delete graphicsContext; - graphicsContext = nullptr; - renderer_inited = false; - } else { - INFO_LOG(G3D, "Not shutting down renderer - not initialized"); - } - { std::lock_guard guard(renderLock); + + if (graphicsContext) { + INFO_LOG(G3D, "Shutting down renderer"); + graphicsContext->Shutdown(); + delete graphicsContext; + graphicsContext = nullptr; + renderer_inited = false; + } else { + INFO_LOG(G3D, "Not shutting down renderer - not initialized"); + } + NativeShutdown(); g_VFS.Clear(); } diff --git a/android/src/org/ppsspp/ppsspp/NativeActivity.java b/android/src/org/ppsspp/ppsspp/NativeActivity.java index 831cc16d7660..4f6cc7835359 100644 --- a/android/src/org/ppsspp/ppsspp/NativeActivity.java +++ b/android/src/org/ppsspp/ppsspp/NativeActivity.java @@ -751,16 +751,21 @@ protected void onDestroy() { super.onDestroy(); Log.i(TAG, "onDestroy"); if (javaGL) { - if (nativeRenderer != null && nativeRenderer.isRenderingFrame()) { - Log.i(TAG, "Waiting for renderer to finish."); - int tries = 200; - do { - try { - Thread.sleep(10); - } catch (InterruptedException e) { - } - tries--; - } while (nativeRenderer.isRenderingFrame() && tries > 0); + if (nativeRenderer != null) { + if (nativeRenderer.isRenderingFrame()) { + Log.i(TAG, "Waiting for renderer to finish."); + int tries = 200; + do { + try { + Thread.sleep(10); + } catch (InterruptedException e) { + } + tries--; + } while (nativeRenderer.isRenderingFrame() && tries > 0); + } else { + Log.i(TAG, "nativerenderer done."); + nativeRenderer = null; + } } mGLSurfaceView.onDestroy(); mGLSurfaceView = null;