Merge pull request #21614 from hrydgard/code-cleanup

Use a proxy activity for Android file pickers, simplify handling of Vblank events

Core/HLE/sceCtrl.cpp

@@ -329,8 +329,7 @@ retry:
 	}
 }
 
-static void __CtrlVblank()
-{
+void __CtrlVblank() {
 	emuRapidFireFrames++;
 	if (emuRapidFireFrames >= emuRapidFireInterval) {
 		emuRapidFireFrames = 0;
@@ -356,10 +355,8 @@ static void __CtrlTimerUpdate(u64 userdata, int cyclesLate)
 	__CtrlDoSample();
 }
 
-void __CtrlInit()
-{
+void __CtrlInit() {
 	ctrlTimer = CoreTiming::RegisterEvent("CtrlSampleTimer", __CtrlTimerUpdate);
-	__DisplayListenVblank(__CtrlVblank);
 
 	ctrlIdleReset = -1;
 	ctrlIdleBack = -1;

Core/HLE/sceCtrl.h

@@ -69,6 +69,7 @@ const int CTRL_STICK_RIGHT = 1;
 void __CtrlInit();
 void __CtrlDoState(PointerWrap &p);
 void __CtrlShutdown();
+void __CtrlVblank();
 
 // Clears and sets selected buttons. NOTE: Clearing happens first.
 void __CtrlUpdateButtons(u32 bitsToSet, u32 bitsToClear);

Core/HLE/sceDisplay.cpp

@@ -180,7 +180,6 @@ static void ScheduleLagSync(int over = 0) {
 
 void __DisplayInit() {
 	__DisplaySetFramerate();
-	DisplayHWReset();
 	hasSetMode = false;
 	mode = 0;
 	resumeMode = 0;

Core/HLE/sceIo.cpp

@@ -598,7 +598,7 @@ static void __IoWakeManager(CoreLifecycle stage) {
 	}
 }
 
-static void __IoVblank() {
+void __IoVblank() {
 	// We update memstick status here just to avoid possible thread safety issues.
 	// It doesn't actually need to be on a vblank.
 
@@ -699,7 +699,6 @@ void __IoInit() {
 	MemoryStick_Init();
 	lastMemStickState = MemoryStick_State();
 	lastMemStickFatState = MemoryStick_FatState();
-	__DisplayListenVblank(__IoVblank);
 }
 
 void __IoDoState(PointerWrap &p) {

Core/HLE/sceIo.h

@@ -23,6 +23,7 @@ class KernelObject;
 void __IoInit();
 void __IoDoState(PointerWrap &p);
 void __IoShutdown();
+void __IoVblank();
 
 struct ScePspDateTime;
 struct tm;
@@ -44,3 +45,4 @@ void Register_IoFileMgrForKernel();
 void Register_IoFileMgrForUser();
 void Register_StdioForKernel();
 void Register_StdioForUser();
+

Core/HW/Display.cpp

@@ -27,13 +27,14 @@
 #include "Core/System.h"
 #include "Core/CoreTiming.h"
 #include "Core/HLE/sceKernel.h"
+#include "Core/HLE/sceCtrl.h"
+#include "Core/HLE/sceIo.h"
 #include "Core/HW/Display.h"
 #include "GPU/GPU.h"
 #include "GPU/GPUCommon.h"
 
 // Called when vblank happens (like an internal interrupt.)  Not part of state, should be static.
 static std::mutex listenersLock;
-static std::vector<VblankCallback> vblankListeners;
 typedef std::pair<FlipCallback, void *> FlipListener;
 static std::vector<FlipListener> flipListeners;
 
@@ -250,15 +251,9 @@ void DisplayFireVblankStart() {
 
 void DisplayFireVblankEnd() {
 	isVblank = 0;
-	std::vector<VblankCallback> toCall;
-	{
-		std::lock_guard<std::mutex> guard(listenersLock);
-		toCall = vblankListeners;
-	}
 
-	for (VblankCallback cb : toCall) {
-		cb();
-	}
+	__IoVblank();
+	__CtrlVblank();
 }
 
 void DisplayFireFlip() {
@@ -279,11 +274,6 @@ void DisplayFireActualFlip() {
 	actualFlips++;
 }
 
-void __DisplayListenVblank(VblankCallback callback) {
-	std::lock_guard<std::mutex> guard(listenersLock);
-	vblankListeners.push_back(callback);
-}
-
 void __DisplayListenFlip(FlipCallback callback, void *userdata) {
 	std::lock_guard<std::mutex> guard(listenersLock);
 	flipListeners.emplace_back(callback, userdata);
@@ -296,13 +286,6 @@ void __DisplayForgetFlip(FlipCallback callback, void *userdata) {
 	}), flipListeners.end());
 }
 
-// This is called when launching a new executable. We do clear vblankListeners here, they will get
-// set up again if needed. However we can't clear fliplisteners, that is used for RetroAchievements updates.
-void DisplayHWReset() {
-	std::lock_guard<std::mutex> guard(listenersLock);
-	vblankListeners.clear();
-}
-
 // This is called on game bootup.
 void DisplayHWInit() {
 	flipListeners.clear();
@@ -330,7 +313,6 @@ void DisplayHWInit() {
 
 void DisplayHWShutdown() {
 	std::lock_guard<std::mutex> guard(listenersLock);
-	vblankListeners.clear();
 	flipListeners.clear();
 }
 

Core/HW/Display.h

@@ -22,9 +22,6 @@
 
 class PointerWrap;
 
-typedef std::function<void()> VblankCallback;
-// Listen for vblank events. Callbacks are cleared in DisplayHWShutdown().
-void __DisplayListenVblank(VblankCallback callback);
 typedef void (*FlipCallback)(void *userdata);
 void __DisplayListenFlip(FlipCallback callback, void *userdata);
 void __DisplayForgetFlip(FlipCallback callback, void *userdata);
@@ -52,9 +49,6 @@ void DisplayFireVblankEnd();
 void DisplayFireFlip();
 void DisplayFireActualFlip();
 
-int DisplayCalculateFrameSkip();
-
 void DisplayHWInit();
-void DisplayHWReset();
 void DisplayHWShutdown();
 void DisplayHWDoState(PointerWrap &p, int hleCompatV2);

GPU/Common/TextureCacheCommon.cpp

@@ -2825,16 +2825,20 @@ bool TextureCacheCommon::PrepareBuildTexture(BuildTexturePlan &plan, TexCacheEnt
 		plan.scaleFactor = plan.scaleFactor > 4 ? 4 : (plan.scaleFactor > 2 ? 2 : 1);
 	}
 
+	if (plan.hardwareScaling) {
+		plan.scaleFactor = shaderScaleFactor_;
+	}
+
 	bool isFakeMipmapChange = false;
 	if (plan.badMipSizes) {
 		isFakeMipmapChange = IsFakeMipmapChange();
 
 		// Check for pure 3D texture.
-		int tw = gstate.getTextureWidth(0);
-		int th = gstate.getTextureHeight(0);
+		const int tw = gstate.getTextureWidth(0);
+		const int th = gstate.getTextureHeight(0);
 		bool pure3D = true;
 		for (int i = 0; i < plan.levelsToLoad; i++) {
-			if (gstate.getTextureWidth(i) != gstate.getTextureWidth(0) || gstate.getTextureHeight(i) != gstate.getTextureHeight(0)) {
+			if (gstate.getTextureWidth(i) != tw || gstate.getTextureHeight(i) != th) {
 				pure3D = false;
 				break;
 			}
@@ -2848,7 +2852,8 @@ bool TextureCacheCommon::PrepareBuildTexture(BuildTexturePlan &plan, TexCacheEnt
 			pure3D = false;
 		} else if (isFakeMipmapChange) {
 			// We don't want to create a volume texture, if this is a "fake mipmap change".
-			// In practice due to the compat flag, the only time we end up here is in JP Tactics Ogre.
+			// In practice due to the compat flag, the only time we end up here is in JP Tactics Ogre,
+			// or with OpenGL ES 2.0.
 			pure3D = false;
 		}
 
@@ -2861,10 +2866,6 @@ bool TextureCacheCommon::PrepareBuildTexture(BuildTexturePlan &plan, TexCacheEnt
 		plan.levelsToCreate = 1;
 	}
 
-	if (plan.hardwareScaling) {
-		plan.scaleFactor = shaderScaleFactor_;
-	}
-
 	// We generate missing mipmaps from maxLevel+1 up to this level. maxLevel can get overwritten below
 	// such as when using replacement textures - but let's keep the same amount of levels for generation.
 	// Not all backends will generate mipmaps, and in GL we can't really control the number of levels.
@@ -2873,7 +2874,7 @@ bool TextureCacheCommon::PrepareBuildTexture(BuildTexturePlan &plan, TexCacheEnt
 	plan.w = gstate.getTextureWidth(0);
 	plan.h = gstate.getTextureHeight(0);
 
-	bool isPPGETexture = entry->addr >= PSP_GetKernelMemoryBase() && entry->addr < PSP_GetKernelMemoryEnd();
+	const bool isPPGETexture = entry->addr >= PSP_GetKernelMemoryBase() && entry->addr < PSP_GetKernelMemoryEnd();
 
 	// Don't scale the PPGe texture.
 	if (isPPGETexture) {

android/AndroidManifest.xml

@@ -113,6 +113,9 @@
 		<activity android:name=".ImageResultProxyActivity"
 			android:theme="@style/Theme.ProxyTransparent"
 			android:launchMode="standard" />
+		<activity android:name=".DocumentResultProxyActivity"
+			android:theme="@style/Theme.ProxyTransparent"
+			android:launchMode="standard" />
         <profileable android:shell="true" android:enabled="true" />
     </application>
 

android/src/org/ppsspp/ppsspp/DocumentResultProxyActivity.java

@@ -0,0 +1,76 @@
+package org.ppsspp.ppsspp;
+
+import android.app.Activity;
+import android.content.Intent;
+import android.net.Uri;
+import android.os.Bundle;
+import android.util.Log;
+
+import androidx.activity.result.ActivityResultLauncher;
+import androidx.activity.result.contract.ActivityResultContracts;
+import androidx.appcompat.app.AppCompatActivity;
+import androidx.documentfile.provider.DocumentFile;
+
+public class DocumentResultProxyActivity extends AppCompatActivity {
+	public static final String TAG = "PPSSPP";
+
+	@Override
+	protected void onCreate(Bundle savedInstanceState) {
+		super.onCreate(savedInstanceState);
+
+		// Get the Intent that was meant for the picker
+		Intent pickerIntent = getIntent().getParcelableExtra("picker_intent");
+		int requestId = getIntent().getIntExtra("request_id", -1);
+
+		Log.i(TAG, "DocumentResultProxy: Setting up activity launch, requestId = " + requestId + (savedInstanceState == null ? " (new)" : " (recreated)"));
+
+		ActivityResultLauncher<Intent> launcher = registerForActivityResult(
+			new ActivityResultContracts.StartActivityForResult(),
+			result -> {
+				Log.i(TAG, "DocumentResultProxy: Packing return intent, requestId = " + requestId);
+
+				Intent returnIntent = new Intent(this, PpssppActivity.class);
+				if (result.getResultCode() == Activity.RESULT_OK && result.getData() != null) {
+					Uri uri = result.getData().getData();
+					if (uri != null) {
+						Log.i(TAG, "DocumentResultProxy: Selected URI: " + uri);
+						try {
+							if (pickerIntent != null && Intent.ACTION_OPEN_DOCUMENT_TREE.equals(pickerIntent.getAction())) {
+								getContentResolver().takePersistableUriPermission(uri, Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
+								DocumentFile documentFile = DocumentFile.fromTreeUri(this, uri);
+								if (documentFile != null) {
+									uri = documentFile.getUri();
+									Log.i(TAG, "DocumentResultProxy: DocumentFile URI: " + uri);
+								}
+							} else {
+								getContentResolver().takePersistableUriPermission(uri, Intent.FLAG_GRANT_READ_URI_PERMISSION);
+							}
+						} catch (Exception e) {
+							Log.w(TAG, "DocumentResultProxy: Exception getting permissions or DocumentFile: " + e);
+						}
+						returnIntent.putExtra("result_path", uri.toString());
+					} else {
+						Log.w(TAG, "DocumentResultProxy: URI is null in result data");
+					}
+				}
+
+				returnIntent.putExtra("result_code", result.getResultCode());
+				returnIntent.putExtra("request_id", requestId);
+
+				// This flag is key: it finds the existing instance of your main activity
+				returnIntent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_SINGLE_TOP);
+				startActivity(returnIntent);
+				finish();
+			}
+		);
+
+		if (savedInstanceState == null) {
+			if (pickerIntent != null) {
+				launcher.launch(pickerIntent);
+			} else {
+				Log.e(TAG, "DocumentResultProxy: No picker intent provided");
+				finish();
+			}
+		}
+	}
+}

android/src/org/ppsspp/ppsspp/PpssppActivity.java

@@ -120,10 +120,6 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList
 	// switched-away from or rotated etc.
 	private boolean shuttingDown;
 
-	private static final int RESULT_LOAD_IMAGE = 101;
-	private static final int RESULT_OPEN_DOCUMENT = 102;
-	private static final int RESULT_OPEN_DOCUMENT_TREE = 103;
-
 	// Allow for multiple connected gamepads but just consider them the same for now.
 	// Actually this is not entirely true, see the code.
 	private final ArrayList<InputDeviceState> inputPlayers = new ArrayList<>();
@@ -1417,101 +1413,10 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList
 		}
 	}
 
-	static int packResultCode(int requestCode, int requestId) {
-		return (requestCode << 16) | (requestId & 0xFFFF);
-	}
-	static int getRequestCode(int packedResult) {
-		return packedResult >> 16;  // This will sign-extend, just like we want.
-	}
-	static int getRequestId(int packedResult) {
-		return packedResult & 0xFFFF;  // The requestID is unsigned, so this is fine.
-	}
-
 	@Override
 	protected void onActivityResult(int packedRequest, int resultCode, Intent data) {
 		super.onActivityResult(packedRequest, resultCode, data);
-
-		int requestCode = getRequestCode(packedRequest);
-		int requestId = getRequestId(packedRequest);
-
-		Log.i(TAG, "onActivityResult: requestCode=" + requestCode + " requestId = " + requestId + " resultCode = " + resultCode);
-
-		if (resultCode != RESULT_OK || data == null) {
-			if (data == null) {
-				Log.i(TAG, "Intent data == null");
-			}
-			NativeApp.sendRequestResult(requestId, false, "", resultCode);
-			return;
-		}
-
-		try {
-			if (requestCode == RESULT_LOAD_IMAGE) {
-				Log.i(TAG, "data: " + data);
-				Uri selectedImage = data.getData();
-				if (selectedImage != null) {
-					if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
-						Log.i(TAG, "Selected image: " + selectedImage);
-						NativeApp.sendRequestResult(requestId, true, selectedImage.toString(), 0);
-					} else {
-						String[] filePathColumn = {MediaStore.Images.Media.DATA};
-						Cursor cursor = getContentResolver().query(selectedImage, filePathColumn, null, null, null);
-						if (cursor != null) {
-							cursor.moveToFirst();
-							int columnIndex = cursor.getColumnIndex(filePathColumn[0]);
-							String picturePath = cursor.getString(columnIndex);
-							cursor.close();
-							Log.i(TAG, "Selected picture path: " + picturePath);
-							NativeApp.sendRequestResult(requestId, true, picturePath, 0);
-						}
-					}
-				} else {
-					Log.i(TAG, "No image data received");
-				}
-			} else if (requestCode == RESULT_OPEN_DOCUMENT) {
-				Uri selectedFile = data.getData();
-				if (selectedFile != null) {
-					try {
-						// Grab permanent permission so we can show it in recents list etc.
-						getContentResolver().takePersistableUriPermission(selectedFile, Intent.FLAG_GRANT_READ_URI_PERMISSION);
-					} catch (Exception e) {
-						Log.w(TAG, "Exception getting permissions for document: " + e);
-						NativeApp.sendRequestResult(requestId, false, "", 0);
-						NativeApp.reportException(e, selectedFile.toString());
-						return;
-					}
-					Log.i(TAG, "Browse file finished:" + selectedFile);
-					NativeApp.sendRequestResult(requestId, true, selectedFile.toString(), 0);
-				}
-			} else if (requestCode == RESULT_OPEN_DOCUMENT_TREE) {
-				Uri selectedDirectoryUri = data.getData();
-				if (selectedDirectoryUri != null) {
-					String path = selectedDirectoryUri.toString();
-					Log.i(TAG, "Browse folder finished: " + path);
-					try {
-						getContentResolver().takePersistableUriPermission(selectedDirectoryUri, Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
-					} catch (Exception e) {
-						Log.w(TAG, "Exception getting permissions for document: " + e);
-						NativeApp.reportException(e, selectedDirectoryUri.toString());
-						// Even if we got an exception getting permissions, continue and try to pass along the file. Maybe this version of Android
-						// doesn't need it. If we can't access it, we'll fail in some other way later.
-					}
-					DocumentFile documentFile = DocumentFile.fromTreeUri(this, selectedDirectoryUri);
-					if (documentFile != null) {
-						Log.i(TAG, "Chosen document name: " + documentFile.getUri());
-						NativeApp.sendRequestResult(requestId, true, documentFile.getUri().toString(), 0);
-					} else {
-						NativeApp.sendRequestResult(requestId, false, "", 0);
-					}
-				}
-			} else {
-				Toast.makeText(getApplicationContext(), "Bad request code: " + requestCode, Toast.LENGTH_LONG).show();
-				NativeApp.sendRequestResult(requestId, false, null, resultCode);
-				// Can't send a sensible request result back to the app without a requestCode
-			}
-		} catch (Exception e) {
-			NativeApp.reportException(e, "(function level)");
-			NativeApp.sendRequestResult(requestId, false, null, resultCode);
-		}
+		Log.i(TAG, "onActivityResult: packedRequest=" + packedRequest + " resultCode=" + resultCode);
 	}
 
 	private AlertDialog.Builder createDialogBuilderWithDeviceThemeAndUiVisibility() {
@@ -1638,13 +1543,12 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList
 		} else if (command.equals("browse_image")) {
 			try {
 				int requestId = Integer.parseInt(params);
-				int packedResultCode = packResultCode(RESULT_LOAD_IMAGE, requestId);
 				// 1. To Launch the picker:
 				Intent picker = new Intent(Intent.ACTION_PICK, MediaStore.Images.Media.EXTERNAL_CONTENT_URI);
 				Intent proxy = new Intent(this, ImageResultProxyActivity.class);
 				proxy.putExtra("picker_intent", picker);
 				proxy.putExtra("request_id", requestId);
-				Log.i(TAG, "image request ID: " + requestId + " packed: " + packedResultCode);
+				Log.i(TAG, "image request ID: " + requestId);
 				startActivity(proxy);
 				return true;
 			} catch (Exception e) { // For example, android.content.ActivityNotFoundException
@@ -1655,8 +1559,7 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList
 		} else if (command.equals("browse_file") || command.equals("browse_file_audio") || command.equals("browse_file_zip")) {
 			try {
 				int requestId = Integer.parseInt(params);
-				int packedResultCode = packResultCode(RESULT_OPEN_DOCUMENT, requestId);
-				Log.i(TAG, "browse_file request ID: " + requestId + " packed: " + packedResultCode);
+				Log.i(TAG, "browse_file request ID: " + requestId);
 				Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT);
 				intent.addCategory(Intent.CATEGORY_OPENABLE);
 				if (command.equals("browse_file_audio")) {
@@ -1670,8 +1573,12 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList
 					intent.setType("*/*");
 				}
 				intent.addFlags(Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
-				startActivityForResult(intent, packedResultCode);
-				// intent.putExtra(DocumentsContract.EXTRA_INITIAL_URI, pickerInitialUri);
+
+				Intent proxy = new Intent(this, DocumentResultProxyActivity.class);
+				proxy.putExtra("picker_intent", intent);
+				proxy.putExtra("request_id", requestId);
+				startActivity(proxy);
+				return true;
 			} catch (Exception e) {
 				NativeApp.reportException(e, params);
 				Log.e(TAG, e.toString());
@@ -1680,14 +1587,17 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList
 		} else if (command.equals("browse_folder")) {
 			try {
 				int requestId = Integer.parseInt(params);
-				int packedResultCode = packResultCode(RESULT_OPEN_DOCUMENT_TREE, requestId);
-				Log.i(TAG, "browse_folder request ID: " + requestId + " packed: " + packedResultCode);
+				Log.i(TAG, "browse_folder request ID: " + requestId);
 				Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT_TREE);
 				intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
 				intent.addFlags(Intent.FLAG_GRANT_PREFIX_URI_PERMISSION);
 				intent.addFlags(Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION);
 				intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true);  // Only allow local folders.
-				startActivityForResult(intent, packedResultCode);
+
+				Intent proxy = new Intent(this, DocumentResultProxyActivity.class);
+				proxy.putExtra("picker_intent", intent);
+				proxy.putExtra("request_id", requestId);
+				startActivity(proxy);
 				return true;
 			} catch (Exception e) {
 				NativeApp.reportException(e, params);
@@ -1970,21 +1880,16 @@ public class PpssppActivity extends AppCompatActivity implements SensorEventList
 
 		if (intent.hasExtra("request_id")) {
 			logIntentExtras(intent);
-			// This was a proxied image request.
 			int requestId = intent.getIntExtra("request_id", -1);
 			int resultCode = intent.getIntExtra("result_code", RESULT_CANCELED);
 			String path = intent.getStringExtra("result_path");
 
-			if (path != null) {
-				Log.i(TAG, "Received valid intent: " + path);
-				Log.i(TAG, "requestId: " + requestId + " resultCode: " + resultCode);
-
-				// Now you can call your native method
-				NativeApp.sendRequestResult(requestId, (resultCode == RESULT_OK), path, resultCode);
+			if (resultCode == RESULT_OK && path != null) {
+				Log.i(TAG, "Received valid proxied result: path='" + path + "' requestId=" + requestId);
+				NativeApp.sendRequestResult(requestId, true, path, 0);
 			} else {
-				Log.i(TAG, "Received failed intent");
-				Log.i(TAG, "requestId: " + requestId + " resultCode: " + resultCode);
-				NativeApp.sendRequestResult(requestId, false,"", resultCode);
+				Log.i(TAG, "Received failed or cancelled proxied result: path='" + path + "' requestId=" + requestId + " resultCode=" + resultCode);
+				NativeApp.sendRequestResult(requestId, false, path != null ? path : "", resultCode);
 			}
 		} else {
 			// Someone launched a shortcut while we were running....