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

Separate narrow phase from solver into NarrowPhasePlugin #100

Merged
merged 11 commits into from
Jul 31, 2023
Merged

Conversation

Jondolf
Copy link
Owner

@Jondolf Jondolf commented Jul 29, 2023

Background

Currently, narrow phase collision detection is tightly coupled with the penetration constraint solver. This makes implementing more advanced collision detection features like child colliders quite cumbersome and difficult, makes things much harder to parallelize, makes it impossible to use custom collision detection implementations without rewriting the entire solver, and is overall very annoying to work with.

To make things more modular and easier to work with, it would be great to have a separate narrow phase. However, XPBD has to deal with changing constraint directions and positions in the solver, which can lead to bodies moving into a penetrating state from a non-penetrating state. This can lead to missed collisions if collisions are computed just once before the solver, which is why the separate narrow phase hasn't been implemented earlier.

Implementation

This PR separates the narrow phase into a NarrowPhasePlugin that nicely encapsulates all collision detection logic and handles collision events. The solver then iterates through the collisions, and creates and solves penetration constraints for them. As a nice bonus, colliders no longer have to be attached to rigid bodies for collisions to be detected.

To solve the problem with missed collisions, the narrow phase uses a "prediction distance". It essentially allows "speculative contacts" between entities that aren't quite penetrating yet but might penetrate at some point during the constraint solve, and the penetration constraints check if the bodies are actually colliding by computing the penetration depth at the current state.

In addition, this PR revamps how collision events and CollidingEntities are handled in order to resolve some problems. Collision events are only sent at the end of each run of the PhysicsSchedule instead of during each substep, which from the user's perspective eliminates duplicate events, since users don't generally schedule their systems inside the SubstepSchedule. CollidingEntities are also updated based on the events that are sent at the end of the physics schedule, so you won't get missing entities in cases where the bodies don't happen to be penetrating during the last substep.

Finally, I added a Collisions resource that stores all collisions between colliders in an IndexMap that uses fxhash. This is mainly used for the collisions event to track when entities begin/stop penetrating, but in the future, it could be used to optimize the contact manifold computation by giving Parry the previous contact manifolds. Collisions also has some potentially useful methods like collisions_with_entity that people can use.

Performance

Separating the narrow phase from the solver like this increases the number of get_many/get_many_mut calls quite significantly. However, I managed to do some other optimization to the narrow phase, and I implemented simple multithreading using Bevy's ComputeTaskPool. This multithreading is controlled by the new parallel feature, which is enabled by default.

Below are Tracy traces that show the performance of the most expensive systems. The traces were running for slightly different durations, so the percentages are more important than the raw lengths in seconds. They were run on a 13th Gen Intel i7-13700F.

Previously:

Old performance

Now, with parallel feature disabled:

New performance, without multithreading

Now, with parallel feature enabled:

New performance, with multithreading

As you can see, the performance seems to be slightly better overall, especially with multithreading, at least on my machine. In cases where there are very few collisions, the multithreading can cause unnecessary overhead, but in general it should be faster. However, Criterion benchmarks seem to indicate that performance is worse, while in my testing it seems better, so the results are a bit contradictory.

The multithreaded version also seems to be deterministic, but I'm not sure if par_splat_map should actually be deterministic, so it could be just coincidence. If it is, we can just sort the collisions by e.g. entity.

Future work

Separating the narrow phase like this makes it much easier to implement more collision detection features, like the following:

  • Child colliders, multiple colliders per body
  • Collision filters
  • Parallel solver

These will be implemented in future PRs.

Todo

  • Filter out non-penetrating collisions before the end of the physics frame
  • Handle CollisionStarted, CollisionEnded and CollidingEntities correctly; they shouldn't be sent/cleared at every substep, but rather at the end of every physics frame
  • Improve performance

@Jondolf Jondolf added C-Enhancement New feature or request A-Collision Relates to the broad phase, narrow phase, colliders, or other collision functionality labels Jul 29, 2023
- Added `Collisions` resource that stores all contact pairs
and has helper methods for accessing them
- Collision events are now sent at the end of each physics frame
instead of after each substep, same for colliding entities
This should have some benefits over `HashMap`
in terms of iteration speed and order,
but we can change it again later if it has unwanted effects.
@Jondolf Jondolf merged commit b491b23 into main Jul 31, 2023
3 checks passed
@Jondolf Jondolf deleted the narrow-phase branch July 31, 2023 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Collision Relates to the broad phase, narrow phase, colliders, or other collision functionality C-Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant