Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please do remove configuration.setForegroundFPS(Lwjgl3ApplicationConfiguration.getDisplayMode().refreshRate); #192

Open
TCreutzenberg opened this issue Aug 11, 2024 · 13 comments

Comments

@TCreutzenberg
Copy link
Contributor

TCreutzenberg commented Aug 11, 2024

Hey guys,

I have a request: Please do remove configuration.setForegroundFPS(Lwjgl3ApplicationConfiguration.getDisplayMode().refreshRate); from Lwjgl3ApplicationConfiguration. It potentially messes up rendering. I'll explain you why:

I was migrating my project to the new gdx liftoff project structure. Everything was greater. But then I noticed that fast moving objects suddenly were laggin/stuttering once every second.
After a lot of checking and testing I finally found the issue. My monitor (Asus 27 L MG279Q) has a refresh rate of 59.951Hz. Lwjgl3ApplicationConfiguration.getDisplayMode().refreshRate returns 59 for it. configuration.setForegroundFPS(59) now messes up the vsync as it limits rendering to 59 fps... not the nearly 60 the monitor can do...
Removing configuration.setForegroundFPS(Lwjgl3ApplicationConfiguration.getDisplayMode().refreshRate); from my config fixes the problem and everything is back to smooth again.

Please consider removing configuration.setForegroundFPS(Lwjgl3ApplicationConfiguration.getDisplayMode().refreshRate); as I for sure will not be the only one encountering this problem.

@Frosty-J
Copy link
Contributor

It also screws up if the refresh rate is raised after the game is launched (from changing display modes or being dragged to a different monitor). Would not recommend using this implementation in production.

@tommyettinger
Copy link
Member

The issue here seems to be that the refresh rate isn't getting rounded, but rather truncated or floored -- I don't think it should be an int at all. There were serious issues reported with VSync not being enabled properly on some (Linux) hardware, even when VSync has been set on in the launcher, and if the refresh rate was set to 0 (unlimited), it used a ton of GPU time and got very, very hot. So, some limit is required. I think we should be fine, honestly, with just adding 1 to refreshRate to account for fractional differences. We'd be leaving VSync on so when that setting works, it's limited to the actual refresh rate, and when VSync settings don't work, it's not uncontrollably higher. It doesn't help with @Frosty-J 's multiple monitor quirk, but I don't have multiple monitors, so he can send a PR if he wants. I know there have been really insane bugs with monitors that have different refresh rates before, unrelated to Liftoff. That seems like a disaster magnet, so I'm not inclined to put much effort into changing behavior for that setup.

tommyettinger added a commit that referenced this issue Aug 11, 2024
Let's see if this works.
@Frosty-J
Copy link
Contributor

Frosty-J commented Aug 11, 2024

Well like I said, it's still an issue for single-monitor setups, be it from the user changing their refresh rate in OS settings after launching the game, the game changing it based on the user's full screen preference, or the user's system changing it automatically (e.g. 60Hz on battery power, 165Hz on AC).

It may be possible to detect if Mesa's software renderer is active and engage the software-driven frame rate cap in that scenario, if that's your Linux issue. Not ideal but better than nothing.

@tommyettinger
Copy link
Member

If the refresh rate really is changing that often at runtime, then I don't see how it's the launcher's responsibility to handle it, since it literally sets that limit once, and users can change it themselves rather easily. Either users could set the refresh rate once a second or something with the current monitor state, or libGDX could be changed in some way to handle Vsync failures better (which is probably an issue in LWJGL3 or GLFW, if it isn't in some Linux-related thing).

I could also just set the frame limit to 300FPS, since I don't know of any hardware that can display at a higher rate, and have Vsync do its thing as well as it can. If Vsync fails, it will be limited to 300FPS on a monitor that's almost guaranteed to have a lower refresh rate, and will probably have screen tearing.

@Frosty-J
Copy link
Contributor

Yes, setting the fps cap periodically instead of only at launch would be better, perhaps with a sanity check since I haven't looked at how getDisplayMode() behaves when there's no active monitor.

@tommyettinger
Copy link
Member

Can anyone interested in this try the solution from the current Liftoff? You don't need to make a new project, since the change is just having Vsync enabled, and using

        configuration.setForegroundFPS(Lwjgl3ApplicationConfiguration.getDisplayMode().refreshRate + 1);

(With the + 1 at the end.) Rounding issues can only be off by as much as 1, so adding 1 and using VSync to clamp to the actual monitor refresh rate if possible should work in this case. Though... a different monitor refresh rate could be very different, and I don't know how any apps or OSes handle that with Vsync. What if a window is moved halfway between two monitors, one with 150Hz and one with 59.951Hz? I suspect using the lower cap is better for screen tearing reasons, unless Vsync already limits one half of the window to a lower refresh rate somehow, which seems unlikely.

@Frosty-J
Copy link
Contributor

Frosty-J commented Aug 20, 2024

Moving the window halfway between two monitors should just vsync to whichever monitor has most of the window. This is OS-dependent, e.g. I have heard older versions of Windows will always sync to the lowest refresh rate monitor. The same can be observed with HDPI applications - they may look too big or small when partially on a monitor of a different density. Since we're only checking at startup, and windows don't normally start straddled between monitors, this isn't a concern.

getFramesPerSecond() readings. Left monitor is 59.88Hz; right monitor is 72Hz:

vsync.mp4

In the event that the monitors are powered by different graphics cards, it seems to continue vsyncing with the card it is running on rather than the one it is being displayed on. To clarify: if an application is running on card 1 but displayed on a monitor driven by card 2, its buffer gets copied to card 2 each frame to be displayed (in order to see - nothing specific to GDX/LWJGL).

Most users will never experience screen tearing in windowed mode (though some will, e.g. Windows XP users, if it still works, and Linux users with a software display compositor such as Marco) - 60fps at 150Hz should just result your game alternating between showing 3 and 4 consecutive duplicate frames, in practice could be messier.

I have tested Lwjgl3ApplicationConfiguration.getDisplayMode() with a custom video mode (running 60Hz monitor at 72Hz) and I'll tell you why: it isn't in the getDisplayModes() list and LWJGL won't go full screen with it. getDisplayMode() returns it fine, so I'm pleased to report that's not an issue for windowed applications.

@TCreutzenberg
Copy link
Contributor Author

Regarding the FPS +1: I actually did a big refactor to my project and it is not compiling again yet... I will test it if it is running again.

@tommyettinger
Copy link
Member

@Frosty-J , any thoughts on the +1 to account for rounding error, and otherwise relying on Vsync?

@Frosty-J
Copy link
Contributor

+1 should be +enough. It's unfortunate how much stuff in computing doesn't handle fractional refresh rates such as 60/1.001 correctly.

Vsync good. Setting your own frame rate cap bad. Setting games to run at half vsync... now there's your answer to making it cinematic.

@TCreutzenberg
Copy link
Contributor Author

TCreutzenberg commented Aug 29, 2024

+1 works fine for me. No stutters.

@SonicGDX
Copy link

SonicGDX commented Sep 18, 2024

Wouldn't a ceiling operation be better than +1? So if the refresh rate is a whole number it won't increase

EDIT: I guess the issue is if using a library implementation of ceil it would add another import.

@TCreutzenberg
Copy link
Contributor Author

TCreutzenberg commented Sep 18, 2024

@SonicGDX The api call returns a bottom value, e.g. 59.999 will be returned as 59, and ceil of 59 is still 59...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants