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

Improve frame synchronization #2610

Open
theyareonit opened this issue Jul 20, 2024 · 6 comments · May be fixed by #2618
Open

Improve frame synchronization #2610

theyareonit opened this issue Jul 20, 2024 · 6 comments · May be fixed by #2618
Labels
F-help-wanted Flag: Help wanted S-accepted Status: Request accepted but needs volunteer T-enhancement Type: Enhancement
Milestone

Comments

@theyareonit
Copy link

Request Description

The function RenderSystem.limitDisplayFPS() in vanilla Minecraft currently looks something like this:

    public static void limitDisplayFPS(int fps) {
        double target = lastDrawTime + 1.0 / (double)fps;

        double now;
        for (now = GLFW.glfwGetTime(); now < target; now = GLFW.glfwGetTime()) {
            GLFW.glfwWaitEventsTimeout(target - now);
        }

        lastDrawTime = now;
    }

However, this is a flawed implementation. For example, if you set 240FPS as your cap, it will result in an actual frame limit of around ~235FPS, due to the fact that glfwWaitEventsTimeout() and glfwGetTime() do not return instantly, resulting in the now timestamp differing from the target timestamp by enough to cause significant desync.

The solution is simply to replace the line lastDrawTime = now; with lastDrawTime = target;, which results in a smoother experience when using capped FPS. However, this leads to an incorrect frame limit for the first few seconds after joining a server or tabbing into the game, so some more work may be needed.

@theyareonit theyareonit added S-needs-triage Status: Needs triage T-enhancement Type: Enhancement labels Jul 20, 2024
@jellysquid3
Copy link
Member

jellysquid3 commented Jul 23, 2024

That's curious. Since you appear to have figured this out already, we would not be opposed to a pull request to address this problem. Maybe we can also look into further improvements since the original Minecraft code seems rather naive.

@jellysquid3 jellysquid3 added S-accepted Status: Request accepted but needs volunteer F-help-wanted Flag: Help wanted and removed S-needs-triage Status: Needs triage labels Jul 23, 2024
@theyareonit
Copy link
Author

theyareonit commented Jul 23, 2024

Alright. I found some places where I can improve it (and fixed the issue of getting the wrong FPS value after tabbing in), so I'll try to make a PR soon.

@theyareonit theyareonit linked a pull request Jul 24, 2024 that will close this issue
@Felix14-v2
Copy link

If I understand correctly, this bug is quite useful for me. My laptop supports a screen refresh rate of up to 144 Hz, but in Minecraft I can only use a 150 FPS limit to make the game smooth. Due to this effect, I get ≈144 FPS on average.

@theyareonit
Copy link
Author

theyareonit commented Jul 28, 2024

If I understand correctly, this bug is quite useful for me. My laptop supports a screen refresh rate of up to 144 Hz, but in Minecraft I can only use a 150 FPS limit to make the game smooth. Due to this effect, I get ≈144 FPS on average.

I think that just means some mod needs to allow the user to input any FPS value instead, lol.

@jellysquid3
Copy link
Member

If I understand correctly, this bug is quite useful for me.

@Felix14-v2
Copy link

Yeah, so true. Just hope that one day modern screen formats support in Minecraft will be better.

@jellysquid3 jellysquid3 modified the milestones: Sodium 0.6, Sodium 0.6.1 Aug 8, 2024
@CaffeineMC CaffeineMC deleted a comment from xboct02 Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-help-wanted Flag: Help wanted S-accepted Status: Request accepted but needs volunteer T-enhancement Type: Enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants