rotations not normalized in solver, causing colliders to expand and eventually explode #235
Labels
A-Dynamics
Relates to rigid body dynamics: motion, mass, constraint solving, joints, CCD, and so on
C-Bug
Something isn't working
Comments
Jondolf
added
C-Bug
mattdm
added a commit
to mattdm/bevy_xpbd
that referenced
this issue
Nov 10, 2023
Adding *body1.rotation.0 = *body1.rotation.0.normalize(); after rotational updates in position_constraints.rs fixes Jondolf#235. (Doesn't seem to be necessarily in angular_constraints.rs, but I think that's because it happens that the constraints are applied with positional_constraints running after in all existing joints which use both — so that should also be done just in case.) I think rather than sticking the normalization after every rotation, it could be done by refactoring get_delta_rot() — this would mean less code duplication, but probably also it getting called a few more times than necessary. Maybe it's actually better to put it up into the solver step somewhere, so it only gets called once. If joints end up getting unified into one 6dof universal joint, that might change this entirely. So, I'm going to submit a PR for quick-and-easy fix now, but am also filing this so we can later look at putting the normalization at the most optimal level.
|
Also, I want to put this here for the record, because it kinda makes me laugh every time I watch it. Screencast.from.2023-11-08.17-30-13.webm |
Jondolf
added a commit
that referenced
this issue
Mar 2, 2024
# Objective Adopted from #237, fixes #235. Constraints update rotation by adding or subtracting quaternions. Unlike multiplying unit quaternions (the typical way to "add" together rotations), this can result in rotations becoming unnormalized. This can cause explosions (see #235), and cause panics when trying to rotate Bevy's direction types. ## Solution Normalize the rotations after the constraint rotation delta has been applied. This does come with a small extra cost, but I believe it is worth it for stability. The normalization is also present in some other XPBD implementations I've found, like [this one](https://github.com/felipeek/raw-physics/blob/96fbc46eb8f89d007967c219d2c22e74d7375fd1/src/physics/pbd_base_constraints.cpp#L111).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Adding
*body1.rotation.0 = *body1.rotation.0.normalize();after rotational updates inposition_constraints.rsfixes this. (Doesn't seem to be necessarily in angular_constraints.rs, but I think that's because it happens that the constraints are applied with positional_constraints running after in all existing joints which use both — so that should also be done just in case.)I think rather than sticking the normalization after every rotation, it could be done by refactoring
get_delta_rot()— this would mean less code duplication, but probably also it getting called a few more times than necessary. Maybe it's actually better to put it up into the solver step somewhere, so it only gets called once.If joints end up getting unified into one 6dof universal joint, that might change this entirely. So, I'm going to submit a PR for quick-and-easy fix now, but am also filing this so we can later look at putting the normalization at the most optimal level.
The text was updated successfully, but these errors were encountered: