Revert "Toggle DisableFrameThrottling setting by clicking on FPS"

This commit reverts PR #2037. Passing `NativeSettings` to emulation code through a member reference, instead of a local variable, caused unpredictable crashes when using custom GPU drivers (v615+) on some Qualcomm SoCs.
The exact cause of the issue remains unknown, my best guess is that it was caused by an incorrect optimization performed on the Kotlin bytecode in release mode, which caused an issue when reading memory that had been forked, because of running emulation in a separate process.
Runtime settings modification will be reimplemented in the future via an alternative method.

app/src/main/cpp/emu_jni.cpp

@@ -27,7 +27,7 @@ std::weak_ptr<skyline::kernel::OS> OsWeak;
 std::weak_ptr<skyline::gpu::GPU> GpuWeak;
 std::weak_ptr<skyline::audio::Audio> AudioWeak;
 std::weak_ptr<skyline::input::Input> InputWeak;
-std::weak_ptr<skyline::AndroidSettings> SettingsWeak;
+std::weak_ptr<skyline::Settings> SettingsWeak;
 
 // https://cs.android.com/android/platform/superproject/+/master:bionic/libc/tzcode/bionic.cpp;l=43;drc=master;bpv=1;bpt=1
 static std::string GetTimeZoneName() {
@@ -85,8 +85,7 @@ extern "C" JNIEXPORT void Java_emu_skyline_EmulationActivity_executeApplication(
 
     auto jvmManager{std::make_shared<skyline::JvmManager>(env, instance)};
 
-    auto androidSettings{std::make_shared<skyline::AndroidSettings>(env, settingsInstance)};
+    std::shared_ptr<skyline::Settings> settings{std::make_shared<skyline::AndroidSettings>(env, settingsInstance)};
-    std::shared_ptr<skyline::Settings> settings{androidSettings};
 
     skyline::JniString publicAppFilesPath(env, publicAppFilesPathJstring);
     skyline::Logger::EmulationContext.Initialize(publicAppFilesPath + "logs/emulation.sklog");
@@ -116,7 +115,7 @@ extern "C" JNIEXPORT void Java_emu_skyline_EmulationActivity_executeApplication(
         GpuWeak = os->state.gpu;
         AudioWeak = os->state.audio;
         InputWeak = os->state.input;
-        SettingsWeak = androidSettings;
+        SettingsWeak = settings;
         jvmManager->InitializeControllers();
 
         skyline::Logger::DebugNoPrefix("Launching ROM {}", skyline::JniString(env, romUriJstring));
@@ -248,7 +247,6 @@ extern "C" JNIEXPORT void JNICALL Java_emu_skyline_utils_NativeSettings_updateNa
     auto settings{SettingsWeak.lock()};
     if (!settings)
         return; // We don't mind if we miss settings updates while settings haven't been initialized
-    settings->BeginTransaction(env);
     settings->Update();
 }
 

app/src/main/cpp/skyline/common/android_settings.h

@@ -20,12 +20,14 @@ namespace skyline {
          * @note Will construct the underlying KtSettings object in-place
          */
         AndroidSettings(JNIEnv *env, jobject settingsInstance) : ktSettings(env, settingsInstance) {
-            ktSettings.BeginTransaction(env);
             Update();
         }
 
-        void BeginTransaction(JNIEnv *env) {
+        /**
-            ktSettings.BeginTransaction(env);
+         * @note Will take ownership of the passed KtSettings object
+         */
+        AndroidSettings(KtSettings &&ktSettings) : ktSettings(std::move(ktSettings)) {
+            Update();
         }
 
         void Update() override {
@@ -48,6 +50,6 @@ namespace skyline {
             disableSubgroupShuffle = ktSettings.GetBool("disableSubgroupShuffle");
             isAudioOutputDisabled = ktSettings.GetBool("isAudioOutputDisabled");
             validationLayer = ktSettings.GetBool("validationLayer");
-        }
+        };
     };
 }

app/src/main/cpp/skyline/gpu/presentation_engine.cpp

@@ -29,13 +29,10 @@ namespace skyline::gpu {
           presentationTrack{static_cast<u64>(trace::TrackIds::Presentation), perfetto::ProcessTrack::Current()},
           vsyncEvent{std::make_shared<kernel::type::KEvent>(state, true)},
           choreographerThread{&PresentationEngine::ChoreographerThread, this},
-          presentationThread{&PresentationEngine::PresentationThread, this},
+          presentationThread{&PresentationEngine::PresentationThread, this} {
-          forceTripleBuffering{*state.settings->forceTripleBuffering},
-          disableFrameThrottling{*state.settings->disableFrameThrottling} {
         auto desc{presentationTrack.Serialize()};
         desc.set_name("Presentation");
         perfetto::TrackEvent::SetTrackDescriptor(presentationTrack, desc);
-        state.settings->disableFrameThrottling.AddCallback([this](auto && value) { OnDisableFrameThrottlingChanged(value); });
     }
 
     PresentationEngine::~PresentationEngine() {
@@ -205,7 +202,7 @@ namespace skyline::gpu {
             }); // We don't care about suboptimal images as they are caused by not respecting the transform hint, we handle transformations externally
         }
 
-        timestamp = (timestamp && !disableFrameThrottling) ? timestamp : getMonotonicNsNow(); // We tie FPS to the submission time rather than presentation timestamp, if we don't have the presentation timestamp available or if frame throttling is disabled as we want the maximum measured FPS to not be restricted to the refresh rate
+        timestamp = (timestamp && !*state.settings->disableFrameThrottling) ? timestamp : getMonotonicNsNow(); // We tie FPS to the submission time rather than presentation timestamp, if we don't have the presentation timestamp available or if frame throttling is disabled as we want the maximum measured FPS to not be restricted to the refresh rate
         if (frameTimestamp) {
             i64 sampleWeight{Fps ? Fps : 1}; //!< The weight of each sample in calculating the average, we want to roughly average the past second
 
@@ -283,7 +280,7 @@ namespace skyline::gpu {
     }
 
     void PresentationEngine::UpdateSwapchain(texture::Format format, texture::Dimensions extent) {
-        auto minImageCount{std::max(vkSurfaceCapabilities.minImageCount, forceTripleBuffering ? 3U : 2U)};
+        auto minImageCount{std::max(vkSurfaceCapabilities.minImageCount, *state.settings->forceTripleBuffering ? 3U : 2U)};
         if (minImageCount > MaxSwapchainImageCount)
             throw exception("Requesting swapchain with higher image count ({}) than maximum slot count ({})", minImageCount, MaxSwapchainImageCount);
 
@@ -307,7 +304,7 @@ namespace skyline::gpu {
         if ((capabilities.supportedUsageFlags & presentUsage) != presentUsage)
             throw exception("Swapchain doesn't support image usage '{}': {}", vk::to_string(presentUsage), vk::to_string(capabilities.supportedUsageFlags));
 
-        auto requestedMode{disableFrameThrottling ? vk::PresentModeKHR::eMailbox : vk::PresentModeKHR::eFifo};
+        auto requestedMode{*state.settings->disableFrameThrottling ? vk::PresentModeKHR::eMailbox : vk::PresentModeKHR::eFifo};
         auto modes{gpu.vkPhysicalDevice.getSurfacePresentModesKHR(**vkSurface)};
         if (std::find(modes.begin(), modes.end(), requestedMode) == modes.end())
             throw exception("Swapchain doesn't support present mode: {}", vk::to_string(requestedMode));
@@ -344,15 +341,6 @@ namespace skyline::gpu {
         swapchainImageCount = vkImages.size();
     }
 
-    void PresentationEngine::OnDisableFrameThrottlingChanged(const bool value) {
-        std::scoped_lock guard{mutex};
-
-        disableFrameThrottling = value;
-
-        if (vkSurface && swapchainExtent && swapchainFormat)
-            UpdateSwapchain(swapchainFormat, swapchainExtent);
-    }
-
     void PresentationEngine::UpdateSurface(jobject newSurface) {
         std::scoped_lock guard{mutex};
 

app/src/main/cpp/skyline/gpu/presentation_engine.h

@@ -46,9 +46,6 @@ namespace skyline::gpu {
         size_t frameIndex{}; //!< The index of the next semaphore/fence to be used for acquiring swapchain images
         size_t swapchainImageCount{}; //!< The number of images in the current swapchain
 
-        bool forceTripleBuffering{}; //!< If the presentation engine should always triple buffer even if the swapchain supports double buffering
-        bool disableFrameThrottling{}; //!< Allow the guest to submit frames without any blocking calls
-
         i64 frameTimestamp{}; //!< The timestamp of the last frame being shown in nanoseconds
         i64 averageFrametimeNs{}; //!< The average time between frames in nanoseconds
         i64 averageFrametimeDeviationNs{}; //!< The average deviation of frametimes in nanoseconds
@@ -108,11 +105,6 @@ namespace skyline::gpu {
          */
         void UpdateSwapchain(texture::Format format, texture::Dimensions extent);
 
-        /**
-         * @brief Handles DisableFrameThrottling setting changed event
-        */
-        void OnDisableFrameThrottlingChanged(const bool value);
-
       public:
         PresentationEngine(const DeviceState &state, GPU &gpu);
 

app/src/main/cpp/skyline/jvm.h

@@ -25,12 +25,12 @@ namespace skyline {
      */
     class KtSettings {
       private:
-        JNIEnv *env{}; //!< The JNI environment
+        JNIEnv *env; //!< A pointer to the current jni environment
         jclass settingsClass; //!< The settings class
         jobject settingsInstance; //!< The settings instance
 
       public:
-        KtSettings(JNIEnv *env, jobject settingsInstance) : settingsInstance(env->NewGlobalRef(settingsInstance)), settingsClass(reinterpret_cast<jclass>(env->NewGlobalRef(env->GetObjectClass(settingsInstance)))) {}
+        KtSettings(JNIEnv *env, jobject settingsInstance) : env(env), settingsInstance(settingsInstance), settingsClass(env->GetObjectClass(settingsInstance)) {}
 
         KtSettings(const KtSettings &) = delete;
 
@@ -38,10 +38,6 @@ namespace skyline {
 
         KtSettings(KtSettings &&) = default;
 
-        void BeginTransaction(JNIEnv *pEnv) {
-            this->env = pEnv;
-        }
-
         /**
          * @param key A null terminated string containing the key of the setting to get
          */

app/src/main/java/emu/skyline/EmulationActivity.kt

@@ -77,8 +77,6 @@ class EmulationActivity : AppCompatActivity(), SurfaceHolder.Callback, View.OnTo
     @Inject
     lateinit var preferenceSettings : PreferenceSettings
 
-    lateinit var nativeSettings : NativeSettings
-
     @Inject
     lateinit var inputManager : InputManager
 
@@ -198,7 +196,7 @@ class EmulationActivity : AppCompatActivity(), SurfaceHolder.Callback, View.OnTo
 
         GpuDriverHelper.ensureFileRedirectDir(this)
         emulationThread = Thread {
-            executeApplication(rom.toString(), romType, romFd.detachFd(), nativeSettings, applicationContext.getPublicFilesDir().canonicalPath + "/", applicationContext.filesDir.canonicalPath + "/", applicationInfo.nativeLibraryDir + "/", assets)
+            executeApplication(rom.toString(), romType, romFd.detachFd(), NativeSettings(this, preferenceSettings), applicationContext.getPublicFilesDir().canonicalPath + "/", applicationContext.filesDir.canonicalPath + "/", applicationInfo.nativeLibraryDir + "/", assets)
             returnFromEmulation()
         }
 
@@ -210,10 +208,7 @@ class EmulationActivity : AppCompatActivity(), SurfaceHolder.Callback, View.OnTo
         super.onCreate(savedInstanceState)
         requestedOrientation = preferenceSettings.orientation
         window.attributes.layoutInDisplayCutoutMode = WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_SHORT_EDGES
-
         inputHandler = InputHandler(inputManager, preferenceSettings)
-        nativeSettings = NativeSettings(this, preferenceSettings)
-
         setContentView(binding.root)
 
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
@@ -254,15 +249,6 @@ class EmulationActivity : AppCompatActivity(), SurfaceHolder.Callback, View.OnTo
                         postDelayed(this, 250)
                     }
                 }, 250)
-                setOnClickListener {
-                    val newValue = !preferenceSettings.disableFrameThrottling
-                    preferenceSettings.disableFrameThrottling = newValue
-                    nativeSettings.disableFrameThrottling = newValue
-
-                    var color = if (newValue) getColor(R.color.colorPerfStatsSecondary) else getColor(R.color.colorPerfStatsPrimary)
-                    binding.perfStats.setTextColor(color)
-                    nativeSettings.updateNative()
-                }
             }
         }