dimforge / rapier

2D and 3D physics engines focused on performance.
https://rapier.rs
Apache License 2.0
3.89k stars 242 forks source link

Possible Performance Bug on Physics::step, specifically Query::update_incremental #466

Closed cybersoulK closed 5 months ago

cybersoulK commented 1 year ago

in rapier3d 0.17.2, i was having a bug that slows down my game state from 120fps + to 30 fps (the same effect as a memory leak, it keeps lowering down)

When i set the step with Some(&mut self.query_pipeline), instead of manually updating the query_pipeline.

It could be possibly because the ::step uses Query::update_incremental instead of Query::update, and there might be a bug related to update_incremental.

LPGhatguy commented 1 year ago

We're also running into this issue. In our case, PhysicsPipeline::step doesn't get any slower, but all of our spatial queries get slower over time. In particular, QueryPipeline::cast_ray_and_get_normal grows to be ~12x slower over the course of about a minute running our game.

When we downgrade the game to rapier3d 0.15.0 with the same game, this issue disappears.

Could this be related to dimforge/parry#136? Both issues came about around the same time for us.

LPGhatguy commented 1 year ago

I was able to fix this regression by changing PhysicsPipeline::step to call QueryPipeline::update instead of QueryPipeline::update_incremental. I think this narrows the issue down to Parry 0.13's new QueryPipeline::update_incremental method.

I've published this change to a fork available here: https://github.com/SecondHalfGames/rapier/tree/update-not-incremental

index 358efed..9301408 100644
--- a/src/pipeline/physics_pipeline.rs
+++ b/src/pipeline/physics_pipeline.rs
@@ -489,7 +489,7 @@ impl PhysicsPipeline {
         );

         if let Some(queries) = query_pipeline.as_deref_mut() {
-            queries.update_incremental(colliders, &modified_colliders, &removed_colliders, false);
+            queries.update(bodies, colliders);
         }

         self.clear_modified_colliders(colliders, &mut modified_colliders);
LPGhatguy commented 1 year ago

More context as we've investigated this: using update instead of update_incremental is not a perfect fix. It appears that using update instead causes the QVBH to become corrupt. Raycasts and shapecasts stop colliding with a lot of the geometry in our scene with this patch.

Here is our kinematic character controller interacting with a table in our game, using rapier3d 0.17.2:

https://user-images.githubusercontent.com/654598/231934774-2486007b-13a6-461b-b6ce-fb12adf7d0ae.mp4

And here is the exact same code and scene, but on rapier3d master + our patch above:

https://user-images.githubusercontent.com/654598/231934858-b814aa33-485f-4449-ad1c-09764e11611c.mp4

As you can see, something about using QueryPipeline::update instead of QueryPipeline::update_incremental breaks the query pipeline. The first way we noticed this manifesting is that this patch also causes about half our zombies to fall through the floor.

We're in kind of a tough spot with this issue. rapier3d 0.15.0 works but is missing a few features that we need. rapier3d 0.17.2 has this performance regression, which is a show-stopper for us. Our patch fixes the performance issues but creates new problems.

Is there any information (or funding) we can provide to help track down this issue sooner than later? Thanks!

LPGhatguy commented 1 year ago

Last update on this! Passing None to PhysicsPipeline::step and updating the query pipeline myself works fine. No performance regressions or weird issues.

sebcrozet commented 1 year ago

This definitely indicates a bug in QueryPipeline::update_incremental.