Skip to content

Commit bdf60d6

Browse files
authored
Warnings and docs for exponential denormalization in rotate functions (alternative to #17604) (#17646)
# Objective - When obtaining an axis from the transform and putting that into `Transform::rotate_axis` or `Transform::rotate_axis_local`, floating point errors could accumulate exponentially, resulting in denormalized rotation. - This is an alternative to and closes #17604, due to lack of consent around this in the [discord discussion](https://discord.com/channels/691052431525675048/1203087353850364004/1334232710658392227) - Closes #16480 ## Solution - Add a warning of this issue and a recommendation to normalize to the API docs. - Add a runtime warning that checks for denormalized axis in debug mode, with a reference to the API docs.
1 parent 2d66099 commit bdf60d6

File tree

1 file changed

+49
-0
lines changed

1 file changed

+49
-0
lines changed

crates/bevy_transform/src/components/transform.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,29 @@ use bevy_ecs::{component::Component, prelude::require};
88
#[cfg(feature = "bevy_reflect")]
99
use {bevy_ecs::reflect::ReflectComponent, bevy_reflect::prelude::*};
1010

11+
/// Checks that a vector with the given squared length is normalized.
12+
///
13+
/// Warns for small error with a length threshold of approximately `1e-4`,
14+
/// and panics for large error with a length threshold of approximately `1e-2`.
15+
#[cfg(debug_assertions)]
16+
fn assert_is_normalized(message: &str, length_squared: f32) {
17+
use bevy_math::ops;
18+
#[cfg(feature = "std")]
19+
use std::eprintln;
20+
21+
let length_error_squared = ops::abs(length_squared - 1.0);
22+
23+
// Panic for large error and warn for slight error.
24+
if length_error_squared > 2e-2 || length_error_squared.is_nan() {
25+
// Length error is approximately 1e-2 or more.
26+
panic!("Error: {message}",);
27+
} else if length_error_squared > 2e-4 {
28+
// Length error is approximately 1e-4 or more.
29+
#[cfg(feature = "std")]
30+
eprintln!("Warning: {message}",);
31+
}
32+
}
33+
1134
/// Describe the position of an entity. If the entity has a parent, the position is relative
1235
/// to its parent position.
1336
///
@@ -312,8 +335,21 @@ impl Transform {
312335
/// Rotates this [`Transform`] around the given `axis` by `angle` (in radians).
313336
///
314337
/// If this [`Transform`] has a parent, the `axis` is relative to the rotation of the parent.
338+
///
339+
/// # Warning
340+
///
341+
/// If you pass in an `axis` based on the current rotation (e.g. obtained via [`Transform::local_x`]),
342+
/// floating point errors can accumulate exponentially when applying rotations repeatedly this way. This will
343+
/// result in a denormalized rotation. In this case, it is recommended to normalize the [`Transform::rotation`] after
344+
/// each call to this method.
315345
#[inline]
316346
pub fn rotate_axis(&mut self, axis: Dir3, angle: f32) {
347+
#[cfg(debug_assertions)]
348+
assert_is_normalized(
349+
"The axis given to `Transform::rotate_axis` is not normalized. This may be a result of obtaining \
350+
the axis from the transform. See the documentation of `Transform::rotate_axis` for more details.",
351+
axis.length_squared(),
352+
);
317353
self.rotate(Quat::from_axis_angle(axis.into(), angle));
318354
}
319355

@@ -350,8 +386,21 @@ impl Transform {
350386
}
351387

352388
/// Rotates this [`Transform`] around its local `axis` by `angle` (in radians).
389+
///
390+
/// # Warning
391+
///
392+
/// If you pass in an `axis` based on the current rotation (e.g. obtained via [`Transform::local_x`]),
393+
/// floating point errors can accumulate exponentially when applying rotations repeatedly this way. This will
394+
/// result in a denormalized rotation. In this case, it is recommended to normalize the [`Transform::rotation`] after
395+
/// each call to this method.
353396
#[inline]
354397
pub fn rotate_local_axis(&mut self, axis: Dir3, angle: f32) {
398+
#[cfg(debug_assertions)]
399+
assert_is_normalized(
400+
"The axis given to `Transform::rotate_axis_local` is not normalized. This may be a result of obtaining \
401+
the axis from the transform. See the documentation of `Transform::rotate_axis_local` for more details.",
402+
axis.length_squared(),
403+
);
355404
self.rotate_local(Quat::from_axis_angle(axis.into(), angle));
356405
}
357406

0 commit comments

Comments
 (0)