bevy_rapier icon indicating copy to clipboard operation
bevy_rapier copied to clipboard

Rapier context as a Component

Open ThierryBerger opened this issue 1 year ago • 1 comments

Alternative to support multiple worlds: https://github.com/dimforge/bevy_rapier/pull/328 + lays out the foundation to split RapierContext in multiple elements https://github.com/dimforge/bevy_rapier/issues/502, still grouped in an entity.

  • By using SystemParams for a DefaultRapierContext, the migration for most users is painless.
  • Users developing libraries should go through a more involved migration by supporting multiple worlds.

Strengths

  • rather low impact on existing users, check the examples.
  • using a RapierContext as component avoids maintaining our own indices for worlds, and groups up the dependencies of a RapierContext in the same entity. I believe it will be most helpful when ECS relations will be a thing.

Weaknesses

  • ⚠️ “unwrap” heavy code, relying on those components being added correctly: (bevy relations please 🥺🌈) ; most should probably be if let else.
    • RapierContextEntityLink on every rapier entities
      • Rigidbodies
      • joints
      • Colliders
      • Others ?
  • The root “physics object (linked to by RapierContextEntityLink) ; should always have:
    • RapierContext -> its acces is simplified with RapierContextAccess
    • RapierConfiguration -> as its usage is more advanced, I think it's fine to keep it in queries.
    • RenderToSimulationTime -> same as RapierConfiguration
    • Maybe I’ve forgotten some..?
  • ⚠️ performance: Configuration::physics_pipeline_active being in the RapierContext entity leads to iteration to all entities for each worlds even though it’s not necessary 🙁 (writeback for example)
    • I imagine performance when disabled is not what we should thrive to optimize, but still a pity.
  • ⚠️ Change detection, tracking all these accesses to RapierContext is not trivial, and we should evaluate regressions. For now all systems share all these things, which I consider low priority to specialize for each RapierContext (them being components would be interesting but not trivial):
    • Their schedule
    • The timestep mode
    • The physics hook
    • DebugRenderContext
      • might be easy to migrate to a component ; it does involve a pipeline creation though.
      • So probably keep the pipeline as a resource, but have the enabled flag as a component on each RapierContext.
  • Testbed uses DefaultRapierContextAccess, I think that’s OK.
  • there's not a tremendous amount of tests, I noticed I broke events very stealthily so I added a regression test https://github.com/dimforge/bevy_rapier/pull/544

Performance analysis

Overall, This pull request doesn't seem to impact significantly the performance based on selected benchmarks:

Comparison with adapted benchmark with baseline from: https://github.com/dimforge/bevy_rapier/pull/551 :

Cubes "master"

     Running benches\cubes.rs (C:\Users\thier\Documents\cargo_target_dir\release\deps\cubes-c309aaa524b91fbb.exe)
Timer precision: 100 ns
cubes                     fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ cubes_10x10__30_steps  207.6 ms      │ 286.1 ms      │ 216.3 ms      │ 218.1 ms      │ 100     │ 100
├─ cubes_3x3_30_steps     5.451 ms      │ 8.012 ms      │ 5.68 ms       │ 5.851 ms      │ 100     │ 100
╰─ cubes_5x5_30_steps     19.01 ms      │ 22.84 ms      │ 19.67 ms      │ 19.98 ms      │ 100     │ 100
Cubes "This PR"

     Running benches\cubes.rs (C:\Users\thier\Documents\cargo_target_dir\release\deps\cubes-c309aaa524b91fbb.exe)
Timer precision: 100 ns
cubes                    fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ cubes_10x10_30_steps  206.4 ms      │ 234.1 ms      │ 213.5 ms      │ 215.3 ms      │ 100     │ 100
├─ cubes_3x3_30_steps    5.777 ms      │ 7.041 ms      │ 5.98 ms       │ 6.065 ms      │ 100     │ 100
╰─ cubes_5x5_30_steps    19.4 ms       │ 24.01 ms      │ 19.9 ms       │ 20.4 ms       │ 100     │ 100
many_pyramids3 "master"

     Running benches\many_pyramids3.rs (C:\Users\thier\Documents\cargo_target_dir\release\deps\many_pyramids3-22265334d52714c0.exe)
Timer precision: 100 ns
many_pyramids3               fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ pyramid_1_with_height_2   20.81 ms      │ 21.44 ms      │ 21.32 ms      │ 21.24 ms      │ 5       │ 25
├─ pyramid_1_with_height_20  1.072 s       │ 1.074 s       │ 1.073 s       │ 1.073 s       │ 2       │ 4
╰─ pyramid_2_with_height_20  4.262 s       │ 4.262 s       │ 4.262 s       │ 4.262 s       │ 1       │ 1
many_pyramids3 "This PR"

     Running benches\many_pyramids3.rs (C:\Users\thier\Documents\cargo_target_dir\release\deps\many_pyramids3-22265334d52714c0.exe)
Timer precision: 100 ns
many_pyramids3               fastest       │ slowest       │ median        │ mean          │ samples │ iters
├─ pyramid_1_with_height_2   21.05 ms      │ 21.98 ms      │ 21.26 ms      │ 21.46 ms      │ 5       │ 25
├─ pyramid_1_with_height_20  1.078 s       │ 1.08 s        │ 1.079 s       │ 1.079 s       │ 2       │ 4
╰─ pyramid_2_with_height_20  4.312 s       │ 4.312 s       │ 4.312 s       │ 4.312 s       │ 1       │ 1
custom_benches "master"

average total time: 0.024122315
average rapier step time: 0.0057954104
average bevy overhead: 0.018326905
total time: 24.19140625
average total time: 124.73746
average rapier step time: 124.52926
average bevy overhead: 0.20819855
total time: 12473.776123046875
custom_benches "This PR"

average total time: 0.023785645
average rapier step time: 0.005746582
average bevy overhead: 0.018039063
total time: 24.62060546875
average total time: 123.43043
average rapier step time: 123.208046
average bevy overhead: 0.22238159
total time: 12343.2841796875

ThierryBerger avatar Jun 27 '24 15:06 ThierryBerger

I'm reaching a point where I'm confident with the pull request, feedback is most welcome :)

ThierryBerger avatar Jul 04 '24 13:07 ThierryBerger

Could we add an easier way to change the fields of RapierContext during its initialization (for the single default-world approach)? Currently I have to write a system that at startup changes those fields, but that seems a bit clunky. Maybe provide the RapierContextInitialization enum with more options?

AnthonyTornetta avatar Jul 08 '24 22:07 AnthonyTornetta

interesting to note differences of system ordering compared to master (from https://github.com/dimforge/bevy_rapier/pull/576)

graphviz

ThierryBerger avatar Aug 12 '24 15:08 ThierryBerger

For now all systems share all these things, which I consider low priority to specialize for each RapierContext (them being components would be interesting but not trivial):

TimestepMode would be useful to make context-centric rather than global for the case where you have a physics-based camera movement and still need the camera to work when the game is paused as you would be able to pause your "camera physics world" and "game simulation world" independently .

You can kind of do this curently by setting physics_pipeline_active = false on the simulation world, but if you have a game where you need to modify the timescales (eg any game with "speed up time" buttons) -- there's no way to have it not also affect the camera world.

peterellisjones avatar Dec 16 '24 10:12 peterellisjones