first round of code fixes based on review

src/android/app/src/main/java/org/citra/citra_emu/activities/EmulationActivity.kt

@@ -64,7 +64,7 @@ class EmulationActivity : AppCompatActivity() {
     private lateinit var binding: ActivityEmulationBinding
     private lateinit var screenAdjustmentUtil: ScreenAdjustmentUtil
     private lateinit var hotkeyUtility: HotkeyUtility
-    lateinit var secondaryDisplay: SecondaryDisplay
+    lateinit var secondaryDisplayManager: SecondaryDisplay
 
     private val onShutdown = Runnable {
         if (intent.getBooleanExtra("launched_from_shortcut", false)) {
@@ -102,8 +102,8 @@ class EmulationActivity : AppCompatActivity() {
 
         super.onCreate(savedInstanceState)
 
-        secondaryDisplay = SecondaryDisplay(this)
-        secondaryDisplay.updateDisplay()
+        secondaryDisplayManager = SecondaryDisplay(this)
+        secondaryDisplayManager.updateDisplay()
 
         binding = ActivityEmulationBinding.inflate(layoutInflater)
         hotkeyUtility = HotkeyUtility(screenAdjustmentUtil, this)
@@ -188,7 +188,7 @@ class EmulationActivity : AppCompatActivity() {
     }
 
     override fun onStop() {
-        secondaryDisplay.releasePresentation()
+        secondaryDisplayManager.releasePresentation()
         super.onStop()
     }
 
@@ -199,7 +199,7 @@ class EmulationActivity : AppCompatActivity() {
 
     public override fun onRestart() {
         super.onRestart()
-        secondaryDisplay.updateDisplay()
+        secondaryDisplayManager.updateDisplay()
         NativeLibrary.reloadCameraDevices()
     }
 
@@ -222,8 +222,8 @@ class EmulationActivity : AppCompatActivity() {
         NativeLibrary.playTimeManagerStop()
         isEmulationRunning = false
         instance = null
-        secondaryDisplay.releasePresentation()
-        secondaryDisplay.releaseVD()
+        secondaryDisplayManager.releasePresentation()
+        secondaryDisplayManager.releaseVD()
 
         super.onDestroy()
     }

src/android/app/src/main/java/org/citra/citra_emu/display/SecondaryDisplay.kt

@@ -6,21 +6,18 @@ package org.citra.citra_emu.display
 
 import android.app.Presentation
 import android.content.Context
-import android.graphics.SurfaceTexture
 import android.hardware.display.DisplayManager
 import android.hardware.display.VirtualDisplay
 import android.os.Build
 import android.os.Bundle
-import android.util.Log
 import android.view.Display
 import android.view.MotionEvent
-import android.view.Surface
 import android.view.SurfaceHolder
 import android.view.SurfaceView
 import android.view.WindowManager
 import org.citra.citra_emu.features.settings.model.IntSetting
-import org.citra.citra_emu.display.SecondaryDisplayLayout
 import org.citra.citra_emu.NativeLibrary
+import org.citra.citra_emu.utils.Log
 
 class SecondaryDisplay(val context: Context) : DisplayManager.DisplayListener {
     private var pres: SecondaryDisplayPresentation? = null
@@ -29,6 +26,9 @@ class SecondaryDisplay(val context: Context) : DisplayManager.DisplayListener {
     var preferredDisplayId = -1
     var currentDisplayId = -1
 
+    val availableDisplays: List<Display>
+        get() = getSecondaryDisplays()
+
     init {
         vd = displayManager.createVirtualDisplay(
             "HiddenDisplay",
@@ -46,7 +46,7 @@ class SecondaryDisplay(val context: Context) : DisplayManager.DisplayListener {
         if (surface != null && surface.isValid) {
             NativeLibrary.secondarySurfaceChanged(surface)
         } else {
-            Log.w("SecondaryDisplay", "Attempted to update null or invalid surface")
+            Log.warning("SecondaryDisplay Attempted to update null or invalid surface")
         }
     }
 
@@ -54,7 +54,7 @@ class SecondaryDisplay(val context: Context) : DisplayManager.DisplayListener {
         NativeLibrary.secondarySurfaceDestroyed()
     }
 
-   fun getSecondaryDisplays(context: Context): List<Display> {
+   private fun getSecondaryDisplays(): List<Display> {
        val dm = context.getSystemService(Context.DISPLAY_SERVICE) as DisplayManager
        val currentDisplayId = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
            context.display.displayId
@@ -67,12 +67,13 @@ class SecondaryDisplay(val context: Context) : DisplayManager.DisplayListener {
         val presDisplays = dm.getDisplays(DisplayManager.DISPLAY_CATEGORY_PRESENTATION);
         return displays.filter {
             val isPresentable = presDisplays.any { pd -> pd.displayId == it.displayId }
-            val isNotDefaultOrPresentable = it != null && it.displayId != Display.DEFAULT_DISPLAY || isPresentable
-                    isNotDefaultOrPresentable &&
-                    it.displayId != currentDisplayId &&
-                    it.name != "HiddenDisplay" &&
-                    it.state != Display.STATE_OFF &&
-                    it.isValid
+            val isNotDefaultOrPresentable = (it != null && it.displayId != Display.DEFAULT_DISPLAY) || isPresentable
+
+            isNotDefaultOrPresentable &&
+            it.displayId != currentDisplayId &&
+            it.name != "HiddenDisplay" &&
+            it.state != Display.STATE_OFF &&
+            it.isValid
         }
     }
 
@@ -81,33 +82,35 @@ class SecondaryDisplay(val context: Context) : DisplayManager.DisplayListener {
         if (context is android.app.Activity && (context.isFinishing || context.isDestroyed)) {
             return
         }
-        val displays = getSecondaryDisplays(context)
-        val display = if (displays.isEmpty() ||
+
+        val displayToUse = if (availableDisplays.isEmpty() ||
             IntSetting.SECONDARY_DISPLAY_LAYOUT.int == SecondaryDisplayLayout.NONE.int
         ) {
             currentDisplayId = -1
             vd.display
-        } else if (preferredDisplayId >=0 && displays.any { it.displayId == preferredDisplayId }) {
+        } else if (preferredDisplayId >=0 && availableDisplays.any { it.displayId == preferredDisplayId }) {
             currentDisplayId = preferredDisplayId
-            displays.first { it.displayId == preferredDisplayId }
+            availableDisplays.first { it.displayId == preferredDisplayId }
         } else {
             val dm = context.getSystemService(Context.DISPLAY_SERVICE) as DisplayManager
             val default = dm.displays.first {it.displayId == Display.DEFAULT_DISPLAY}
             // prioritize displays that have a different name from the default display, as
             // some devices such as the Odin 2 create a permanent virtual display with the same
             // name as the default display that should be skipped in most cases
-            currentDisplayId = displays.firstOrNull{it.name != default.name && !it.name.contains("Built",true)}?.displayId ?: displays[0].displayId
-            displays.first{ it.displayId == currentDisplayId }
+            currentDisplayId = availableDisplays.firstOrNull{
+                    it.name != default.name && !it.name.contains("Built",true)}?.displayId ?:
+                    availableDisplays[0].displayId
+            availableDisplays.first{ it.displayId == currentDisplayId }
         }
 
         // if our presentation is already on the right display, ignore
-        if (pres?.display == display) return
+        if (pres?.display == displayToUse) return
 
         // otherwise, make a new presentation
         releasePresentation()
 
         try {
-            pres = SecondaryDisplayPresentation(context, display!!, this)
+            pres = SecondaryDisplayPresentation(context, displayToUse!!, this)
             pres?.show()
         }
         // catch BadTokenException and InvalidDisplayException,
@@ -162,18 +165,18 @@ class SecondaryDisplayPresentation(
         surfaceView = SurfaceView(context)
         surfaceView.holder.addCallback(object : SurfaceHolder.Callback {
             override fun surfaceCreated(holder: SurfaceHolder) {
-                Log.d("SecondaryDisplay", "Surface created")
+                Log.debug("SecondaryDisplay Surface created")
             }
 
             override fun surfaceChanged(
                 holder: SurfaceHolder, format: Int, width: Int, height: Int
             ) {
-                Log.d("SecondaryDisplay", "Surface changed: ${width}x${height}")
+                Log.debug("SecondaryDisplay Surface changed: ${width}x${height}")
                 parent.updateSurface()
             }
 
             override fun surfaceDestroyed(holder: SurfaceHolder) {
-                Log.d("SecondaryDisplay", "Surface destroyed")
+                Log.debug("SecondaryDisplay Surface destroyed")
                 parent.destroySurface()
             }
         })

src/android/app/src/main/java/org/citra/citra_emu/fragments/EmulationFragment.kt

@@ -78,7 +78,6 @@ import org.citra.citra_emu.utils.BuildUtil
 import org.citra.citra_emu.utils.DirectoryInitialization
 import org.citra.citra_emu.utils.DirectoryInitialization.DirectoryInitializationState
 import org.citra.citra_emu.utils.EmulationMenuSettings
-import org.citra.citra_emu.utils.FileUtil
 import org.citra.citra_emu.utils.GameHelper
 import org.citra.citra_emu.utils.GameIconUtils
 import org.citra.citra_emu.utils.EmulationLifecycleUtil
@@ -93,7 +92,8 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
     private lateinit var emulationState: EmulationState
     private var perfStatsUpdater: Runnable? = null
 
-    private lateinit var emulationActivity: EmulationActivity
+    private val emulationActivity: EmulationActivity
+        get() = (requireActivity() as EmulationActivity)
 
     private var _binding: FragmentEmulationBinding? = null
     private val binding get() = _binding!!
@@ -116,8 +116,7 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
     override fun onAttach(context: Context) {
         super.onAttach(context)
         if (context is EmulationActivity) {
-            emulationActivity = context
-            NativeLibrary.setEmulationActivity(context)
+             NativeLibrary.setEmulationActivity(context)
         } else {
             throw IllegalStateException("EmulationFragment must have EmulationActivity parent")
         }
@@ -183,7 +182,6 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
         // So this fragment doesn't restart on configuration changes; i.e. rotation.
         retainInstance = true
         emulationState = EmulationState(game.path)
-        emulationActivity = requireActivity() as EmulationActivity
         screenAdjustmentUtil =
             ScreenAdjustmentUtil(requireContext(), requireActivity().windowManager, settings)
         EmulationLifecycleUtil.addPauseResumeHook(onPause)
@@ -197,7 +195,7 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
     ): View {
         _binding = FragmentEmulationBinding.inflate(inflater)
         binding.inGameMenu.menu.findItem(R.id.menu_secondary_screen_layout).isVisible =
-            emulationActivity.secondaryDisplay.getSecondaryDisplays(emulationActivity).isNotEmpty()
+            emulationActivity.secondaryDisplayManager.availableDisplays.isNotEmpty()
         binding.inGameMenu.menu.findItem(R.id.menu_landscape_screen_layout).isVisible =
             CitraApplication.appContext.resources.configuration.orientation !=
                     Configuration.ORIENTATION_PORTRAIT
@@ -1070,7 +1068,7 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
         val enableSecondaryCheckbox = popupMenu.menu.findItem(R.id.menu_secondary_layout_none)
         chooserMenu?.subMenu?.removeGroup(R.id.menu_secondary_management_display_group)
         val displays =
-            emulationActivity.secondaryDisplay.getSecondaryDisplays(emulationActivity)
+            emulationActivity.secondaryDisplayManager.availableDisplays
 
         if (selectedLayout == SecondaryDisplayLayout.NONE.int) {
             enableSecondaryCheckbox.isChecked = false
@@ -1082,9 +1080,8 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
             chooserMenu.isVisible = (displays.size > 1)
         }
         val layoutOptionMenuItem = when (selectedLayout) {
-            SecondaryDisplayLayout.NONE.int -> {
+            SecondaryDisplayLayout.NONE.int ->
                 R.id.menu_secondary_layout_opposite
-            }
 
             SecondaryDisplayLayout.REVERSE_PRIMARY.int ->
                 R.id.menu_secondary_layout_opposite
@@ -1108,9 +1105,11 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
                 R.id.menu_secondary_layout_side_by_side
         }
         popupMenu.menu.findItem(layoutOptionMenuItem).isChecked = true
-
+        // Add the available secondary displays to the display chooser list
+        // Use the display ID as the menu ID - since generated menu IDs are all > 1,000,000 this
+        // *should* result in unique ids
         if (displays.size > 1 && selectedLayout != SecondaryDisplayLayout.NONE.int) {
-            val current = emulationActivity.secondaryDisplay.currentDisplayId
+            val current = emulationActivity.secondaryDisplayManager.currentDisplayId
             chooserMenu.isVisible = true
             displays.forEachIndexed { index, display ->
                 chooserMenu?.subMenu?.add(
@@ -1137,7 +1136,7 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
                     } else {
                         screenAdjustmentUtil.changeSecondaryOrientation(SecondaryDisplayLayout.NONE.int)
                     }
-                    emulationActivity.secondaryDisplay.updateDisplay()
+                    emulationActivity.secondaryDisplayManager.updateDisplay()
                     showSecondaryScreenLayoutMenu() // reopen menu to get new behaviors
                     true
                 }
@@ -1183,8 +1182,11 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback, Choreographer.Fram
 
                 else -> {
                     // display ID selection
-                    emulationActivity.secondaryDisplay.preferredDisplayId = it.itemId
-                    emulationActivity.secondaryDisplay.updateDisplay()
+                    // If we are clicking on a menu item that isn't one of the options above, it must
+                    // be one of the dynamically generated menu items added to the secondary display
+                    // choice list.
+                    emulationActivity.secondaryDisplayManager.preferredDisplayId = it.itemId
+                    emulationActivity.secondaryDisplayManager.updateDisplay()
                     true
                 }
             }

src/android/app/src/main/jni/emu_window/emu_window.cpp

@@ -18,20 +18,16 @@
 #include "video_core/renderer_base.h"
 
 bool EmuWindow_Android::OnSurfaceChanged(ANativeWindow* surface) {
-    int w = surface == NULL ? 0 : ANativeWindow_getWidth(surface);
-    int h = surface == NULL ? 0 : ANativeWindow_getHeight(surface);
-    if (render_window == surface && w == window_width && h == window_height) {
+    int temp_width = surface == nullptr ? 0 : ANativeWindow_getWidth(surface);
+    int temp_height = surface == nullptr ? 0 : ANativeWindow_getHeight(surface);
+    if (render_window == surface && temp_width == window_width && temp_height == window_height) {
         return false;
     }
-    window_width = w;
-    window_height = h;
+    window_width = temp_width;
+    window_height = temp_height;
     render_window = surface;
     window_info.type = Frontend::WindowSystemType::Android;
     window_info.render_surface = surface;
-    if (surface != nullptr) {
-        window_width = ANativeWindow_getWidth(surface);
-        window_height = ANativeWindow_getHeight(surface);
-    }
     StopPresenting();
     OnFramebufferSizeChanged();
     return true;