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

fix: Use effective tpf value in LoopRunner (onUpdate) #1298

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

DeathPhoenix22
Copy link
Contributor

@DeathPhoenix22 DeathPhoenix22 commented Oct 10, 2023

Hi,

Issue Link: #1297

This PR contains fixes for the FPS & the TPF calculations. The FPS calculation is now refreshed every 500 millis. The TPF will be based on current TPF.

Note that this change was a game changer. I was experiencing frequent freeze and FPS issues (I was wondering also why the Profiler was showing 60 steady FPS when it was lagging). With this change, the Profiler will show realtime FPS and recover from lag instantly (when possible).


listOf(
// run with a given ticks per second (via scheduled service tick)
LoopRunner(60) { t += it; Thread.sleep(lag) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that this test case doesn't use: LoopRunner { t += it; Thread.sleep(lag) } case.
Mainly, JavaFX will not be affected by the Lag and will process 60 frames anyway.

runnable(tpf)
tpf = (now - lastFrameNanos).toDouble() / 1_000_000_000

val ticksPerSecond = if (ticksPerSecond < 0) 60 else ticksPerSecond // For JavaFX loops, cap at 60fps too
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that this is a safety mesure. I was supprised to see that JavaFX was defaulted to 120FPS on a Mac (instead of the 60FPS defined in the JavaFX documentation). In order to make tests "predictable", I had to cap it to 60FPS as each LoopRunner { t += it; Thread.sleep(lag) } test cases were failing by returning 120FPS

@DeathPhoenix22
Copy link
Contributor Author

Hi @AlmasB,

I've resolved a bug where pausing would cumulate all tpf while paused. I've adjusted the loop and the tests to reflect that.

Sorry for that!

@DeathPhoenix22
Copy link
Contributor Author

Hi @AlmasB ,

I've been using this fix for quite a while now. I've since experienced stable FPS. I must say that this change is a game changer for the Engine. The previous LoopRunner was causing "fake" lag (highly noticeable whenever it happened).
I must admit that my project requires serious resources (RAM, CPU, GPU, HDD, etc), so I was facing the "fake lag" frequently.
I can't wait to show my project to the FXGL community, but it's not ready yet. (It's an RTS)

@DeathPhoenix22
Copy link
Contributor Author

DeathPhoenix22 commented Jan 31, 2024

Without this PR, my game wouldn't run. The more resources your game needs, the worst the FPS gets without this fix.
Also, the FPS indicated in the Profiling tool aren't accurate.

@AlmasB
Copy link
Owner

AlmasB commented Jan 31, 2024

Noted, I'll take a look at it over the weekend.

Copy link
Owner

@AlmasB AlmasB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments below

if (lastFPSUpdateNanos == 0L) {
lastFPSUpdateNanos = now
fpsBuffer2sec = 0
val ticksPerSecond = if (ticksPerSecond < 0) 60 else ticksPerSecond // For JavaFX loops, cap at 60fps too
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When ticksPerSecond is -1 or 0, JavaFX will match the display refresh rate (which is what we want), so we shouldn't cap it at 60

if (now - lastFPSUpdateNanos >= 2_000_000_000) {
// Update the FPS value every 500 millis
val timeSinceLastFPSUpdateNanos = now - lastFPSUpdateNanos;
if (timeSinceLastFPSUpdateNanos >= 500_000_000) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's beneficial for the developers to have access to this "fps refresh interval", then we move the responsibility from us to them. We use a default value of X and let them pick a different one to suit their project.

@DeathPhoenix22
Copy link
Contributor Author

Hi @AlmasB,

I've added the possibility to configure the FPS sampling rate.

Thanks,

@AlmasB AlmasB added this to the 21.2 milestone Mar 31, 2024
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

Successfully merging this pull request may close these issues.

2 participants