From 073ef616430d0c029be7601ea8e4fd581ed7cecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Fri, 5 Sep 2025 10:29:31 +0200 Subject: [PATCH] Merge the two functions, fix JNI lifetime issues Fixes the last part of #10885 finally. --- Common/File/AndroidStorage.cpp | 27 +++- Common/File/AndroidStorage.h | 4 +- android/jni/app-android.cpp | 129 +++++++----------- .../src/org/ppsspp/ppsspp/NativeActivity.java | 1 - .../org/ppsspp/ppsspp/ShortcutActivity.java | 32 ++--- 5 files changed, 93 insertions(+), 100 deletions(-) diff --git a/Common/File/AndroidStorage.cpp b/Common/File/AndroidStorage.cpp index 5004b716cf..fe74605c35 100644 --- a/Common/File/AndroidStorage.cpp +++ b/Common/File/AndroidStorage.cpp @@ -29,12 +29,13 @@ static jmethodID computeRecursiveDirectorySize; static jobject g_nativeActivity; static jclass g_classActivity; -void Android_StorageSetNativeActivity(jobject nativeActivity) { +void Android_StorageSetActivity(jobject nativeActivity) { g_nativeActivity = nativeActivity; } void Android_RegisterStorageCallbacks(JNIEnv * env, jobject obj) { - jclass localClass = env->GetObjectClass(obj); + jclass localClass = env->FindClass("org/ppsspp/ppsspp/PpssppActivity"); + _dbg_assert_(localClass); openContentUri = env->GetStaticMethodID(localClass, "openContentUri", "(Landroid/app/Activity;Ljava/lang/String;Ljava/lang/String;)I"); _dbg_assert_(openContentUri); @@ -69,6 +70,28 @@ void Android_RegisterStorageCallbacks(JNIEnv * env, jobject obj) { env->DeleteLocalRef(localClass); // cleanup local ref } +void Android_UnregisterStorageCallbacks(JNIEnv * env) { + if (g_classActivity) { + env->DeleteGlobalRef(g_classActivity); + g_classActivity = nullptr; + } + g_nativeActivity = nullptr; + openContentUri = nullptr; + listContentUriDir = nullptr; + contentUriCreateFile = nullptr; + contentUriCreateDirectory = nullptr; + contentUriCopyFile = nullptr; + contentUriMoveFile = nullptr; + contentUriRemoveFile = nullptr; + contentUriRenameFileTo = nullptr; + contentUriGetFileInfo = nullptr; + contentUriFileExists = nullptr; + contentUriGetFreeStorageSpace = nullptr; + filePathGetFreeStorageSpace = nullptr; + isExternalStoragePreservedLegacy = nullptr; + computeRecursiveDirectorySize = nullptr; +} + bool Android_IsContentUri(std::string_view filename) { return startsWith(filename, "content://"); } diff --git a/Common/File/AndroidStorage.h b/Common/File/AndroidStorage.h index faf8ee682a..26fdc8ada2 100644 --- a/Common/File/AndroidStorage.h +++ b/Common/File/AndroidStorage.h @@ -41,7 +41,7 @@ extern std::string g_nativeLibDir; #include -void Android_StorageSetNativeActivity(jobject nativeActivity); +void Android_StorageSetActivity(jobject nativeActivity); bool Android_IsContentUri(std::string_view uri); int Android_OpenContentUriFd(std::string_view uri, const Android_OpenContentUriMode mode); @@ -65,7 +65,9 @@ const char *Android_ErrorToString(StorageError error); // TODO: prefix doesn't do anything yet. std::vector Android_ListContentUri(const std::string &uri, const std::string &prefix, bool *exists); +// Don't need to add these below, they're only used from app-android.cpp. void Android_RegisterStorageCallbacks(JNIEnv * env, jobject obj); +void Android_UnregisterStorageCallbacks(JNIEnv * env); #else diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index eca011fbb4..70c869e263 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -554,11 +554,11 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeActivity_registerCallbacks(JNIEnv * _dbg_assert_(getDebugString); Android_RegisterStorageCallbacks(env, obj); - Android_StorageSetNativeActivity(nativeActivity); + Android_StorageSetActivity(nativeActivity); } extern "C" void Java_org_ppsspp_ppsspp_NativeActivity_unregisterCallbacks(JNIEnv *env, jobject obj) { - Android_StorageSetNativeActivity(nullptr); + Android_StorageSetActivity(nullptr); env->DeleteGlobalRef(nativeActivity); nativeActivity = nullptr; } @@ -1700,46 +1700,60 @@ static void VulkanEmuThread(ANativeWindow *wnd) { WARN_LOG(Log::G3D, "Render loop function exited."); } -extern "C" jstring Java_org_ppsspp_ppsspp_ShortcutActivity_queryGame(JNIEnv * env, jclass, jobject activity, jstring jpath) { +extern "C" JNIEXPORT jobjectArray JNICALL +Java_org_ppsspp_ppsspp_ShortcutActivity_queryGameInfo(JNIEnv * env, jclass, jobject activity, jstring jpath) { + + jobject activityRef = nullptr; + bool teardownThreadManager = false; + // Maybe we should just check nativeActivity instead. if (!g_threadManager.IsInitialized()) { - // Need a thread manager. teardownThreadManager = true; g_threadManager.Init(1, 1); g_logManager.SetOutputsEnabled(LogOutput::Stdio); g_logManager.SetAllLogLevels(LogLevel::LDEBUG); - Android_StorageSetNativeActivity(activity); - Android_RegisterStorageCallbacks(env, activity); + activityRef = env->NewGlobalRef(activity); + Android_StorageSetActivity(activityRef); + Android_RegisterStorageCallbacks(env, activityRef); INFO_LOG(Log::System, "No thread manager - initializing one"); } Path path = Path(GetJavaString(env, jpath)); + INFO_LOG(Log::System, "queryGameInfo(%s)", path.c_str()); - INFO_LOG(Log::System, "queryGameName(%s)", path.c_str()); - - std::string result; + std::string gameName; + jbyteArray gameIcon = nullptr; GameInfoCache *cache = new GameInfoCache(); - std::shared_ptr info = cache->GetInfo(nullptr, path, GameInfoFlags::PARAM_SFO); - // Wait until it's done: this is synchronous, unfortunately. + std::shared_ptr info = cache->GetInfo(nullptr, path, GameInfoFlags::PARAM_SFO | GameInfoFlags::ICON); + if (info) { INFO_LOG(Log::System, "GetInfo successful, waiting"); - while (!info->Ready(GameInfoFlags::PARAM_SFO)) { - sleep_ms(1, "info-poll"); + + // Wait for both name and icon + int attempts = 1000; + while ((!info->Ready(GameInfoFlags::PARAM_SFO) || !info->Ready(GameInfoFlags::ICON)) && attempts > 0) { + sleep_ms(1, "info-icon-poll"); + attempts--; } INFO_LOG(Log::System, "Done waiting"); + if (info->fileType != IdentifiedFileType::UNKNOWN) { - result = info->GetTitle(); - - // Pretty arbitrary, but the home screen will often truncate titles. - // Let's remove "The " from names since it's common in English titles. - if (result.length() > strlen("The ") && startsWithNoCase(result, "The ")) { - result = result.substr(strlen("The ")); + // Get the game title + gameName = info->GetTitle(); + if (gameName.length() > strlen("The ") && startsWithNoCase(gameName, "The ")) { + gameName = gameName.substr(strlen("The ")); } + INFO_LOG(Log::System, "Got name: '%s'", gameName.c_str()); - INFO_LOG(Log::System, "queryGameName: Got '%s'", result.c_str()); + // Get the game icon if available + if (info->Ready(GameInfoFlags::ICON) && !info->icon.data.empty()) { + INFO_LOG(Log::System, "Got icon"); + gameIcon = env->NewByteArray((jsize)info->icon.data.size()); + env->SetByteArrayRegion(gameIcon, 0, (jsize)info->icon.data.size(), (const jbyte *)info->icon.data.data()); + } } else { - INFO_LOG(Log::System, "queryGameName: Filetype unknown"); + INFO_LOG(Log::System, "Failed to query game info"); } } else { INFO_LOG(Log::System, "No info from cache"); @@ -1748,69 +1762,24 @@ extern "C" jstring Java_org_ppsspp_ppsspp_ShortcutActivity_queryGame(JNIEnv * en if (teardownThreadManager) { g_logManager.SetOutputsEnabled((LogOutput)0); + Android_UnregisterStorageCallbacks(env); + Android_StorageSetActivity(nullptr); g_threadManager.Teardown(); } - return env->NewStringUTF(result.c_str()); -} + // Construct a Java Object[] with two entries: name (String), icon (byte[]) + jobjectArray result = env->NewObjectArray(2, env->FindClass("java/lang/Object"), nullptr); + jstring jname = env->NewStringUTF(gameName.c_str()); + env->SetObjectArrayElement(result, 0, jname); + env->DeleteLocalRef(jname); -extern "C" -JNIEXPORT jbyteArray JNICALL -Java_org_ppsspp_ppsspp_ShortcutActivity_queryGameIcon(JNIEnv * env, jclass, jobject activity, jstring jpath) { - bool teardownThreadManager = false; - if (!g_threadManager.IsInitialized()) { - // Need a thread manager. - teardownThreadManager = true; - g_threadManager.Init(1, 1); - g_logManager.SetOutputsEnabled(LogOutput::Stdio); - g_logManager.SetAllLogLevels(LogLevel::LDEBUG); - - Android_StorageSetNativeActivity(activity); - Android_RegisterStorageCallbacks(env, activity); - INFO_LOG(Log::System, "No thread manager - initializing one"); + if (gameIcon != nullptr) { + env->SetObjectArrayElement(result, 1, gameIcon); + env->DeleteLocalRef(gameIcon); } - // TODO: implement requestIcon() - - Path path = Path(GetJavaString(env, jpath)); - - INFO_LOG(Log::System, "queryGameIcon(%s)", path.c_str()); - - jbyteArray result = nullptr; - - GameInfoCache *cache = new GameInfoCache(); - std::shared_ptr info = cache->GetInfo(nullptr, path, GameInfoFlags::ICON); - // Wait until it's done: this is synchronous, unfortunately. - if (info) { - INFO_LOG(Log::System, "GetInfo successful, waiting"); - int attempts = 1000; - while (!info->Ready(GameInfoFlags::ICON)) { - sleep_ms(1, "icon-poll"); - attempts--; - if (!attempts) { - break; - } - } - INFO_LOG(Log::System, "Done waiting"); - if (info->Ready(GameInfoFlags::ICON)) { - if (!info->icon.data.empty()) { - INFO_LOG(Log::System, "requestIcon: Got icon"); - result = env->NewByteArray((jsize)info->icon.data.size()); - env->SetByteArrayRegion(result, 0, (jsize)info->icon.data.size(), (const jbyte *)info->icon.data.data()); - } - } else { - INFO_LOG(Log::System, "requestIcon: Filetype unknown"); - } - } else { - INFO_LOG(Log::System, "No info from cache"); - } - - delete cache; - - if (teardownThreadManager) { - g_logManager.SetOutputsEnabled((LogOutput)0); - g_threadManager.Teardown(); - } - - return result; + if (activityRef) { + env->DeleteGlobalRef(activityRef); + } + return result; } diff --git a/android/src/org/ppsspp/ppsspp/NativeActivity.java b/android/src/org/ppsspp/ppsspp/NativeActivity.java index 37c0a3d13d..6e2c566f6e 100644 --- a/android/src/org/ppsspp/ppsspp/NativeActivity.java +++ b/android/src/org/ppsspp/ppsspp/NativeActivity.java @@ -267,7 +267,6 @@ public abstract class NativeActivity extends AppCompatActivity implements Sensor if (list == null) { Log.i(TAG, "getSdCardPaths: Attempting fallback"); // Try another method. - String removableStoragePath; list = new ArrayList<>(); File[] fileList = new File("/storage/").listFiles(); if (fileList != null) { diff --git a/android/src/org/ppsspp/ppsspp/ShortcutActivity.java b/android/src/org/ppsspp/ppsspp/ShortcutActivity.java index 568089efbc..8f73c786bc 100644 --- a/android/src/org/ppsspp/ppsspp/ShortcutActivity.java +++ b/android/src/org/ppsspp/ppsspp/ShortcutActivity.java @@ -74,8 +74,7 @@ public class ShortcutActivity extends Activity { finish(); } - public static native String queryGameName(Activity activity, String path); - public static native byte[] queryGameIcon(Activity activity, String path); + private static native Object[] queryGameInfo(android.app.Activity activity, String path); // Create shortcut as response for ACTION_CREATE_SHORTCUT intent. private void respondToShortcutRequest(@NonNull Uri uri) { @@ -84,7 +83,7 @@ public class ShortcutActivity extends Activity { // target class. Add any parameter as data. Intent shortcutIntent = new Intent(getApplicationContext(), PpssppActivity.class); shortcutIntent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_SINGLE_TOP); - Log.i(TAG, "Shortcut URI: " + uri.toString()); + Log.i(TAG, "Shortcut URI: " + uri); shortcutIntent.setData(uri); String path = uri.toString(); shortcutIntent.putExtra(PpssppActivity.SHORTCUT_EXTRA_KEY, path); @@ -116,16 +115,18 @@ public class ShortcutActivity extends Activity { String name = pathSegments[pathSegments.length - 1]; PpssppActivity.CheckABIAndLoadLibrary(); - String gameName = queryGameName(this, path); - byte [] iconData = null; - if (gameName.isEmpty()) { - Log.i(TAG, "Failed to retrieve game name - ignoring."); - // This probably happened because PPSSPP isn't running so the GameInfoCache isn't working. - // Let's just continue with our fallback name until we can fix that. - // return; + String gameName = name; + byte[] iconData = null; + Object[] result = queryGameInfo(this, path); + if (result != null && result.length >= 2) { + if (result[0] != null) { + gameName = (String) result[0]; // index 0 = name + } + iconData = (byte[]) result[1]; // index 1 = raw PNG/JPEG bytes, or null + + Log.i(TAG, "Game name: " + gameName); } else { - name = gameName; - iconData = queryGameIcon(this, path); + Log.e(TAG, "Bad return value from queryGameInfo"); } Log.i(TAG, "Game name: " + name + " : Creating shortcut to " + uri); @@ -136,7 +137,6 @@ public class ShortcutActivity extends Activity { responseIntent.putExtra(Intent.EXTRA_SHORTCUT_INTENT, shortcutIntent); responseIntent.putExtra(Intent.EXTRA_SHORTCUT_NAME, name); - boolean setIcon = false; if (iconData != null) { // Try to create a PNG from the iconData. Bitmap bmp = BitmapFactory.decodeByteArray(iconData, 0, iconData.length); @@ -156,13 +156,13 @@ public class ShortcutActivity extends Activity { // Bitmap scaledBitmap = Bitmap.createScaledBitmap(bmp, 144, 72, true); responseIntent.putExtra(Intent.EXTRA_SHORTCUT_ICON, paddedBitmap); } - setIcon = true; - } - if (!setIcon) { + } else { + Log.i(TAG, "No icon available, falling back to PPSSPP icon"); // Fall back to the PPSSPP icon. ShortcutIconResource iconResource = ShortcutIconResource.fromContext(this, R.mipmap.ic_launcher); responseIntent.putExtra(Intent.EXTRA_SHORTCUT_ICON_RESOURCE, iconResource); } + setResult(RESULT_OK, responseIntent); // Must call finish for result to be returned immediately