Merge pull request #88 from EmbarkStudios/refactor-params

Small refactor of IntegrationParameters

10 files changed, 51 insertions, 47 deletions

diff --git a/src/dynamics/integration_parameters.rs b/src/dynamics/integration_parameters.rs
index b31c3f6..56b5801 100644
--- a/src/dynamics/integration_parameters.rs
+++ b/src/dynamics/integration_parameters.rs
@@ -3,9 +3,8 @@
 #[cfg_attr(feature = "serde-serialize", derive(Serialize, Deserialize))]
 pub struct IntegrationParameters {
     /// The timestep length (default: `1.0 / 60.0`)
-    dt: f32,
-    /// The inverse of `dt`.
-    inv_dt: f32,
+    pub dt: f32,
+
     //    /// If `true` and if rapier is compiled with the `parallel` feature, this will enable rayon-based multithreading (default: `true`).
     //    ///
     //    /// This parameter is ignored if rapier is not compiled with is `parallel` feature.
@@ -29,7 +28,7 @@ pub struct IntegrationParameters {
     /// Contacts at points where the involved bodies have a relative
     /// velocity smaller than this threshold wont be affected by the restitution force (default: `1.0`).
     pub restitution_velocity_threshold: f32,
-    /// Amount of penetration the engine wont attempt to correct (default: `0.001m`).
+    /// Amount of penetration the engine wont attempt to correct (default: `0.005m`).
     pub allowed_linear_error: f32,
     /// The maximal distance separating two objects that will generate predictive contacts (default: `0.002`).
     pub prediction_distance: f32,
@@ -87,6 +86,7 @@ pub struct IntegrationParameters {
 
 impl IntegrationParameters {
     /// Creates a set of integration parameters with the given values.
+    #[deprecated = "Use `IntegrationParameters { dt: 60.0, ..Default::default() }` instead"]
     pub fn new(
         dt: f32,
         //        multithreading_enabled: bool,
@@ -110,7 +110,6 @@ impl IntegrationParameters {
     ) -> Self {
         IntegrationParameters {
             dt,
-            inv_dt: if dt == 0.0 { 0.0 } else { 1.0 / dt },
             //            multithreading_enabled,
             erp,
             joint_erp,
@@ -140,30 +139,29 @@ impl IntegrationParameters {
 
     /// The current time-stepping length.
     #[inline(always)]
+    #[deprecated = "You can just read the `IntegrationParams::dt` value directly"]
     pub fn dt(&self) -> f32 {
         self.dt
     }
 
-    /// The inverse of the time-stepping length.
+    /// The inverse of the time-stepping length, i.e. the steps per seconds (Hz).
     ///
     /// This is zero if `self.dt` is zero.
     #[inline(always)]
     pub fn inv_dt(&self) -> f32 {
-        self.inv_dt
+        if self.dt == 0.0 {
+            0.0
+        } else {
+            1.0 / self.dt
+        }
     }
 
     /// Sets the time-stepping length.
-    ///
-    /// This automatically recompute `self.inv_dt`.
     #[inline]
+    #[deprecated = "You can just set the `IntegrationParams::dt` value directly"]
     pub fn set_dt(&mut self, dt: f32) {
         assert!(dt >= 0.0, "The time-stepping length cannot be negative.");
         self.dt = dt;
-        if dt == 0.0 {
-            self.inv_dt = 0.0
-        } else {
-            self.inv_dt = 1.0 / dt
-        }
     }
 
     /// Sets the inverse time-stepping length (i.e. the frequency).
@@ -171,7 +169,6 @@ impl IntegrationParameters {
     /// This automatically recompute `self.dt`.
     #[inline]
     pub fn set_inv_dt(&mut self, inv_dt: f32) {
-        self.inv_dt = inv_dt;
         if inv_dt == 0.0 {
             self.dt = 0.0
         } else {
@@ -182,26 +179,32 @@ impl IntegrationParameters {
 
 impl Default for IntegrationParameters {
     fn default() -> Self {
-        Self::new(
-            1.0 / 60.0,
-            //            true,
-            0.2,
-            0.2,
-            1.0,
-            1.0,
-            0.005,
-            0.001,
-            0.2,
-            0.2,
-            0.002,
-            0.2,
-            4,
-            1,
-            10,
-            1,
-            false,
-            false,
-            false,
-        )
+        Self {
+            dt: 1.0 / 60.0,
+            //        multithreading_enabled:             true,
+            return_after_ccd_substep: false,
+            erp: 0.2,
+            joint_erp: 0.2,
+            warmstart_coeff: 1.0,
+            restitution_velocity_threshold: 1.0,
+            allowed_linear_error: 0.005,
+            prediction_distance: 0.002,
+            allowed_angular_error: 0.001,
+            max_linear_correction: 0.2,
+            max_angular_correction: 0.2,
+            max_stabilization_multiplier: 0.2,
+            max_velocity_iterations: 4,
+            max_position_iterations: 1,
+            // FIXME: what is the optimal value for min_island_size?
+            // It should not be too big so that we don't end up with
+            // huge islands that don't fit in cache.
+            // However we don't want it to be too small and end up with
+            // tons of islands, reducing SIMD parallelism opportunities.
+            min_island_size: 128,
+            max_ccd_position_iterations: 10,
+            max_ccd_substeps: 1,
+            multiple_ccd_substep_sensor_events_enabled: false,
+            ccd_on_penetration_enabled: false,
+        }
     }
 }
diff --git a/src/dynamics/solver/island_solver.rs b/src/dynamics/solver/island_solver.rs
index 7ce142a..b548c6b 100644
--- a/src/dynamics/solver/island_solver.rs
+++ b/src/dynamics/solver/island_solver.rs
@@ -59,8 +59,7 @@ impl IslandSolver {
         }
 
         counters.solver.velocity_update_time.resume();
-        bodies
-            .foreach_active_island_body_mut_internal(island_id, |_, rb| rb.integrate(params.dt()));
+        bodies.foreach_active_island_body_mut_internal(island_id, |_, rb| rb.integrate(params.dt));
         counters.solver.velocity_update_time.pause();
 
         if manifold_indices.len() != 0 || joint_indices.len() != 0 {
diff --git a/src/dynamics/solver/parallel_island_solver.rs b/src/dynamics/solver/parallel_island_solver.rs
index 3b7ab9f..1393067 100644
--- a/src/dynamics/solver/parallel_island_solver.rs
+++ b/src/dynamics/solver/parallel_island_solver.rs
@@ -234,7 +234,7 @@ impl ParallelIslandSolver {
                         let dvel = mj_lambdas[rb.active_set_offset];
                         rb.linvel += dvel.linear;
                         rb.angvel += rb.world_inv_inertia_sqrt.transform_vector(dvel.angular);
-                        rb.integrate(params.dt());
+                        rb.integrate(params.dt);
                         positions[rb.active_set_offset] = rb.position;
                     }
                 }
diff --git a/src/dynamics/solver/velocity_constraint.rs b/src/dynamics/solver/velocity_constraint.rs
index 948081d..f9e38ac 100644
--- a/src/dynamics/solver/velocity_constraint.rs
+++ b/src/dynamics/solver/velocity_constraint.rs
@@ -144,6 +144,7 @@ impl VelocityConstraint {
         out_constraints: &mut Vec<AnyVelocityConstraint>,
         push: bool,
     ) {
+        let inv_dt = params.inv_dt();
         let rb1 = &bodies[manifold.body_pair.body1];
         let rb2 = &bodies[manifold.body_pair.body2];
         let mj_lambda1 = rb1.active_set_offset;
@@ -244,7 +245,7 @@ impl VelocityConstraint {
                         rhs += manifold.restitution * rhs
                     }
 
-                    rhs += manifold_point.dist.max(0.0) * params.inv_dt();
+                    rhs += manifold_point.dist.max(0.0) * inv_dt;
 
                     let impulse = manifold_points[k].impulse * warmstart_coeff;
 
diff --git a/src/dynamics/solver/velocity_ground_constraint.rs b/src/dynamics/solver/velocity_ground_constraint.rs
index d8ef8be..23b41c1 100644
--- a/src/dynamics/solver/velocity_ground_constraint.rs
+++ b/src/dynamics/solver/velocity_ground_constraint.rs
@@ -63,6 +63,7 @@ impl VelocityGroundConstraint {
         out_constraints: &mut Vec<AnyVelocityConstraint>,
         push: bool,
     ) {
+        let inv_dt = params.inv_dt();
         let mut rb1 = &bodies[manifold.body_pair.body1];
         let mut rb2 = &bodies[manifold.body_pair.body2];
         let flipped = !rb2.is_dynamic();
@@ -176,7 +177,7 @@ impl VelocityGroundConstraint {
                         rhs += manifold.restitution * rhs
                     }
 
-                    rhs += manifold_point.dist.max(0.0) * params.inv_dt();
+                    rhs += manifold_point.dist.max(0.0) * inv_dt;
 
                     let impulse = manifold_points[k].impulse * warmstart_coeff;
 
diff --git a/src/pipeline/physics_pipeline.rs b/src/pipeline/physics_pipeline.rs
index 996b28a..1c8666f 100644
--- a/src/pipeline/physics_pipeline.rs
+++ b/src/pipeline/physics_pipeline.rs
@@ -160,7 +160,7 @@ impl PhysicsPipeline {
         self.counters.stages.update_time.start();
         bodies.foreach_active_dynamic_body_mut_internal(|_, b| {
             b.update_world_mass_properties();
-            b.integrate_accelerations(integration_parameters.dt(), *gravity)
+            b.integrate_accelerations(integration_parameters.dt, *gravity)
         });
         self.counters.stages.update_time.pause();
 
@@ -239,7 +239,7 @@ impl PhysicsPipeline {
                 rb.linvel = na::zero();
                 rb.angvel = na::zero();
             } else {
-                rb.update_predicted_position(integration_parameters.dt());
+                rb.update_predicted_position(integration_parameters.dt);
             }
 
             rb.update_colliders_positions(colliders);
diff --git a/src_testbed/box2d_backend.rs b/src_testbed/box2d_backend.rs
index f448a6f..941c6d5 100644
--- a/src_testbed/box2d_backend.rs
+++ b/src_testbed/box2d_backend.rs
@@ -211,7 +211,7 @@ impl Box2dWorld {
 
         counters.step_started();
         self.world.step(
-            params.dt(),
+            params.dt,
             params.max_velocity_iterations as i32,
             params.max_position_iterations as i32,
         );
diff --git a/src_testbed/harness/mod.rs b/src_testbed/harness/mod.rs
index 51bf315..2251038 100644
--- a/src_testbed/harness/mod.rs
+++ b/src_testbed/harness/mod.rs
@@ -238,7 +238,7 @@ impl Harness {
 
         self.events.poll_all();
 
-        self.state.time += self.physics.integration_parameters.dt();
+        self.state.time += self.physics.integration_parameters.dt;
         self.state.timestep_id += 1;
     }
 
diff --git a/src_testbed/nphysics_backend.rs b/src_testbed/nphysics_backend.rs
index e112c22..d988904 100644
--- a/src_testbed/nphysics_backend.rs
+++ b/src_testbed/nphysics_backend.rs
@@ -143,7 +143,7 @@ impl NPhysicsWorld {
             .max_velocity_iterations = params.max_velocity_iterations;
         self.mechanical_world
             .integration_parameters
-            .set_dt(params.dt());
+            .set_dt(params.dt);
 
         counters.step_started();
         self.mechanical_world.step(
diff --git a/src_testbed/physx_backend.rs b/src_testbed/physx_backend.rs
index acec5c9..6ea207a 100644
--- a/src_testbed/physx_backend.rs
+++ b/src_testbed/physx_backend.rs
@@ -396,7 +396,7 @@ impl PhysxWorld {
 
     pub fn step(&mut self, counters: &mut Counters, params: &IntegrationParameters) {
         counters.step_started();
-        self.scene.step(params.dt(), true);
+        self.scene.step(params.dt, true);
         counters.step_completed();
     }