[Feature] Option to override the display refresh rate

[Nabile-Rahmani]

What should happen

(Related issue, not sure if quite the same as it's not about eliminating stutters, please close and I apologise if wrong)

We could add an option to override the console's refresh rate to match the host's display.

I've patched PPSSPP with a 3-line change since forever (and my NixOS config automatically builds it for me), but it'd be great to upgrade this to an option for a few reasons:

• Not all games handle arbitrary refresh rate well, so using runtime game configs on a per-game basis is better than juggling with two builds
• Allow everyone with a good monitor to enjoy glorious high-framerate gaming
• Use distros' binary caching to not waste time and energy rebuilding it every time

I can work on this, but I'd rather ask for permission first.

The option could be one of:

• bool UseHostRefreshRate = false
• enum RefreshRateMode { PSP, Host } = RefreshRateMode.PSP, for future proofing (e.g. eventually a Custom refresh rate value ?)

When enabled, PPSSPP will use the window's display refresh rate for its presentation.

Because the behaviour is in the core (HLE/sceDisplay & HW/Display), it likely needs to be able to request the front-end (e.g. SDL) about the host's refresh rate.

This function can call something like SDL_GetCurrentDisplayMode() and return mode.refresh_rate.

(The patch in question, where 165 would be fetched at runtime instead, or 60:)

diff --git a/Core/HLE/sceDisplay.cpp b/Core/HLE/sceDisplay.cpp
index 3ad5060d..76310145 100644
--- a/Core/HLE/sceDisplay.cpp
+++ b/Core/HLE/sceDisplay.cpp
@@ -103,7 +103,7 @@ static int height;
 static bool wasPaused;
 static bool flippedThisFrame;
 
-static int framerate = 60;
+static int framerate = 165;
 
 // 1.001f to compensate for the classic 59.94 NTSC framerate that the PSP seems to have.
 static double timePerVblank = 1.001f / (float)framerate;
diff --git a/Core/HW/Display.cpp b/Core/HW/Display.cpp
index acead8d7..3308ffb5 100644
--- a/Core/HW/Display.cpp
+++ b/Core/HW/Display.cpp
@@ -78,7 +78,7 @@ static void CalculateFPS() {
        actualFps = (float)(actualFlips - lastActualFlips);
 
        fps = frames / (now - lastFpsTime);
-       flips = (float)(60.0 * (double)(gpuStats.numFlips - lastNumFlips) / frames);
+       flips = (float)(165.0 * (double)(gpuStats.numFlips - lastNumFlips) / frames);
 
        lastFpsFrame = numVBlanks;
        lastNumFlips = gpuStats.numFlips;
@@ -148,7 +148,7 @@ uint64_t DisplayFrameStartTicks() {
 
 uint32_t __DisplayGetCurrentHcount() {
    const int ticksIntoFrame = (int)(CoreTiming::GetTicks() - frameStartTicks);
-   const int ticksPerVblank = CoreTiming::GetClockFrequencyHz() / 60 / hCountPerVblank;
+   const int ticksPerVblank = CoreTiming::GetClockFrequencyHz() / 165 / hCountPerVblank;
    // Can't seem to produce a 0 on real hardware, offsetting by 1 makes things look right.
    return 1 + (ticksIntoFrame / ticksPerVblank);
 }

Who would this benefit

Everyone with a high refresh rate monitor.

Platform (if relevant)

None

Games this would be useful in

All variably timestepped games

Other emulators or software with a similar feature

No response

Checklist

• Check the latest git build in case it's already implemented.
• Search for other requests of the same feature.

[anr2me]

Since you already using the patch and testing it, you can just create the PR so it can be reviewed and the artifacts can be tested by other people to see whether there will be problem or not.

[hrydgard]

Please do so, yeah.

Also, while cool with the games that work with it, this is one of those options that people will enable out of curiousity, forgot they have it on and games will break left and right. People even do that with things like tilt controls, then send me email about why the virtual joystick is moving around so much! So, we need a UI solution for that to consider merging it.

[LunaMoo]

One idea to make user experience better would be to limit all problematic settings to be visible only as per game settings, so user will have to intentionally activate them for each game separately instead of just enable and forget.

[Nabile-Rahmani]

For UX, there's a few things we could do:

• Reuse the warning popup as seen in the texture upscale option when enabling the override.

• Add a section to the FAQ for any commonly asked issues (including the gyro one if not already) so that people first check this list before raising issues. I really appreciate projects thoroughly describing every edge case you can run into.

• If it's such a bother, maybe we could regroup all advanced options to a dedicated tab to discourage inexperienced users from touching them. Then again, the gyro thing is in the input config, there's only so much we can do apart from writing good FAQs.

As for the patch: nope, there's no option code yet as said, I have to first familiarise myself with its system.

Hopefully there's a config change event I can hook into to only update the framerate as needed as opposed to every frame for performance reasons. Wouldn't mind any pointers to save time, of course.

[iota97]

Tbh this looks scary enough I might want to bury it into developers settings or as ini only options.

About options I'm not aware of hooks or anything, but I haven't been on the project for quite a while now.

The easiest way to implement this would be add a new integer in the configs (just look at any small PR that add a new settings and you will understand how to do it). Then you have to access the global config struct with the value in the code.

By leaving this as an ini only settings you would have to manually edit it, a bit of a pain maybe, but reduces the odds someone will do it randomly and then forgets about it. As it will be for more power user you can just leave the integer as auto detecting the frequency might fails sometimes or maybe some games work better with other values (es a multiple of 60).

This would also remove the requirements of doing an UI and translating the strings (that would be quite required with the warning and so on).

PS: don't expect people to read the FAQs anyway.

[Nabile-Rahmani]

Right, I don't want to burden you with more work !

Excellent points, that'll make it easier to implement and takes care of all the things. I'm not switching monitors any time soon anyways.

I'm wondering if leaving it in the dev tab is a good compromise so that it's somewhat discoverable, then only remove the UI element if too many people accidentally report this to you for your taste. Your call !

And thanks for the help :)

[anr2me]

If you do want to expose the UI, you can use this PR as a reference #18867
But that PR add an integer input (sub-option of the bEnableUPnP option) instead of a boolean (i don't remembered any PR with boolean option).

If it's a boolean option, you can check nearby g_Config.bEnable...something, the UI (at UI/GameSettingsScreen.cpp) should be much simpler.
If your changes doesn't works on libretro, you can ignore the UI for libretro.

Basically, the option will goes to g_Config variable and you check this variable before applying your patch's changes.

[Nabile-Rahmani]

I added the DisplayRefreshRate = 60 per-game option.

There already was a __DisplaySetFramerate() used to update framerate-related variables, only called from Common/VR/PPSSPPVR.cpp.

I used this in __DisplayFlip(), and moved the Force72Hz option check inside __DisplaySetFramerate() so the VR mode still works as expected.

This adds one branch to __DisplayFlip() because I don't have a callback on config change to directly call __DisplaySetFramerate(). Hopefully this is insignificant. Is there an unlikely branch macro I can add to the if statement ? And do you want me to do performance profiling or is it overkill ?

One problem is that Force72Hz defaults to true, and 72 Hz seems like an arbitrarily low value (don't most headsets target 90+ Hz ?).

The VR mode could take advantage of the new DisplayRefreshRate and sunset Force72Hz as it's obsoleted by this far more versatile option. Sounds good ?

[anr2me]

So you're modifying the VR source code too?
For VR-stuff, may be @lvonasek know why it's 72Hz

[Nabile-Rahmani]

Only this, because __DisplayFlip() takes care of the frame rate now:

diff --git a/Common/VR/PPSSPPVR.cpp b/Common/VR/PPSSPPVR.cpp
index 84c76e97..2b1e81ab 100644
--- a/Common/VR/PPSSPPVR.cpp
+++ b/Common/VR/PPSSPPVR.cpp
@@ -664,7 +664,6 @@ bool StartVRRender() {
                vrCompat[VR_COMPAT_SKYPLANE] = PSP_CoreParameter().compat.vrCompat().Skyplane;
 
                // Set customizations
-               __DisplaySetFramerate(g_Config.bForce72Hz ? 72 : 60);
                VR_SetConfigFloat(VR_CONFIG_CANVAS_DISTANCE, vrScene && (appMode == VR_GAME_MODE) ? g_Config.fCanvas3DDistance : g_Config.fCanvasDistance);
                VR_SetConfig(VR_CONFIG_PASSTHROUGH, g_Config.bPassthrough);
                return true;
diff --git a/Core/HLE/sceDisplay.cpp b/Core/HLE/sceDisplay.cpp
index 31879032..b6844bf3 100644
--- a/Core/HLE/sceDisplay.cpp
+++ b/Core/HLE/sceDisplay.cpp
@@ -106,7 +106,7 @@ static int height;
 static bool wasPaused;
 static bool flippedThisFrame;
 
-static int framerate = 60;
+static int framerate = g_Config.iDisplayRefreshRate;
 
 // 1.001f to compensate for the classic 59.94 NTSC framerate that the PSP seems to have.
 static double timePerVblank = 1.001f / (float)framerate;
@@ -152,6 +152,7 @@ void __DisplayVblankBeginCallback(SceUID threadID, SceUID prevCallbackId);
 void __DisplayVblankEndCallback(SceUID threadID, SceUID prevCallbackId);
 
 void __DisplayFlip(int cyclesLate);
+static void __DisplaySetFramerate(int value);
 
 static bool UseLagSync() {
        return g_Config.bForceLagSync && !g_Config.bAutoFrameSkip;
@@ -562,6 +563,9 @@ static void NotifyUserIfSlow() {
 }
 
 void __DisplayFlip(int cyclesLate) {
+       if (g_Config.iDisplayRefreshRate != framerate)
+               __DisplaySetFramerate(g_Config.iDisplayRefreshRate);
+
        flippedThisFrame = true;
        // We flip only if the framebuffer was dirty. This eliminates flicker when using
        // non-buffered rendering. The interaction with frame skipping seems to need
@@ -1126,8 +1130,8 @@ void Register_sceDisplay_driver() {
        RegisterModule("sceDisplay_driver", ARRAY_SIZE(sceDisplay), sceDisplay);
 }
 
-void __DisplaySetFramerate(int value) {
-       framerate = value;
+static void __DisplaySetFramerate(int value) {
+       framerate = g_Config.bForce72Hz ? 72 : value;
        timePerVblank = 1.001f / (float)framerate;
        frameMs = 1001.0 / (double)framerate;
 }

The idea is that now you can set any refresh rate, not just 72. So it shouldn't need to be hardcoded and checked specifically anymore.

We remove Force72Hz and VR users use DisplayRefreshRate to match their headset's refresh rate, which should be much higher than 72 Hz in most cases.

The option lives in dev tools:

[lvonasek]

60Hz display refresh rate is only supported on Quest 2 and for some reason it feels like CRT screen. All other supported VR headsets have 72Hz as a minimum.

In VR there are so called reproduction errors when game renders on different refresh rate than the display. The reprojections errors happen when you look around, it is a flickering which makes users dizzy. 72Hz hack reduces this flickering (but also speeds up the game).

I could implement 90Hz support during the week but it would probably not be a preferred option as GPU is a bottleneck on VR version (you need a high resolution for VR but the HW mostly cannot handle it).

[Nabile-Rahmani]

I'm not seeing VR settings anywhere in the UI, so I had to manually edit the file so that it doesn't run at 72 Hz by default.

Can we make the VR UI's force 72 Hz option internally set DisplayRefreshRate between 72 and 60 instead and remove the boolean field ?

... But since it's a hack in the first place, why not let users set the value to whatever looks right to them based on their hardware ?

[lvonasek]

I'm not seeing VR settings anywhere in the UI, so I had to manually edit the file so that it doesn't run at 72 Hz by default

The VR options are visible only on VR device:

ppsspp/UI/GameSettingsScreen.cpp

Line 259 in 630cae9

CreateVRSettings(vrSettings);

Can we make the VR UI's force 72 Hz option internally set DisplayRefreshRate between 72 and 60 instead and remove the boolean field ?

Yes, feel free to do that.

But since it's a hack in the first place, why not let users set the value to whatever looks right to them based on their hardware ?

Most of the users go with the default settings and as the experience without the hack was in 2022 terrible we set the hack by default enabled (not sure if it is now possible to change refresh rate without speeding up the game).

[Nabile-Rahmani]

If I remove the boolean, the refresh rate will default to 60, so VR users who update PPSSPP will have to go to the setting and tick the force 72 Hz option again.

If I leave it, after the update, PPSSPP will run at 72 Hz for everyone because it's checking for that option which was set to true by default, and there's no UI to disable it if you don't have an headset.

So yeah, the boolean has got to go away, but if VR users have to tick the force 72 Hz checkbox again, I feel like they may as well pick the value. Ugh.

We could leave the boolean alone, and check it when detecting the VR headset, then set DisplayRefreshRate. Same goes for the VR option so that on restarts it picks between 72 and 60.

__DisplaySetFramerate() doesn't have to check the boolean anymore, everybody is happy. I think !

Any pointers on that VR detection function appreciated.

... Well, exactly where I removed the line, heh. I just have to set the option there. StartVRRender() is only called when a VR headset is detected, correct ?

And does it do it every time you launch a game ? Because if that's the case I don't have to do anything to the Force72Hz option.

diff --git a/Common/VR/PPSSPPVR.cpp b/Common/VR/PPSSPPVR.cpp
index 84c76e97..007e8370 100644
--- a/Common/VR/PPSSPPVR.cpp
+++ b/Common/VR/PPSSPPVR.cpp
@@ -664,7 +664,7 @@ bool StartVRRender() {
                vrCompat[VR_COMPAT_SKYPLANE] = PSP_CoreParameter().compat.vrCompat().Skyplane;
 
                // Set customizations
-               __DisplaySetFramerate(g_Config.bForce72Hz ? 72 : 60);
+               g_Config.iDisplayRefreshRate = g_Config.bForce72Hz ? 72 : 60;
                VR_SetConfigFloat(VR_CONFIG_CANVAS_DISTANCE, vrScene && (appMode == VR_GAME_MODE) ? g_Config.fCanvas3DDistance :
 g_Config.fCanvasDistance);
                VR_SetConfig(VR_CONFIG_PASSTHROUGH, g_Config.bPassthrough);
                return true;
diff --git a/Core/HLE/sceDisplay.cpp b/Core/HLE/sceDisplay.cpp
index 31879032..fbaf9d15 100644
--- a/Core/HLE/sceDisplay.cpp
+++ b/Core/HLE/sceDisplay.cpp
-void __DisplaySetFramerate(int value) {
+static void __DisplaySetFramerate(int value) {
        framerate = value;
        timePerVblank = 1.001f / (float)framerate;
        frameMs = 1001.0 / (double)framerate;

Problem solved ? It's working in pancake mode, but I can't test VR mode myself.

[lvonasek]

Well, exactly where I removed the line, heh. I just have to set the option there. StartVRRender() is only called when a VR headset is detected, correct ?

Correct

Problem solved ? It's working in pancake mode, but I can't test VR mode myself.

It sounds good, I will test your branch during the week to confirm.

[anr2me]

Regarding the hook you're looking, is it something like UI::EventReturn HostnameSelectScreen::OnIPClick(UI::EventParams& e) function at GameSettingsScreen.cpp which supposed to copy the IP to Clipboard (but System_CopyStringToClipboard failed to copy it as i remembered LOL) when a user select an IP from a dropdown/combobox menu at "Change proAdhocServer Address" setting?

Or in your case may be something like UI::EventReturn GameSettingsScreen::OnChangeNickname(UI::EventParams &e) (i don't remembered any integer input that have an event handler)

[Nabile-Rahmani]

Yup, and one when starting the game loop.

It would eliminate the per-frame branch, even though it may not have any performance impact in the grand scheme of things, but I'd rather do things right.

I don't have potato hardware and haven't measured, so it could be insignificant after all and not worth doing.

And I presume I thought about it initially because I was going to call out to the front-end to query the display refresh rate, so there would have been potentially more workload than there is right now. :p

[hrydgard]

One branch per frame is not something to worry about, even on potatoes :)