Skip to content

Commit

Permalink
Warnings and docs for exponential denormalization in rotate functions…
Browse files Browse the repository at this point in the history
… (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.
  • Loading branch information
Novakasa authored Feb 3, 2025
1 parent 2d66099 commit bdf60d6
Showing 1 changed file with 49 additions and 0 deletions.
49 changes: 49 additions & 0 deletions crates/bevy_transform/src/components/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,29 @@ use bevy_ecs::{component::Component, prelude::require};
#[cfg(feature = "bevy_reflect")]
use {bevy_ecs::reflect::ReflectComponent, bevy_reflect::prelude::*};

/// Checks that a vector with the given squared length is normalized.
///
/// Warns for small error with a length threshold of approximately `1e-4`,
/// and panics for large error with a length threshold of approximately `1e-2`.
#[cfg(debug_assertions)]
fn assert_is_normalized(message: &str, length_squared: f32) {
use bevy_math::ops;
#[cfg(feature = "std")]
use std::eprintln;

let length_error_squared = ops::abs(length_squared - 1.0);

// Panic for large error and warn for slight error.
if length_error_squared > 2e-2 || length_error_squared.is_nan() {
// Length error is approximately 1e-2 or more.
panic!("Error: {message}",);
} else if length_error_squared > 2e-4 {
// Length error is approximately 1e-4 or more.
#[cfg(feature = "std")]
eprintln!("Warning: {message}",);
}
}

/// Describe the position of an entity. If the entity has a parent, the position is relative
/// to its parent position.
///
Expand Down Expand Up @@ -312,8 +335,21 @@ impl Transform {
/// Rotates this [`Transform`] around the given `axis` by `angle` (in radians).
///
/// If this [`Transform`] has a parent, the `axis` is relative to the rotation of the parent.
///
/// # Warning
///
/// If you pass in an `axis` based on the current rotation (e.g. obtained via [`Transform::local_x`]),
/// floating point errors can accumulate exponentially when applying rotations repeatedly this way. This will
/// result in a denormalized rotation. In this case, it is recommended to normalize the [`Transform::rotation`] after
/// each call to this method.
#[inline]
pub fn rotate_axis(&mut self, axis: Dir3, angle: f32) {
#[cfg(debug_assertions)]
assert_is_normalized(
"The axis given to `Transform::rotate_axis` is not normalized. This may be a result of obtaining \
the axis from the transform. See the documentation of `Transform::rotate_axis` for more details.",
axis.length_squared(),
);
self.rotate(Quat::from_axis_angle(axis.into(), angle));
}

Expand Down Expand Up @@ -350,8 +386,21 @@ impl Transform {
}

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

Expand Down

0 comments on commit bdf60d6

Please sign in to comment.