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
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 25 additions & 18 deletions fxgl/src/main/kotlin/com/almasb/fxgl/app/LoopRunner.kt
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ internal class LoopRunner(
private set

private var lastFPSUpdateNanos = 0L
private var fpsBuffer2sec = 0
private var fpsSamplingCount = 0
private var lastFrameNanos = 0L

private val impl by lazy {
if (ticksPerSecond <= 0) {
Expand Down Expand Up @@ -76,15 +77,14 @@ internal class LoopRunner(
fun resume() {
log.debug("Resuming loop")

lastFrameNanos = 0
impl.resume()
}

fun pause() {
log.debug("Pausing loop")

impl.pause()

lastFPSUpdateNanos = 0L
}

fun stop() {
Expand All @@ -94,29 +94,36 @@ internal class LoopRunner(
}

private fun frame(now: Long) {
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 (lastFrameNanos == 0L) {
lastFrameNanos = now - (1_000_000_000.0 / ticksPerSecond).toLong()
lastFPSUpdateNanos = lastFrameNanos
fpsSamplingCount = 1
}

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

// The "executor" will call 60 times per seconds even if the game runs under 60 fps.
// If it's not even "half" a tick long, skip
if(tpf < (1_000_000_000 / (ticksPerSecond * 1.5)) / 1_000_000_000 ) {
return
}

fpsBuffer2sec++
fpsSamplingCount++

// if 2 seconds have passed
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.

lastFPSUpdateNanos = now
fps = fpsBuffer2sec / 2
fpsBuffer2sec = 0
fps = (fpsSamplingCount.toLong() * 1_000_000_000 / timeSinceLastFPSUpdateNanos).toInt()
fpsSamplingCount = 0
}

// tweak potentially erroneous reads
if (fps < 5)
fps = 60
lastFrameNanos = now

// update tpf for the next 2 seconds
tpf = 1.0 / fps
cpuNanoTime = measureNanoTime {
runnable(tpf)
}
}
}
Expand Down
69 changes: 57 additions & 12 deletions fxgl/src/test/kotlin/com/almasb/fxgl/app/LoopRunnerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -106,44 +106,89 @@ class LoopRunnerTest {
fun `LoopRunner resets ticks after pause`() {
var count1 = 0.0
var count2 = 0.0
val frameTime = 1_000L / 60

listOf(
// run with a given ticks per second (via scheduled service tick)
LoopRunner(60) {

count1 += it
},

// run with display refresh rate (via JavaFX pulse tick)
LoopRunner {

count2 += it
}
).forEach {
it.start()

// 16.6 per frame, so 10 frames
Thread.sleep(166)
// 16.6 per frame, so 50 frames
Thread.sleep(frameTime * 50)

it.pause()

// sleep for 150 frames = 2.5 sec
Thread.sleep(166 * 15)
Thread.sleep(frameTime * 150)

it.resume()

// 16.6 per frame, so 10 frames
Thread.sleep(166)
// 16.6 per frame, so 50 frames
Thread.sleep(frameTime * 50)

it.stop()
}

// in total we should have computed 20 frames, ~20 * 0.017 = ~0.34
// We processed approximately 100 frames (150 where in Pause)
assertThat(count1, greaterThan((98 * frameTime).toDouble() / 1_000))
assertThat(count1, lessThan((102 * frameTime).toDouble() / 1_000))

assertThat(count1, greaterThan(0.0))
assertThat(count1, lessThan(0.75))
assertThat(count2, greaterThan((98 * frameTime).toDouble() / 1_000))
assertThat(count2, lessThan((102 * frameTime).toDouble() / 1_000))
}

assertThat(count2, greaterThan(0.0))
assertThat(count2, lessThan(0.75))
@Test
@EnabledIfEnvironmentVariable(named = "CI", matches = "true")
fun `Lag Recovery`() {
var t = 0.0
var lag = 200L

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.

).forEach { loop ->
t = 0.0

loop.start()

Thread.sleep(2500) // Sample for more than 2 seconds, to cover the 2SecsBuffer case

loop.pause()

// We know that a single tick will take at least "lag" millis, so TPFs should be around 200 millis
assertThat(loop.tpf, closeTo(lag.toDouble() / 1000.0, 0.02))
assertThat(loop.fps.toDouble(), closeTo(5.0, 1.0))

// The game loop should have completed 2.5 seconds of game time at this stage
assertThat(t, closeTo(2.5, 0.2))

lag = 1L // Stop Lag

loop.resume()

Thread.sleep(1000) // Need to wait at least 2 seconds for the FPS sampling to recalculate

loop.stop()

// The 2 seconds Buffer shouldn't cause tpf to be 200 millis anymore
assertThat(loop.tpf, closeTo(0.016, 0.09))

assertThat(t, closeTo(3.5, 0.4))

// shouldn't change anything since loop is stopped
Thread.sleep(300)

assertThat(loop.tpf, closeTo(0.016, 0.09))

assertThat(t, closeTo(3.5, 0.4))
}
}
}