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

Performance regression #36

Closed
mrdoob opened this issue Jun 23, 2020 · 12 comments · Fixed by #40
Closed

Performance regression #36

mrdoob opened this issue Jun 23, 2020 · 12 comments · Fixed by #40

Comments

@mrdoob
Copy link

mrdoob commented Jun 23, 2020

We are considering using cannon-es in three.js but seems like there's a performance regression.

@Mugen87 converted cannon.js to modules and here's the running example:
https://raw.githack.com/mrdoob/three.js/dev/examples/physics_cannon_instancing.html

Screen Shot 2020-06-22 at 11 14 43 PM

This is the same example using cannon-es:
https://raw.githack.com/Mugen87/three.js/dev45/examples/physics_cannon_instancing.html

Screen Shot 2020-06-22 at 11 12 39 PM

mrdoob/three.js#19709

@marcofugaro
Copy link
Member

Thanks for the issue and the usage example.
This also looks related to #16. I'll look into it soon!

@codynova
Copy link
Member

codynova commented Jun 23, 2020

I only had a moment to look at this as I'm headed to bed but I definitely agree it looks related to #16 and #18

That githack service is awesome, I've never seen that before.

@arpu
Copy link

arpu commented Jun 23, 2020

maybe i am wrong here but the last para should be not the frameRate
https://github.com/mrdoob/three.js/blob/dev/examples/jsm/physics/CannonPhysics.js#L193

more a value of 10 for maxSubSteps

@marcofugaro
Copy link
Member

marcofugaro commented Jun 27, 2020

Hello friends, I've discovered the issue.

Basically there are tons or commit which are in the cannon.js repo but haven't been published to npm or built into the cannon.js bundle. Here is the full list.
When we built the source files again in cannon-es, we inherited automatically all those commits.

Between these commits there is 86b0444, where schteppe implemented the fixed timestepping from the last snippet of this famouse article, however it was left unfinished or was not tested properly, leading to the framerate issues.

I've tried to fix those issues in #40, but of course I'm not schteppe. If you guys @mrdoob and @Mugen87 could test with the cannon-es bundle from that branch that would be grand 🙏

Also @codynova issue #16 should be fixed as well, could you do a test? 🙏

@arpu is right, the third parameter is used to tell how many maximum internal steps the step() function can take if there is too much lag. If you set it to 1, it doesn't take any internal steps, but the simulation is technically slower. If you set it to an high number, it takes as many steps as needed to make the interaction have a fixed timestep, however if the step is expensive (a lot of boxes), taking too many steps can cause lag by itself.

@Mugen87
Copy link

Mugen87 commented Jun 27, 2020

@marcofugaro The performance seems better with your changes. However if I have more than 400 cubes in the scene, the performance drops significantly to an unusable framerate (approx. 8 FPS). This does not happen with the original cannon.js version.

@marcofugaro
Copy link
Member

marcofugaro commented Jun 27, 2020

@Mugen87 Oddly enough, that's a feature.
Please read the description of #40, it's the whole idea behind this article

If you want the behaviour of the previous cannon.js, you can call world.step(frameTime) without any other arguments or just world.step(frameTime, delta, 1). The simulation with 400 boxes will run "in slow motion" but at a stable framerate.

However now that you mention it, it makes sense to have the threshold at at least 30fps, not 10.

EDIT: I've updated the PR, could you test now?

@marcofugaro
Copy link
Member

@Mugen87 do you have time to test again? 🙏
It shouldn't go below 30fps, rather it will slow down a bit.

@Mugen87
Copy link

Mugen87 commented Jul 4, 2020

Performance seems better now. Mostly around 25-27 FPS when having 400 cubes in physics_cannon_instancing.

However, the performance of cannon-es is still worse than the original cannon.js. With the same settings (400 cubes in physics_cannon_instancing), I get around 52 FPS with cannon.js.

@marcofugaro
Copy link
Member

Alright, that is working as intended! 🎉

The simulation now has more time to catch up than before, this works in favor of people using cannon.js in a web worker.

Users can still get the previous cannon.js behaviour (~60fps) by passing 1 to the third argument of the step() function (suggested when there are a lot of bodies to simulate).

world.step(frameTime, delta, 1)

Thanks for testing again!

@Mugen87
Copy link

Mugen87 commented Jul 4, 2020

So THREE.CannonPhysics needs an change in the following place to restore the "old" behavior?

https://github.com/mrdoob/three.js/blob/3ca144cac1c81ada7496e2f4790877848c6b4aa9/examples/jsm/physics/CannonPhysics.js#L193

@marcofugaro
Copy link
Member

marcofugaro commented Jul 4, 2020

Yup, currently in that line, the third parameter frameRate is wrong, as it's not related to the framerate at all.

It should be called maxSubSteps and should be set to a value of 1 or a really small integer in your case.
It was like this also in the original cannon.js.

We should really write this stuff in the docs once they are up.

@codynova
Copy link
Member

codynova commented Jul 6, 2020

A new version of cannon-es has been released with Marco's fixes for variable framerate simulation cannon-es@0.15.0

This issue was closed.
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 a pull request may close this issue.

5 participants