From c007dd1852896efe1671ade96fa50080a030a51b Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 4 Nov 2019 17:15:13 +0100 Subject: [PATCH] Android: Replace emulation rotation crash workaround with proper fix The workaround was added in 0446a58. The underlying problem is that we must not destroy the surface while the video backend is initializing, otherwise the video backend may reference nullptr. I've also cleaned up the logic for when to destroy the surface. Note that the comment in EmulationFragment.java about only being able to destroy the surface when emulation is running is not true anymore (due to de632fc, it seems like). --- .../dolphinemu/dolphinemu/NativeLibrary.java | 2 ++ .../activities/EmulationActivity.java | 15 +++++-------- .../fragments/EmulationFragment.java | 22 ++++++++----------- Source/Android/jni/MainAndroid.cpp | 8 +++++++ Source/Core/Core/Core.cpp | 12 +++++++++- Source/Core/Core/Core.h | 1 + 6 files changed, 36 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 dc3c48c338..0595100e09 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 @@ -400,6 +400,8 @@ public final class NativeLibrary */ public static native void StopEmulation(); + public static native void WaitUntilDoneBooting(); + /** * Returns true if emulation is running (or is paused). */ diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.java index 518a7bc38c..ad97625af0 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.java @@ -235,10 +235,6 @@ public final class EmulationActivity extends AppCompatActivity { super.onCreate(savedInstanceState); - // Find the EmulationFragment - mEmulationFragment = (EmulationFragment) getSupportFragmentManager() - .findFragmentById(R.id.frame_emulation_fragment); - if (savedInstanceState == null) { // Get params we were passed @@ -251,9 +247,7 @@ public final class EmulationActivity extends AppCompatActivity } else { - // Could have recreated the activity(rotate) before creating the fragment. If the fragment - // doesn't exist, treat this as a new start. - activityRecreated = mEmulationFragment != null; + activityRecreated = true; restoreState(savedInstanceState); } @@ -311,9 +305,10 @@ public final class EmulationActivity extends AppCompatActivity setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_SENSOR_LANDSCAPE); } - if (!(mDeviceHasTouchScreen && lockLandscape && - getResources().getConfiguration().orientation == Configuration.ORIENTATION_PORTRAIT) && - mEmulationFragment == null) + // Find or create the EmulationFragment + mEmulationFragment = (EmulationFragment) getSupportFragmentManager() + .findFragmentById(R.id.frame_emulation_fragment); + if (mEmulationFragment == null) { mEmulationFragment = EmulationFragment.newInstance(mPaths); getSupportFragmentManager().beginTransaction() 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 1ff9ba8ea0..64959601bb 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 @@ -316,8 +316,6 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C state = State.PAUSED; Log.debug("[EmulationFragment] Pausing emulation."); - // Release the surface before pausing, since emulation has to be running for that. - NativeLibrary.SurfaceDestroyed(); NativeLibrary.PauseEmulation(); } else @@ -381,19 +379,17 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C mSurface = null; Log.debug("[EmulationFragment] Surface destroyed."); - if (state == State.RUNNING) + if (state != State.STOPPED) { - NativeLibrary.SurfaceDestroyed(); - state = State.PAUSED; - } - else if (state == State.PAUSED) - { - Log.warning("[EmulationFragment] Surface cleared while emulation paused."); - } - else - { - Log.warning("[EmulationFragment] Surface cleared while emulation stopped."); + // In order to avoid dereferencing nullptr, we must not destroy the surface while booting + // the core, so wait here if necessary. An easy (but not 100% consistent) way to reach + // this method while the core is booting is by having landscape orientation lock enabled + // and starting emulation while the phone is in portrait mode, leading to the activity + // being recreated very soon after NativeLibrary.Run has been called. + NativeLibrary.WaitUntilDoneBooting(); } + + NativeLibrary.SurfaceDestroyed(); } } diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index e1b19314c3..66d7b0a1bb 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -197,6 +197,8 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_PauseEmulati jobject obj); JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulation(JNIEnv* env, jobject obj); +JNIEXPORT void JNICALL +Java_org_dolphinemu_dolphinemu_NativeLibrary_WaitUntilDoneBooting(JNIEnv* env, jobject obj); JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunning(JNIEnv* env, jobject obj); JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_onGamePadEvent( @@ -283,6 +285,12 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulatio s_update_main_frame_event.Set(); // Kick the waiting event } +JNIEXPORT void JNICALL +Java_org_dolphinemu_dolphinemu_NativeLibrary_WaitUntilDoneBooting(JNIEnv* env, jobject obj) +{ + Core::WaitUntilDoneBooting(); +} + JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunning(JNIEnv* env, jobject obj) { diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index 5908cc1347..4030ec9acc 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -94,6 +94,7 @@ static bool s_is_stopping = false; static bool s_hardware_initialized = false; static bool s_is_started = false; static Common::Flag s_is_booting; +static Common::Event s_done_booting; static std::thread s_emu_thread; static StateChangedCallbackFunc s_on_state_changed_callback; @@ -236,6 +237,8 @@ bool Init(std::unique_ptr boot, const WindowSystemInfo& wsi) g_video_backend->PrepareWindow(wsi); // Start the emu thread + s_done_booting.Reset(); + s_is_booting.Set(); s_emu_thread = std::thread(EmuThread, std::move(boot), wsi); return true; } @@ -412,11 +415,11 @@ static void FifoPlayerThread(const std::optional& savestate_path, static void EmuThread(std::unique_ptr boot, WindowSystemInfo wsi) { const SConfig& core_parameter = SConfig::GetInstance(); - s_is_booting.Set(); if (s_on_state_changed_callback) s_on_state_changed_callback(State::Starting); Common::ScopeGuard flag_guard{[] { s_is_booting.Clear(); + s_done_booting.Set(); s_is_started = false; s_is_stopping = false; s_wants_determinism = false; @@ -539,6 +542,7 @@ static void EmuThread(std::unique_ptr boot, WindowSystemInfo wsi // The hardware is initialized. s_hardware_initialized = true; s_is_booting.Clear(); + s_done_booting.Set(); // Set execution state to known values (CPU/FIFO/Audio Paused) CPU::Break(); @@ -662,6 +666,12 @@ State GetState() return State::Uninitialized; } +void WaitUntilDoneBooting() +{ + if (s_is_booting.IsSet() || !s_hardware_initialized) + s_done_booting.Wait(); +} + static std::string GenerateScreenshotFolderPath() { const std::string& gameId = SConfig::GetInstance().GetGameID(); diff --git a/Source/Core/Core/Core.h b/Source/Core/Core/Core.h index 530079b8ab..4015b98e15 100644 --- a/Source/Core/Core/Core.h +++ b/Source/Core/Core/Core.h @@ -56,6 +56,7 @@ bool WantsDeterminism(); // [NOT THREADSAFE] For use by Host only void SetState(State state); State GetState(); +void WaitUntilDoneBooting(); void SaveScreenShot(bool wait_for_completion = false); void SaveScreenShot(std::string_view name, bool wait_for_completion = false);