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

MassProperties::from_compound produces NaN values if total mass is 0 #20

Closed
Demindiro opened this issue Mar 31, 2021 · 0 comments · Fixed by #24
Closed

MassProperties::from_compound produces NaN values if total mass is 0 #20

Demindiro opened this issue Mar 31, 2021 · 0 comments · Fixed by #24

Comments

@Demindiro
Copy link
Contributor

Demindiro commented Mar 31, 2021

Due to a division by 0 in Sum<MassProperties> it is possible to get NaN values.

total_com /= total_mass;

This can happen when e.g. trying to compute the MassProperties of a HeighField.

I believe the sum function should either panic or return as soon as it detects the total mass is 0.

Reproduction project:

// src/main.rs

use rapier3d::pipeline::*;
use rapier3d::dynamics::*;
use rapier3d::geometry::*;
use rapier3d::na::*;

fn main() {

	let mut pipeline = PhysicsPipeline::new();
	let gravity = Vector3::new(0.0, -9.81, 0.0);
	let integration_parameters = IntegrationParameters::default();
	let mut broad_phase = BroadPhase::new();
	let mut narrow_phase = NarrowPhase::new();
	let mut bodies = RigidBodySet::new();
	let mut colliders = ColliderSet::new();
	let mut joints = JointSet::new();
	let physics_hooks = ();
	let event_handler = ();

	let body = RigidBodyBuilder::new_dynamic().translation(0.0, 1.01, 0.0).build();
	let handle = bodies.insert(body);
	let collider = ColliderBuilder::ball(1.0).build();
	colliders.insert(collider, handle, &mut bodies);

	let shape = SharedShape::heightfield(DMatrix::zeros(10, 10), Vector3::new(9.0, 1.0, 9.0));
	let collider = ColliderBuilder::new(shape.clone()).build();
	let mut body = RigidBodyBuilder::new_static().build();
	let position = *body.position();
	let mp = MassProperties::from_compound(1.0, &vec![(position, shape)][..]);
	dbg!(collider.mass_properties()); // This is fine
	dbg!(mp); // This has NaN center of mass
	body.set_mass_properties(mp, true);
	let handle = bodies.insert(body);
	colliders.insert(collider, handle, &mut bodies);

	loop {
		pipeline.step(
			&gravity,
			&integration_parameters,
			&mut broad_phase,
			&mut narrow_phase,
			&mut bodies,
			&mut colliders,
			&mut joints,
			&physics_hooks,
			&event_handler,
		);
		for (_, b) in bodies.iter() {
			if b.is_dynamic() {
				// The position of the ball also becomes NaN due to this
				println!("{}", b.position());
				assert!(!b.position().translation.x.is_nan());
			}
		}
	}
}
# Cargo.toml

[package]
name = "rapier3d_heightfield_nan"
version = "0.1.0"
authors = ["David Hoppenbrouwers <david@salt-inc.org>"]
edition = "2018"

[dependencies]
rapier3d = "*"
Demindiro added a commit to Demindiro/godot_rapier3d that referenced this issue Mar 31, 2021
This commit is messy as I touched way too many things fixing what turned
out to be principal inertia.

* Added a heightmap test scene. It contains just a flat heightmap with a
  ball. This was mainly to debug a pesky NaN issue
  (dimforge/parry#20).
* Reduce the size of the "floor" in the single ray test due to
  performance issues with large colliders (see
  dimforge/rapier#153).
* Implement more methods of `PluggablePhysicsDirectBodyState`. It is
  still very unfinished but it works for wheels now.
* Implement `body_apply_impulse`, `body_set_param`.
* Calculate inertia properly. It is not very efficient and called in too
  many places right now, but it's fine for now.
* Remove many redundant `println`/`dbg`/... messages
* Fix update-cargo_branch not detecting `cargo-only` branch.

There is still a ton of stuff that is broken (especially heightmap &
concave colliders are wrong). This will be fixed soon.
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.

1 participant