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

chore(testbed): step at a fixed interval #144

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

alexandernanberg
Copy link
Contributor

@alexandernanberg alexandernanberg commented Jun 21, 2022

Why

Ensure simulation runs at the same speed on different monitors (60hz/120hz etc)

Changes

  • Remove dead highlight code
  • Run world.step at a fixed consistent rate (60hz)
  • Interpolate position of graphics objects

Loosely based on https://gafferongames.com/post/fix_your_timestep/

@alexandernanberg
Copy link
Contributor Author

Hmm this completely destroyed the perf of the keva tower demo, probably because each step takes more than 16ms 🤔

@LeXXik
Copy link

LeXXik commented Jun 21, 2022

Could you please record a few seconds of performance log in chrome dev tools? Curious to what exactly is tanking the performance.

@alexandernanberg
Copy link
Contributor Author

alexandernanberg commented Jun 21, 2022

Here is the profile Profile-20220621T200802.json.zip. This is what you were asking for right, or did you want screenshots?

Looks like because the fixed update step will ALWAYS run it builds up a lot of work in 1 frame when world.step take more than a few ms to complete

@LeXXik
Copy link

LeXXik commented Jun 21, 2022

Yes, that was what I wanted to look at. Not sure what is causing the so many steps to be executed in a single frame yet.

One thing - this part should be outside of while loop, after it ended. We are probably interested in taking the snapshot of the current state, the intermediate steps won't be visible, as their states won't propagate to the renderer.

                if (!!this.parameters.debugInfos) {
                    let t0 = performance.now();
                    let snapshot = this.world.takeSnapshot();
                    let t1 = performance.now();
                    let snapshotTime = t1 - t0;

                    let debugInfos: DebugInfos = {
                        token: this.demoToken,
                        stepId: this.stepId,
                        worldHash: "",
                        worldHashTime: 0,
                        snapshotTime: 0,
                    };
                    t0 = performance.now();
                    debugInfos.worldHash = md5(snapshot);
                    t1 = performance.now();
                    let worldHashTime = t1 - t0;

                    debugInfos.worldHashTime = worldHashTime;
                    debugInfos.snapshotTime = snapshotTime;

                    this.gui.setDebugInfos(debugInfos);
                }`

@sebcrozet
Copy link
Member

Thank you for this PR. However, I’m not sure that this is the actual cause of #92 and dimforge/rapier#345. I think these problems were related to the way our webworker was setup. So I believe that these issues are already fixed (I just didn’t upload the updated demos until now).

@alexandernanberg
Copy link
Contributor Author

Ah that could be the case! But I'm not 100% the demos are deterministic right now since requestAnimationFrame runs in at a variable rate. E.g. I have a 120hz monitor so the demos run a double speed for me compared to 60hz.

Maybe a good enough solution is just to get the time delta in requestAnimationFrame and set world.timestep before stepping?

@sebcrozet
Copy link
Member

Ah that could be the case! But I'm not 100% the demos are deterministic right now since requestAnimationFrame runs in at a _» > variable rate. E.g. I have a 120hz monitor so the demos run a double speed for me compared to 60hz.

Determinism means that for the same number of physics steps, you get the exact same result. It is unrelated to the frequency at which you call world.step. So, with both a 120Hz and 60Hz monitor, you will get the exact same result after the 60th step even if it took half a real-world second to run these with the 120Hz monitor, and a full real-world second to run them in the 60Hz monitor.

In other words determinism doesn’t mean you get the same result after the same amount of real-world time. It means you get the same result after the same amount of physics-world time.

Maybe a good enough solution is just to get the time delta in requestAnimationFrame and set world.timestep before stepping?

Setting the world.timestep to the delta from requestAnimationFrame would effectively break determinism (resulting in different results) because you will be running the simulation with different parameters.

@alexandernanberg
Copy link
Contributor Author

I think I get it. Would determinism break if the app drops some frames? E.g. some intensive is happening and fps drop from 60hz to 50 etc.

How is it recommended to step in apps right now if you don't want simulations on 120hz screens to run faster than on 60hz?

@sebcrozet
Copy link
Member

sebcrozet commented Jun 22, 2022

The app is free to drop any frame they want. All that matters is that at all times, the physics is stepped with the same inputs. The app can choose to step whenever they want (for example, it chooses not to call the physics stepping at all when you pause the simulation, this doesn’t break determinism).

How is it recommended to step in apps right now if you don't want simulations on 120hz screens to run faster than on 60hz?

To clarify my first comment on this thread: my intent was only to say that this PR isn’t really addressing #92 and dimforge/rapier#345. However, I do believe this PR is a good addition, because stepping the physics 60 times per real-world second (while keeping world.timestep constant to 1.0 / 60.0, and interpolating positions for rendering), independently from the framerate/monitor frequency is the right approach here to address the difference of perceived simulation speed.

If the framerate is smaller than 60Hz (because running on a 30Hz monitor, or because the physics takes more than 0.016 seconds to compute), I suggest simply running the stepping only once per "slow" frame. Real games may want to do extrapolations or other strategies, but I don’t think it’s worth it for simple demos.

In other words:

  • If elapsed real-world time since last physics step< 0.016 seconds, then don’t step.
  • If elapsed real-world time since last physics step >= 0.016 seconds, then step the physics exactly once (i.e. don’t try to step it multiple times if the time since last step > 0.032).

@alexandernanberg
Copy link
Contributor Author

alexandernanberg commented Jun 22, 2022

Gotcha! I see, makes sense. Updated the PR description.

I updated the PR to work like you described (I think), but with the new approach I don't really know how to calculate the alpha used for interpolation. Any ideas?

Side note, the new approach fixed the perf issues on the keva tower demo 🎉

@alexandernanberg
Copy link
Contributor Author

@LeXXik

One thing - this part should be outside of while loop

Not sure I understand why, IMO it only makes sense to create a snapshot each time we've run world.step? Or maybe this isn't relevant anymore since the while loop is gone?

@alexandernanberg alexandernanberg force-pushed the an/chore/testbed-fixed-update-step branch from be86fdf to e982ce5 Compare May 5, 2023 15:51
@alexandernanberg
Copy link
Contributor Author

@sebcrozet Made some more improvements and added the same logic to the 2d testbed, from my testing everything looks good but let me know if there is anything I've missed or you want me to change!

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.

None yet

3 participants