From 2c564a0b9d654d860dcc2832b2492f8e1b94813b Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 8 Aug 2021 12:10:03 +0200 Subject: [PATCH 1/5] Android: Remove mSurface from EmulationState --- .../dolphinemu/dolphinemu/NativeLibrary.java | 2 ++ .../fragments/EmulationFragment.java | 30 ++++--------------- Source/Android/jni/MainAndroid.cpp | 7 +++++ 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java index 0ac2463217..5ab4e15993 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java @@ -393,6 +393,8 @@ public final class NativeLibrary public static native void SurfaceDestroyed(); + public static native boolean HasSurface(); + /** * Unpauses emulation from a paused state. */ diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java index 7d18c47936..2ea106cb4a 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java @@ -173,13 +173,15 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) { Log.debug("[EmulationFragment] Surface changed. Resolution: " + width + "x" + height); - mEmulationState.newSurface(holder.getSurface()); + NativeLibrary.SurfaceChanged(holder.getSurface()); + mEmulationState.newSurface(); } @Override public void surfaceDestroyed(@NonNull SurfaceHolder holder) { - mEmulationState.clearSurface(); + Log.debug("[EmulationFragment] Surface destroyed."); + NativeLibrary.SurfaceDestroyed(); } public void stopEmulation() @@ -219,7 +221,6 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C private final String[] mGamePaths; private State state; - private Surface mSurface; private boolean mRunWhenSurfaceIsValid; private boolean loadPreviousTemporaryState; private final String temporaryStatePath; @@ -304,7 +305,7 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C } // If the surface is set, run now. Otherwise, wait for it to get set. - if (mSurface != null) + if (NativeLibrary.HasSurface()) { runWithValidSurface(); } @@ -314,31 +315,14 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C } } - // Surface callbacks - public synchronized void newSurface(Surface surface) + public synchronized void newSurface() { - mSurface = surface; if (mRunWhenSurfaceIsValid) { runWithValidSurface(); } } - public synchronized void clearSurface() - { - if (mSurface == null) - { - Log.warning("[EmulationFragment] clearSurface called, but surface already null."); - } - else - { - mSurface = null; - Log.debug("[EmulationFragment] Surface destroyed."); - - NativeLibrary.SurfaceDestroyed(); - } - } - private void runWithValidSurface() { mRunWhenSurfaceIsValid = false; @@ -346,7 +330,6 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C { Thread emulationThread = new Thread(() -> { - NativeLibrary.SurfaceChanged(mSurface); if (loadPreviousTemporaryState) { Log.debug("[EmulationFragment] Starting emulation thread from previous state."); @@ -363,7 +346,6 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C } else if (state == State.PAUSED) { - NativeLibrary.SurfaceChanged(mSurface); if (!EmulationActivity.getHasUserPausedEmulation() && !NativeLibrary.IsShowingAlertMessage()) { diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index 765bab1bcb..eedc728ad9 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -442,6 +442,13 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceDestr } } +JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_HasSurface(JNIEnv*, jclass) +{ + std::lock_guard guard(s_surface_lock); + + return s_surf ? JNI_TRUE : JNI_FALSE; +} + JNIEXPORT jfloat JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetGameAspectRatio(JNIEnv*, jclass) { From 446e2d9119a3c4281e72feefca9bbcd2b1eb21e3 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 8 Aug 2021 14:33:06 +0200 Subject: [PATCH 2/5] Android: Remove state from EmulationState --- .../dolphinemu/dolphinemu/NativeLibrary.java | 9 +++ .../fragments/EmulationFragment.java | 79 +++---------------- Source/Android/jni/MainAndroid.cpp | 16 +++- 3 files changed, 36 insertions(+), 68 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java index 5ab4e15993..123cd6c066 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java @@ -410,6 +410,13 @@ public final class NativeLibrary */ public static native void StopEmulation(); + /** + * Ensures that IsRunning will return true from now on until emulation exits. + * (If this is not called, IsRunning will start returning true at some point + * after calling Run.) + */ + public static native void SetIsBooting(); + /** * Returns true if emulation is running (or is paused). */ @@ -417,6 +424,8 @@ public final class NativeLibrary public static native boolean IsRunningAndStarted(); + public static native boolean IsRunningAndUnpaused(); + /** * Enables or disables CPU block profiling * diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java index 2ea106cb4a..ce35f1e98e 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java @@ -123,8 +123,12 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C @Override public void onPause() { - if (mEmulationState.isRunning() && !NativeLibrary.IsShowingAlertMessage()) - mEmulationState.pause(); + if (NativeLibrary.IsRunningAndUnpaused() && !NativeLibrary.IsShowingAlertMessage()) + { + Log.debug("[EmulationFragment] Pausing emulation."); + NativeLibrary.PauseEmulation(); + } + super.onPause(); } @@ -186,7 +190,8 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C public void stopEmulation() { - mEmulationState.stop(); + Log.debug("[EmulationFragment] Stopping emulation."); + NativeLibrary.StopEmulation(); } public void startConfiguringControls() @@ -214,13 +219,7 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C private static class EmulationState { - private enum State - { - STOPPED, RUNNING, PAUSED - } - private final String[] mGamePaths; - private State state; private boolean mRunWhenSurfaceIsValid; private boolean loadPreviousTemporaryState; private final String temporaryStatePath; @@ -229,56 +228,6 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C { mGamePaths = gamePaths; this.temporaryStatePath = temporaryStatePath; - // Starting state is stopped. - state = State.STOPPED; - } - - // Getters for the current state - - public synchronized boolean isStopped() - { - return state == State.STOPPED; - } - - public synchronized boolean isPaused() - { - return state == State.PAUSED; - } - - public synchronized boolean isRunning() - { - return state == State.RUNNING; - } - - // State changing methods - - public synchronized void stop() - { - if (state != State.STOPPED) - { - Log.debug("[EmulationFragment] Stopping emulation."); - state = State.STOPPED; - NativeLibrary.StopEmulation(); - } - else - { - Log.warning("[EmulationFragment] Stop called while already stopped."); - } - } - - public synchronized void pause() - { - if (state != State.PAUSED) - { - state = State.PAUSED; - Log.debug("[EmulationFragment] Pausing emulation."); - - NativeLibrary.PauseEmulation(); - } - else - { - Log.warning("[EmulationFragment] Pause called while already paused."); - } } public synchronized void run(boolean isActivityRecreated) @@ -288,7 +237,6 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C if (NativeLibrary.IsRunning()) { loadPreviousTemporaryState = false; - state = State.PAUSED; deleteFile(temporaryStatePath); } else @@ -326,8 +274,10 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C private void runWithValidSurface() { mRunWhenSurfaceIsValid = false; - if (state == State.STOPPED) + if (!NativeLibrary.IsRunning()) { + NativeLibrary.SetIsBooting(); + Thread emulationThread = new Thread(() -> { if (loadPreviousTemporaryState) @@ -344,7 +294,7 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C }, "NativeEmulation"); emulationThread.start(); } - else if (state == State.PAUSED) + else { if (!EmulationActivity.getHasUserPausedEmulation() && !NativeLibrary.IsShowingAlertMessage()) @@ -353,11 +303,6 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C NativeLibrary.UnPauseEmulation(); } } - else - { - Log.debug("[EmulationFragment] Bug, run called while already running."); - } - state = State.RUNNING; } } diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index eedc728ad9..1b54cd592c 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -23,6 +23,7 @@ #include "Common/CommonTypes.h" #include "Common/Event.h" #include "Common/FileUtil.h" +#include "Common/Flag.h" #include "Common/IniFile.h" #include "Common/Logging/LogManager.h" #include "Common/MsgHandler.h" @@ -81,6 +82,7 @@ Common::Event s_update_main_frame_event; std::mutex s_surface_lock; bool s_need_nonblocking_alert_msg; +Common::Flag s_is_booting; bool s_have_wm_user_stop = false; bool s_game_metadata_is_valid = false; } // Anonymous namespace @@ -247,9 +249,14 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulatio s_update_main_frame_event.Set(); } +JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetIsBooting(JNIEnv*, jclass) +{ + s_is_booting.Set(); +} + JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunning(JNIEnv*, jclass) { - return static_cast(Core::IsRunning()); + return s_is_booting.IsSet() || static_cast(Core::IsRunning()); } JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunningAndStarted(JNIEnv*, @@ -258,6 +265,12 @@ JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunnin return static_cast(Core::IsRunningAndStarted()); } +JNIEXPORT jboolean JNICALL +Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunningAndUnpaused(JNIEnv*, jclass) +{ + return static_cast(Core::GetState() == Core::State::Running); +} + JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_onGamePadEvent( JNIEnv* env, jclass, jstring jDevice, jint Button, jint Action) { @@ -561,6 +574,7 @@ static void Run(JNIEnv* env, const std::vector& paths, } } + s_is_booting.Clear(); s_need_nonblocking_alert_msg = false; surface_guard.unlock(); From 3eb07e977269b6d2ddd126f78acd6d4b4a219e71 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 8 Aug 2021 16:22:52 +0200 Subject: [PATCH 3/5] Android: Don't rely on onPause for pausing before destroying surface Fixes a crash which was uncovered (or just made more likely?) by the previous commit. --- .../fragments/EmulationFragment.java | 1 + Source/Android/jni/MainAndroid.cpp | 20 ++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java index ce35f1e98e..d4780e9605 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java @@ -186,6 +186,7 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C { Log.debug("[EmulationFragment] Surface destroyed."); NativeLibrary.SurfaceDestroyed(); + mRunWhenSurfaceIsValid = true; } public void stopEmulation() diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index 1b54cd592c..53a87da1bc 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -443,7 +443,25 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceChang JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceDestroyed(JNIEnv*, jclass) { - std::lock_guard guard(s_surface_lock); + { + // If emulation continues running without a valid surface, we will probably crash, + // so pause emulation until we get a valid surface again. EmulationFragment handles resuming. + + std::unique_lock host_identity_guard(s_host_identity_lock); + + while (s_is_booting.IsSet()) + { + // Need to wait for boot to finish before we can pause + host_identity_guard.unlock(); + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + host_identity_guard.lock(); + } + + if (Core::GetState() == Core::State::Running) + Core::SetState(Core::State::Paused); + } + + std::lock_guard surface_guard(s_surface_lock); if (g_renderer) g_renderer->ChangeSurface(nullptr); From 2cd09b8eb3bdc39f455f47c40ce165c7965b74e3 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 8 Aug 2021 14:56:07 +0200 Subject: [PATCH 4/5] Android: Remove synchronized keywords from EmulationState These methods are only being called from the GUI thread anyway... --- .../dolphinemu/dolphinemu/fragments/EmulationFragment.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java index d4780e9605..aecf2c8a83 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java @@ -231,7 +231,7 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C this.temporaryStatePath = temporaryStatePath; } - public synchronized void run(boolean isActivityRecreated) + public void run(boolean isActivityRecreated) { if (isActivityRecreated) { @@ -264,7 +264,7 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C } } - public synchronized void newSurface() + public void newSurface() { if (mRunWhenSurfaceIsValid) { From 53d7d595e694eab21eae2e44f988242063518598 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 8 Aug 2021 15:01:12 +0200 Subject: [PATCH 5/5] Android: Remove the EmulationState class The purpose of this class was to keep track of state which the emulation core was already keeping track of. This is rather risky - if we update the state of one of the two without updating the other, the two become out of sync, leading to some rather confusing problems. This duplicated state was removed from EmulationState in the previous commits, so now there isn't much left in the class. Might as well move its members directly into EmulationFragment. --- .../fragments/EmulationFragment.java | 133 ++++++++---------- 1 file changed, 57 insertions(+), 76 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java index aecf2c8a83..4e9e864137 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.java @@ -32,7 +32,9 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C private InputOverlay mInputOverlay; - private EmulationState mEmulationState; + private String[] mGamePaths; + private boolean mRunWhenSurfaceIsValid; + private boolean mLoadPreviousTemporaryState; private EmulationActivity activity; @@ -73,8 +75,7 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C // So this fragment doesn't restart on configuration changes; i.e. rotation. setRetainInstance(true); - String[] gamePaths = getArguments().getStringArray(KEY_GAMEPATHS); - mEmulationState = new EmulationState(gamePaths, getTemporaryStateFilePath()); + mGamePaths = getArguments().getStringArray(KEY_GAMEPATHS); } /** @@ -117,7 +118,7 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C public void onResume() { super.onResume(); - mEmulationState.run(activity.isActivityRecreated()); + run(activity.isActivityRecreated()); } @Override @@ -178,7 +179,10 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C { Log.debug("[EmulationFragment] Surface changed. Resolution: " + width + "x" + height); NativeLibrary.SurfaceChanged(holder.getSurface()); - mEmulationState.newSurface(); + if (mRunWhenSurfaceIsValid) + { + runWithValidSurface(); + } } @Override @@ -218,91 +222,68 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C return mInputOverlay != null && mInputOverlay.isInEditMode(); } - private static class EmulationState + private void run(boolean isActivityRecreated) { - private final String[] mGamePaths; - private boolean mRunWhenSurfaceIsValid; - private boolean loadPreviousTemporaryState; - private final String temporaryStatePath; - - EmulationState(String[] gamePaths, String temporaryStatePath) + if (isActivityRecreated) { - mGamePaths = gamePaths; - this.temporaryStatePath = temporaryStatePath; + if (NativeLibrary.IsRunning()) + { + mLoadPreviousTemporaryState = false; + deleteFile(getTemporaryStateFilePath()); + } + else + { + mLoadPreviousTemporaryState = true; + } + } + else + { + Log.debug("[EmulationFragment] activity resumed or fresh start"); + mLoadPreviousTemporaryState = false; + // activity resumed without being killed or this is the first run + deleteFile(getTemporaryStateFilePath()); } - public void run(boolean isActivityRecreated) + // If the surface is set, run now. Otherwise, wait for it to get set. + if (NativeLibrary.HasSurface()) { - if (isActivityRecreated) + runWithValidSurface(); + } + else + { + mRunWhenSurfaceIsValid = true; + } + } + + private void runWithValidSurface() + { + mRunWhenSurfaceIsValid = false; + if (!NativeLibrary.IsRunning()) + { + NativeLibrary.SetIsBooting(); + + Thread emulationThread = new Thread(() -> { - if (NativeLibrary.IsRunning()) + if (mLoadPreviousTemporaryState) { - loadPreviousTemporaryState = false; - deleteFile(temporaryStatePath); + Log.debug("[EmulationFragment] Starting emulation thread from previous state."); + NativeLibrary.Run(mGamePaths, getTemporaryStateFilePath(), true); } else { - loadPreviousTemporaryState = true; + Log.debug("[EmulationFragment] Starting emulation thread."); + NativeLibrary.Run(mGamePaths); } - } - else - { - Log.debug("[EmulationFragment] activity resumed or fresh start"); - loadPreviousTemporaryState = false; - // activity resumed without being killed or this is the first run - deleteFile(temporaryStatePath); - } - - // If the surface is set, run now. Otherwise, wait for it to get set. - if (NativeLibrary.HasSurface()) - { - runWithValidSurface(); - } - else - { - mRunWhenSurfaceIsValid = true; - } + EmulationActivity.stopIgnoringLaunchRequests(); + }, "NativeEmulation"); + emulationThread.start(); } - - public void newSurface() + else { - if (mRunWhenSurfaceIsValid) + if (!EmulationActivity.getHasUserPausedEmulation() && !NativeLibrary.IsShowingAlertMessage()) { - runWithValidSurface(); - } - } - - private void runWithValidSurface() - { - mRunWhenSurfaceIsValid = false; - if (!NativeLibrary.IsRunning()) - { - NativeLibrary.SetIsBooting(); - - Thread emulationThread = new Thread(() -> - { - if (loadPreviousTemporaryState) - { - Log.debug("[EmulationFragment] Starting emulation thread from previous state."); - NativeLibrary.Run(mGamePaths, temporaryStatePath, true); - } - else - { - Log.debug("[EmulationFragment] Starting emulation thread."); - NativeLibrary.Run(mGamePaths); - } - EmulationActivity.stopIgnoringLaunchRequests(); - }, "NativeEmulation"); - emulationThread.start(); - } - else - { - if (!EmulationActivity.getHasUserPausedEmulation() && - !NativeLibrary.IsShowingAlertMessage()) - { - Log.debug("[EmulationFragment] Resuming emulation."); - NativeLibrary.UnPauseEmulation(); - } + Log.debug("[EmulationFragment] Resuming emulation."); + NativeLibrary.UnPauseEmulation(); } } }