Conversation
Co-authored-by: Tristan Guichaoua <33934311+tguichaoua@users.noreply.github.com>
Co-authored-by: Robert Walter <robwalter96@gmail.com>
crates/bevy_math/src/rotation2d.rs
Outdated
| /// ``` | ||
| #[inline] | ||
| pub fn lerp(self, end: Self, s: f32) -> Self { | ||
| Self::from_sin_cos(self.sin.lerp(end.sin, s), self.cos.lerp(end.cos, s)) |
There was a problem hiding this comment.
I'm about 99% sure this results in invalid rotations most of the time.
To make this work, one needs to normalise such that sin^2 + cos^2 = 1 (effectively meaning that the vector (sin, cos) has unit length).
To imagine this using complex numbers, every rotation is an unit complex number cos + i * sin and for it to be valid, it has to lie on the unit circle. Lerping moves us between two points on the unit circle along a line. However it is easy to see that this line passes through the inside of the circle, rather than along the surface.
This brings me to another point:
When we lerp two opposite (180° apart) transformations, the midpoint of the lerp is the point (0; 0) which I don't know how to map to any rotation (and normalisation of it would cause a division by 0).
There was a problem hiding this comment.
I added methods for normalizing and getting the length/norm of the complex number stored in Rotation2d, and changed this lerp to normalize the result.
For the case of opposite angles and s == 0.5, I made it just return the original angle for now (this is documented). I'm not sure if this is the best approach; we could also
- Just return NaN/infinity for the rotation
- Return the midpoint between the angles (on which side though?)
- Panic
- Something else?
NthTensor
left a comment
There was a problem hiding this comment.
Looks good to me now that lerp is fixed.
Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
|
Addressed the review suggestions. I also updated the 2D bounding volume transformations to use |
Objective
Rotating vectors is a very common task. It is required for a variety of things both within Bevy itself and in many third party plugins, for example all over physics and collision detection, and for things like Bevy's bounding volumes and several gizmo implementations.
For 3D, we can do this using a
Quat, but for 2D, we do not have a clear and efficient option.Mat2can be used for rotating vectors if created usingMat2::from_angle, but this is not obvious to many users, it doesn't have many rotation helpers, and the type does not give any guarantees that it represents a valid rotation.We should have a proper type for 2D rotations. In addition to allowing for potential optimization, it would allow us to have a consistent and explicitly documented representation used throughout the engine, i.e. counterclockwise and in radians.
Representation
The mathematical formula for rotating a 2D vector is the following:
Here,
sinandcosare the sine and cosine of the rotation angle. Computing these every time when a vector needs to be rotated can be expensive, so the rotation shouldn't be just anf32angle. Instead, it is often more efficient to represent the rotation using the sine and cosine of the angle instead of storing the angle itself. This can be freely passed around and reused without unnecessary computations.The two options are either a 2x2 rotation matrix or a unit complex number where the cosine is the real part and the sine is the imaginary part. These are equivalent for the most part, but the unit complex representation is a bit more memory efficient (two
f32s instead of four), so I chose that. This is like Nalgebra'sUnitComplextype, which can be used for theRotation2type.Implementation
Add a
Rotation2dtype represented as a unit complex number:Using it is similar to using
Quat, but in 2D:There's also a
From<f32>implementation forRotation2d, which means that methods can still accept radians as floats if the argument usesimpl Into<Rotation2d>. This means that addingRotation2dshouldn't even be a breaking change.Changelog
Rotation2dimpl Into<Rotation2d>impl Into<Rotation2d>Future use cases
Transformhelpers (e.g. return a 2D rotation about the Z axis from aTransform)Transform2d(AddTransform2dcomponent #8268)