Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android: Call UICommon::Init at app start instead of emulation start #8190

Merged
merged 4 commits into from
Aug 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,29 @@ public static native String GetConfig(String configFile, String Section, String

public static native int DefaultCPUCore();

public static native void ReloadConfig();

/**
* Initializes the native parts of the app.
*
* Should be called at app start before running any other native code
* (other than the native methods in DirectoryInitialization).
*/
public static native void Initialize();

/**
* Tells analytics that Dolphin has been started.
*
* Since users typically don't explicitly close Android apps, it's appropriate to
* call this not only when the app starts but also when the user returns to the app
* after not using it for a significant amount of time.
*/
public static native void ReportStartToAnalytics();

/**
* Begins emulation.
*/
public static native void Run(String[] path, boolean firstOpen);
public static native void Run(String[] path);

/**
* Begins emulation from the specified savestate.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import android.text.TextUtils;

import org.dolphinemu.dolphinemu.NativeLibrary;
import org.dolphinemu.dolphinemu.features.settings.ui.SettingsActivityView;
import org.dolphinemu.dolphinemu.features.settings.utils.SettingsFile;

Expand Down Expand Up @@ -175,6 +176,9 @@ public void saveSettings(SettingsActivityView view)

SettingsFile.saveFile(fileName, iniSections, view);
}

// Notify the native code of the changes
NativeLibrary.ReloadConfig();
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,7 @@ public void onCreate(Bundle savedInstanceState)
mPreferences = PreferenceManager.getDefaultSharedPreferences(getActivity());

String[] gamePaths = getArguments().getStringArray(KEY_GAMEPATHS);
SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(getActivity());
boolean firstOpen = preferences.getBoolean(StartupHandler.NEW_SESSION, true);
SharedPreferences.Editor sPrefsEditor = preferences.edit();
sPrefsEditor.putBoolean(StartupHandler.NEW_SESSION, false);
sPrefsEditor.apply();
mEmulationState = new EmulationState(gamePaths, getTemporaryStateFilePath(), firstOpen);
mEmulationState = new EmulationState(gamePaths, getTemporaryStateFilePath());
}

/**
Expand Down Expand Up @@ -271,12 +266,10 @@ private enum State
private Surface mSurface;
private boolean mRunWhenSurfaceIsValid;
private boolean loadPreviousTemporaryState;
private boolean firstOpen;
private final String temporaryStatePath;

EmulationState(String[] gamePaths, String temporaryStatePath, boolean firstOpen)
EmulationState(String[] gamePaths, String temporaryStatePath)
{
this.firstOpen = firstOpen;
mGamePaths = gamePaths;
this.temporaryStatePath = temporaryStatePath;
// Starting state is stopped.
Expand Down Expand Up @@ -420,7 +413,7 @@ private void runWithValidSurface()
else
{
Log.debug("[EmulationFragment] Starting emulation thread.");
NativeLibrary.Run(mGamePaths, firstOpen);
NativeLibrary.Run(mGamePaths);
}
}, "NativeEmulation");
mEmulationThread.start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.dolphinemu.dolphinemu.model.GameFile;
import org.dolphinemu.dolphinemu.model.GameFileCache;
import org.dolphinemu.dolphinemu.ui.platform.Platform;
import org.dolphinemu.dolphinemu.utils.AfterDirectoryInitializationRunner;

import java.io.File;
import java.util.ArrayList;
Expand Down Expand Up @@ -94,7 +95,8 @@ private static void startService(Context context, String action)
*/
public static void startLoad(Context context)
{
startService(context, ACTION_LOAD);
new AfterDirectoryInitializationRunner().run(context,
() -> startService(context, ACTION_LOAD));
}

/**
Expand All @@ -104,7 +106,8 @@ public static void startLoad(Context context)
*/
public static void startRescan(Context context)
{
startService(context, ACTION_RESCAN);
new AfterDirectoryInitializationRunner().run(context,
() -> startService(context, ACTION_RESCAN));
}

public static GameFile addOrGet(String gamePath)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package org.dolphinemu.dolphinemu.utils;

import android.content.Context;
import android.content.IntentFilter;
import android.support.v4.content.LocalBroadcastManager;

public class AfterDirectoryInitializationRunner
{
private DirectoryStateReceiver directoryStateReceiver;

/**
* Executes a Runnable after directory initialization has finished.
*
* If this is called when directory initialization already is done,
* the Runnable will be executed immediately. If this is called before
* directory initialization is done, the Runnable will be executed
* after directory initialization finishes successfully, or never
* in case directory initialization doesn't finish successfully.
*
* Calling this function multiple times per object is not supported.
*/
public void run(Context context, Runnable runnable)
{
if (!DirectoryInitialization.areDolphinDirectoriesReady())
{
// Wait for directories to get initialized
IntentFilter statusIntentFilter = new IntentFilter(
DirectoryInitialization.BROADCAST_ACTION);

directoryStateReceiver = new DirectoryStateReceiver(directoryInitializationState ->
{
if (directoryInitializationState ==
DirectoryInitialization.DirectoryInitializationState.DOLPHIN_DIRECTORIES_INITIALIZED)
{
LocalBroadcastManager.getInstance(context).unregisterReceiver(directoryStateReceiver);
directoryStateReceiver = null;
runnable.run();
}
});
// Registers the DirectoryStateReceiver and its intent filters
LocalBroadcastManager.getInstance(context).registerReceiver(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a 1:1 thing? It feels like it would just be a list of receivers, each notified in turn when something happens.
If it is indeed a list, I'd expect any older invocations to still be alive; rendering the last paragraph of the comment above slightly fuzzy.
Unless we want it to be like that; then we're probably supposed to call unregisterReceiver in case directoryStateReceiver is not null.

...not that it matters too much, since you keep creating new instances in the places where it is used; so they'd likely end up running anyways.

Copy link
Member Author

@JosJuice JosJuice Jul 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was assuming that the old directoryStateReceiver would get deallocated once we create a new directoryStateReceiver, but I suppose that might not be true since the LocalBroadcastManager keeps a reference to it... Since I'm not entirely sure how it works, I think I'll just change the comment to say that multiple calls is not a supported use case, because that's how I've been thinking of it while writing the code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'll just delete the part of the comment that mentions multiple calls.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, but if it works anything close to C# wrt the GC, it should still be referenced by the LocalBroadcastManager (and the closure itself should still be referencing the passed runnable, keeping it alive as well)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard my previous comment – I've realized that multiple calls should not be encouraged, because if old DirectoryStateReceivers indeed are kept around (which is likely), the way we do unregistration means that newer calls will get dropped rather than old calls. I will make the comment say that multiple calls is not a supported use case.

directoryStateReceiver,
statusIntentFilter);
}
else
{
runnable.run();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

public class Analytics
{
private static DirectoryStateReceiver directoryStateReceiver;

private static final String analyticsAsked =
Settings.SECTION_ANALYTICS + "_" + SettingsFile.KEY_ANALYTICS_PERMISSION_ASKED;
private static final String analyticsEnabled =
Expand All @@ -35,31 +33,8 @@ public static void checkAnalyticsInit(Context context)
SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context);
if (!preferences.getBoolean(analyticsAsked, false))
{
if (!DirectoryInitialization.areDolphinDirectoriesReady())
{
// Wait for directories to get initialized
IntentFilter statusIntentFilter = new IntentFilter(
DirectoryInitialization.BROADCAST_ACTION);

directoryStateReceiver = new DirectoryStateReceiver(directoryInitializationState ->
{
if (directoryInitializationState ==
DirectoryInitialization.DirectoryInitializationState.DOLPHIN_DIRECTORIES_INITIALIZED)
{
LocalBroadcastManager.getInstance(context).unregisterReceiver(directoryStateReceiver);
directoryStateReceiver = null;
showMessage(context, preferences);
}
});
// Registers the DirectoryStateReceiver and its intent filters
LocalBroadcastManager.getInstance(context).registerReceiver(
directoryStateReceiver,
statusIntentFilter);
}
else
{
showMessage(context, preferences);
}
new AfterDirectoryInitializationRunner().run(context,
() -> showMessage(context, preferences));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ private static void init(Context context)
{
initializeInternalStorage(context);
initializeExternalStorage(context);
NativeLibrary.Initialize();
NativeLibrary.ReportStartToAnalytics();

directoryState = DirectoryInitializationState.DOLPHIN_DIRECTORIES_INITIALIZED;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@
import android.support.v4.app.FragmentActivity;
import android.text.TextUtils;

import org.dolphinemu.dolphinemu.NativeLibrary;
import org.dolphinemu.dolphinemu.activities.EmulationActivity;

import java.util.Date;

public final class StartupHandler
{
public static final String NEW_SESSION = "NEW_SESSION";
public static final String LAST_CLOSED = "LAST_CLOSED";
public static final Long SESSION_TIMEOUT = 21600000L; // 6 hours in milliseconds
public static final long SESSION_TIMEOUT = 21600000L; // 6 hours in milliseconds

public static void HandleInit(FragmentActivity parent)
{
Expand Down Expand Up @@ -66,15 +66,13 @@ public static void setSessionTime(Context context)
*/
public static void checkSessionReset(Context context)
{
Long currentTime = new Date(System.currentTimeMillis()).getTime();
long currentTime = new Date(System.currentTimeMillis()).getTime();
SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(context);
Long lastOpen = preferences.getLong(LAST_CLOSED, 0);
long lastOpen = preferences.getLong(LAST_CLOSED, 0);
if (currentTime > (lastOpen + SESSION_TIMEOUT))
{
// Passed at emulation start to trigger first open event.
SharedPreferences.Editor sPrefsEditor = preferences.edit();
sPrefsEditor.putBoolean(NEW_SESSION, true);
sPrefsEditor.apply();
new AfterDirectoryInitializationRunner().run(context,
() -> NativeLibrary.ReportStartToAnalytics());
}
}
}
52 changes: 32 additions & 20 deletions Source/Android/jni/MainAndroid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,14 @@ JNIEXPORT jint JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_DefaultCPUCo
JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SetProfiling(JNIEnv* env,
jobject obj,
jboolean enable);
JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Initialize(JNIEnv* env,
jobject obj);
JNIEXPORT void JNICALL
Java_org_dolphinemu_dolphinemu_NativeLibrary_ReportStartToAnalytics(JNIEnv* env, jobject obj);
JNIEXPORT void JNICALL
Java_org_dolphinemu_dolphinemu_NativeLibrary_WriteProfileResults(JNIEnv* env, jobject obj);

JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run__Ljava_lang_String_2Z(
JNIEnv* env, jobject obj, jstring jFile, jboolean jfirstOpen);

JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run__Ljava_lang_String_2(
JNIEnv* env, jobject obj, jstring jFile);
JNIEXPORT void JNICALL
Java_org_dolphinemu_dolphinemu_NativeLibrary_Run__Ljava_lang_String_2Ljava_lang_String_2Z(
JNIEnv* env, jobject obj, jstring jFile, jstring jSavestate, jboolean jDeleteSavestate);
Expand Down Expand Up @@ -585,6 +587,27 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ReloadWiimot
Wiimote::LoadConfig();
}

JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ReloadConfig(JNIEnv* env,
jobject obj)
{
SConfig::GetInstance().LoadSettings();
}

JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Initialize(JNIEnv* env,
jobject obj)
{
Common::RegisterMsgAlertHandler(&MsgAlert);
Common::AndroidSetReportHandler(&ReportSend);
DolphinAnalytics::AndroidSetGetValFunc(&GetAnalyticValue);
UICommon::Init();
}

JNIEXPORT void JNICALL
Java_org_dolphinemu_dolphinemu_NativeLibrary_ReportStartToAnalytics(JNIEnv* env, jobject obj)
{
DolphinAnalytics::Instance().ReportDolphinStart(GetAnalyticValue("DEVICE_TYPE"));
}

// Returns the scale factor for imgui rendering.
// Based on the scaledDensity of the device's display metrics.
static float GetRenderSurfaceScale(JNIEnv* env)
Expand Down Expand Up @@ -630,23 +653,13 @@ static float GetRenderSurfaceScale(JNIEnv* env)
return scaled_density;
}

static void Run(JNIEnv* env, const std::vector<std::string>& paths, bool first_open,
static void Run(JNIEnv* env, const std::vector<std::string>& paths,
std::optional<std::string> savestate_path = {}, bool delete_savestate = false)
{
ASSERT(!paths.empty());
__android_log_print(ANDROID_LOG_INFO, DOLPHIN_TAG, "Running : %s", paths[0].c_str());

Common::RegisterMsgAlertHandler(&MsgAlert);
Common::AndroidSetReportHandler(&ReportSend);
DolphinAnalytics::AndroidSetGetValFunc(&GetAnalyticValue);

std::unique_lock<std::mutex> guard(s_host_identity_lock);
UICommon::Init();

if (first_open)
{
DolphinAnalytics::Instance().ReportDolphinStart(GetAnalyticValue("DEVICE_TYPE"));
}

WiimoteReal::InitAdapterClass();

Expand Down Expand Up @@ -679,7 +692,6 @@ static void Run(JNIEnv* env, const std::vector<std::string>& paths, bool first_o

Core::Shutdown();
ButtonManager::Shutdown();
UICommon::Shutdown();
guard.unlock();

if (s_surf)
Expand All @@ -689,17 +701,17 @@ static void Run(JNIEnv* env, const std::vector<std::string>& paths, bool first_o
}
}

JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run___3Ljava_lang_String_2Z(
JNIEnv* env, jobject obj, jobjectArray jPaths, jboolean jfirstOpen)
JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run___3Ljava_lang_String_2(
JNIEnv* env, jobject obj, jobjectArray jPaths)
{
Run(env, JStringArrayToVector(env, jPaths), jfirstOpen);
Run(env, JStringArrayToVector(env, jPaths));
}

JNIEXPORT void JNICALL
Java_org_dolphinemu_dolphinemu_NativeLibrary_Run___3Ljava_lang_String_2Ljava_lang_String_2Z(
JNIEnv* env, jobject obj, jobjectArray jPaths, jstring jSavestate, jboolean jDeleteSavestate)
{
Run(env, JStringArrayToVector(env, jPaths), false, GetJString(env, jSavestate), jDeleteSavestate);
Run(env, JStringArrayToVector(env, jPaths), GetJString(env, jSavestate), jDeleteSavestate);
}

JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_ChangeDisc(JNIEnv* env,
Expand Down
Loading