Skip to content

Commit

Permalink
Android: Replace emulation rotation crash workaround with proper fix
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
JosJuice committed Nov 5, 2019
1 parent 0f4c971 commit c007dd1
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,8 @@ public static native String GetConfig(String configFile, String Section, String
*/
public static native void StopEmulation();

public static native void WaitUntilDoneBooting();

/**
* Returns true if emulation is running (or is paused).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,6 @@ protected void onCreate(Bundle savedInstanceState)
{
super.onCreate(savedInstanceState);

// Find the EmulationFragment
mEmulationFragment = (EmulationFragment) getSupportFragmentManager()
.findFragmentById(R.id.frame_emulation_fragment);

if (savedInstanceState == null)
{
// Get params we were passed
Expand All @@ -251,9 +247,7 @@ protected void onCreate(Bundle savedInstanceState)
}
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);
}

Expand Down Expand Up @@ -311,9 +305,10 @@ protected void onCreate(Bundle savedInstanceState)
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,6 @@ public synchronized void pause()
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
Expand Down Expand Up @@ -381,19 +379,17 @@ public synchronized void clearSurface()
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();
}
}

Expand Down
8 changes: 8 additions & 0 deletions Source/Android/jni/MainAndroid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
{
Expand Down
12 changes: 11 additions & 1 deletion Source/Core/Core/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -236,6 +237,8 @@ bool Init(std::unique_ptr<BootParameters> 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;
}
Expand Down Expand Up @@ -412,11 +415,11 @@ static void FifoPlayerThread(const std::optional<std::string>& savestate_path,
static void EmuThread(std::unique_ptr<BootParameters> 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;
Expand Down Expand Up @@ -539,6 +542,7 @@ static void EmuThread(std::unique_ptr<BootParameters> 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();
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions Source/Core/Core/Core.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit c007dd1

Please sign in to comment.