From 6cb43bf3e157f32418f3a8cde1ad357a401a6328 Mon Sep 17 00:00:00 2001 From: Teddy Tennant Date: Wed, 8 Jul 2026 12:50:07 -0400 Subject: [PATCH] fix: clamp acos argument in Rotation3::euler_angles_ordered to avoid NaN Floating-point rounding in the chained matrix products computing `o_t` can push `o_t.m33` slightly outside `[-1, 1]` (e.g. -1.0000000000000002 for a near-nadir orientation), so `acos` returns NaN and silently poisons the whole returned angle array. Clamp the argument into the valid range before calling `acos`, mirroring the existing handling in `Matrix::angle`. Fixes #1481. --- CHANGELOG.md | 6 ++++++ src/geometry/rotation_specialization.rs | 2 +- tests/geometry/rotation.rs | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d64160d3..0e34868c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ This project adheres to [Semantic Versioning](https://semver.org/). **nalgebra-lapack change log** is found [here](https://github.com/dimforge/nalgebra/blob/main/nalgebra-lapack/CHANGELOG.md) starting with `nalgebra-lapack` version `0.27.0`. +## Unreleased + +### Fixed + +- Clamp the `acos` argument in `Rotation3::euler_angles_ordered` so that rounding in the intermediate matrix products can no longer push the middle angle to `NaN` [#1481](https://github.com/dimforge/nalgebra/issues/1481). + ## [0.35.0] (24 May 2026) ### Added diff --git a/src/geometry/rotation_specialization.rs b/src/geometry/rotation_specialization.rs index a2c5a5ad7..946794879 100644 --- a/src/geometry/rotation_specialization.rs +++ b/src/geometry/rotation_specialization.rs @@ -1098,7 +1098,7 @@ impl Rotation3 { c1, ); let o_t = c * self.matrix() * (c.transpose() * r1l); - angles[1] = o_t.m33.acos(); + angles[1] = o_t.m33.clamp(-T::one(), T::one()).acos(); let safe1 = angles[1].abs() >= eps; let safe2 = (angles[1] - T::pi()).abs() >= eps; diff --git a/tests/geometry/rotation.rs b/tests/geometry/rotation.rs index f461558aa..7c35353a9 100644 --- a/tests/geometry/rotation.rs +++ b/tests/geometry/rotation.rs @@ -85,6 +85,29 @@ fn quaternion_euler_angles_issue_494() { assert_eq!(angs.2, 0.0); } +#[test] +fn euler_angles_ordered_issue_1481() { + // A valid unit quaternion can yield a rotation matrix whose diagonal + // entry is slightly outside [-1, 1] due to rounding; feeding it to acos + // produced NaN before the clamp fix. + let q = UnitQuaternion::new_normalize(Quaternion::::new( + 0.1045170328727042, + -0.6993398242910853, + 0.10451703287270421, + 0.6993398242910854, + )); + let seq = [ + UnitVector3::new_normalize(Vector3::z()), + UnitVector3::new_normalize(Vector3::y()), + UnitVector3::new_normalize(Vector3::x()), + ]; + let (angles, _observable) = q.to_rotation_matrix().euler_angles_ordered(seq, false); + assert!( + angles.iter().all(|a| a.is_finite()), + "euler angles must be finite, got {angles:?}" + ); +} + #[cfg(feature = "proptest-support")] mod proptest_tests { use approx::AbsDiffEq;