Add Radians and Degrees types and Angle trait

[Jondolf]

Objective

Fixes #11132.

Angles are a core part of math and come up quite frequently. However, they come with several ambiguities. Consider the following:

transform.rotate_x(1.4);

Is the angle in degrees or radians? While experienced Bevy users might know that it's in radians and counterclockwise, it's not immediately clear from the actual code and requires checking the documentation.

The code above is just a single example, but the issue is quite common. To improve readability and ergonomics, it would be nice to have a consistent and expressive way of representing angles and/or 2D rotations.

See #11132 for more background and discussion.

Solution

Add Radians and Degrees types and a shared Angle trait, similar to the approach proposed by @aleqdev here.

Creating and converting angles

Creating angles is very simple:

let alpha = Radians(std::f32::consts::PI); // or Radians::PI directly
let beta = Degrees(180.0); // or Degrees::HALF

The unit can be converted in a few ways:

// These are equivalent
let alpha_2 = alpha.to_degrees();
let alpha_2 = Degrees::from(alpha);

// These are equivalent
let beta_2 = beta.to_radians();
let beta_2 = Radians::from(beta);

Angles can be considered equal even if the unit is different:

assert_eq!(Radians::PI, Degrees::HALF);

Of course, numerical issues could result in the values being slightly different, but that seems like an unavoidable issue unless we use some epsilon value in the comparison.

Angle math

Add and Sub work as expected:

assert_eq!(Radians::PI + Radians::TAU, 3.0 * Radians::PI);
assert_eq!(Degrees(30.0) + Degrees(45.0), Degrees(75.0));

However, you can't add or subtract an f32 directly, as that can be considered to be an implicit conversion.

If one angle is in Radians and the other is in Degrees, the left-hand operand is always returned as the output:

let alpha: Radians = Radians::PI + Degrees::HALF;
let beta: Degrees = Degrees::HALF + Radians::PI;

For now, there are special methods that allow type inference to work:

// This should return Degrees correctly
let alpha: Degrees = Radians::PI.sub_angle(Degrees(45.0));
assert_eq!(alpha, Degrees(45.0));

Mul<Self> is not implemented, because multiplying two angles doesn't make sense mathematically. You can multiply or divide by f32 though:

assert_eq!(Degrees(30.0) * 2.0, Degrees(60.0));
assert_eq!(Degrees(30.0) / 2.0, Degrees(15.0));

Div<Self> and Rem<Self> return an f32, because the angles cancel out and represent a fraction/remainder:

assert_eq!(Radians::TAU / Radians::PI, 2.0);
assert_eq!(Radians::TAU % Radians::PI, 0.0);

And of course, the angles have common math operations like sin, cos, tan, abs, min, and so on implemented.

Note that Radians and Degrees do not implement Deref or DerefMut, because that would have the footgun of methods like to_radians not working how you'd expect, and Degrees needs to first be converted to Radians for e.g. trigonometric operations.

Using angles in Bevy's APIs

One of the most prevalent places where Angle is now used is the rotation methods in Transform. They take an impl Into<Radians> so you can pass any angle type:

// Both of these work
transform.rotate_x(Radians::FRAC_PI_4);
transform.rotate_x(Degrees(45.0);

For now, I decided not to implement From<f32> for the angle types. This might be one of the more controversial parts of this PR, as some people prefer to be able to write just the float value like before. Personally, I'm a big fan of the explicitness of having to use an angle type, and the API is concise and ergonomic enough that I don't find the few extra characters to be a big deal. I'm open to changing this if that's the general consensus though.

The Angle trait also makes it possible to make methods accept any angle type using generics without having to convert to a specific type. This might be useful for logic that is unit-agnostic and just wants to e.g. get the sine of the angle.

In the future, I'd expect these angles to also be used for e.g. Transform2d rotation (and skew?), 2D rotations provided when constructing bounding volumes (originally inspired this PR), geometric methods for some primitive shapes, 2D angular velocity, and many other use cases as well as user code.

Open questions

• Should we implement From<f32> for Radians and Degrees and allow implicit angles to be passed to functions as floats?
• Which one is better, Radians and Degrees or Rad and Deg?
• Do we like the approach shown here, or would the third option shown in Angle type #11132 be better?
• How do we implement Radians + Degrees return type?

Todo

• Add proper tests for all common tasks and operations
• Implement missing methods (sinh, sqrt, etc.)

In the future, we could also add an f64 version of Angle or make it generic. I didn't do this yet as most angles use f32, and generics would require a generic number trait, which adds some complexity.

---

Changelog

• Added Radians and Degrees types and Angle trait
• Changed Transform methods with f32 rotations to take an impl Into<Radians>
• Changed Transform method docs to mention that the rotation is in the counterclockwise direction
• Changed examples to use Radians for properties or arguments that represent angles or 2D angular velocity

Migration Guide

Wrap angles passed to Transform methods like rotate_x inside the Radians type:

// Before
transform.rotate_x(angle);

// After
transform.rotate_x(Radians(angle));

Alternatively, use Degrees if you expect the angle to use degrees.

It's also recommended to use Radians and Degrees for angle types in your own code, but this is optional.

[Jondolf]

For now, I needed to skip deriving Reflect for the angle types because bevy_math doesn't have bevy_reflect and it can't be properly done externally because the impl_reflect_struct! macro doesn't support tuple structs just yet.

Because of this, spotlights still use f32 angles at the moment. The alternative would've been to remove the Reflect impl from the spotlight.

[RobWalt]

From a first rough review I would say it looks solid 👌

Just some thoughts:

• would it make sense to add as_radians and as_degrees to the trait? This way the user of the trait can convert to a specific type if that's useful and they don't have to deal with T: Angle
• I have some concerns about the operators:
  • AddAssign/SubAssign of an angle type in combination with f32 feels like an implicit conversion, I'd vote to scratch that
  • Mul of angle type with another angle type feels weird if you treat degrees as a unit (what's the unit of the result? square degrees?), I would vote for removing that and just keeping the Mul of angle * f32 and vice versa
  • Div of angle type with another angle type feels like it should return a f32. Again thinking of degrees as units and we would scratch them with the division. It reads "how many times does this rotation fit into the other one". Div of angle with f32 is fine and reads "how big is the rotation that is a n-th of this other rotation"
  • similar reasoning would apply to both Rem impls
• I'm not sure why the operator methods in the trait exist, did you face any problems with the regular operators?

[djeedai]

Added a Controversial label. The issue is under discussion, and not sure there's a clear consensus. Nice to see code to support the argumentation thought, thanks!

[Jondolf]

@RobWalt

would it make sense to add as_radians and as_degrees to the trait?

Yeah, I think that would make sense, it'd make the trait more useful. f32 has to_radians and to_degrees, so that naming could also make sense (what Radians and Degrees currently have), but e.g. Radians::to_radians wouldn't actually convert anything. So perhaps as_ would be better since "radians as radians" might make a bit more sense than "radians to radians".

AddAssign/SubAssign of an angle type in combination with f32 feels like an implicit conversion

I'm kinda on the fence here, but I think I might agree. It just means that we should also remove Add<f32> and Sub<f32> since that's basically the same kind of implicit convertion.

Mul of angle type with another angle type feels weird if you treat degrees as a unit...
Div of angle type with another angle type feels like it should return a f32...

This is something I was wondering too. If we consider units, you're definitely correct. I was thinking that it'd be nice to have angle * angle work for usability reasons, but it's probably just confusing, and you could always do angle * angle.0 instead.

I think I'll change it to be like you suggested since that's more "correct".

I'm not sure why the operator methods in the trait exist, did you face any problems with the regular operators?

Do you mean why methods like add_angle exist instead of just doing it in Add?

So the goal is that we can do Degrees(90.0) + Radians(PI) and have it return either Degrees or Radians based on what type is inferred from the surroundings.

I might be dumb and missing an obvious solution, but Add for a given type pair X + Y can only have a single Output, like this:

impl Add<Radians> for Degrees {
    type Output = Radians;

    fn add(self, rhs: Radians) -> Self::Output {
        self.to_radians() + rhs
    }
}

You can't implement this a second time with Output = Degrees, because then there would be duplicate implementations of Add<Radians>. And you also can't do this:

impl<T: Angle> Add<Radians> for Degrees {
    type Output = T;

    fn add(self, rhs: Radians) -> Self::Output {
        Self::Output::from(self) + Self::Output::from(rhs)
    }
}

because T is "not constrained by the impl trait, self type, or predicates". However, a separate method works, because there it is constrained:

/// Adds the given `angle` to `self`, returning the inferred type of angle.
fn add_angle<T: Angle, Output: Angle>(self, angle: T) -> Output
where
    Output: From<Self> + From<T>,
{
    Output::from(self) + Output::from(angle)
}

[djeedai]

Thanks for the change. But I'm personally less and less convinced we should add this abstraction, at least in this form. There's just too much eyebrow raising.

• operator overloading are unconvincing, seems to cause more confusion than help
• implicit use of f32 as radian in operators seem to defeat the purpose of the change
• no clear example where some public API is improved; instead PR shows an internal change which doesn't benefit at all from the abstraction
• quite a few bug-looking changes for a PR touching fundamental pieces like Transform
• unclear performance impact for touching same pieces

I think it's worth discussing further on the issue to find a consensus before continuing on an implementation.

[djeedai]

This is highly opinionated I think. I don't feel adding degrees and radians has more an expectation to yield radians than degrees. I think the existence of this not only adds confusion to code (you need to know the result is radian when reading the code to understand it) but also will be annoying to users who want degrees as result.

let r = Radians(0);
let x = r + Degrees(0); // doesn't build
let y = Degrees(0) + r; // Radians

This also shows addition is not symmetric! This is very confusing for types that are supposed to represent the same thing (an angle). And if you consider they're different types, this goes against the common convention of, say, Instant + Duration, which yields the type of the left hand side operand, not the right hand side one.

[Jondolf]

This does compile and pass though. It is symmetric, unless I'm missing something.

let r = Radians(PI);
let x = r + Degrees(180.0);
let y = Degrees(180.0) + r;
assert_eq!(x, y);
assert_eq!(x, Radians::TAU);

I agree that it is highly opinionated to yield Radians, and I don't like it. I just haven't found a way to make type inference work properly without the add_angle/sub_angle methods. I would like to fix this if it's possible, but I'm not sure if it is.

Even with this, though, users can do math on angles without having to care much about what the angle type is in intermediary computations. The result should be basically the same whether you add Degrees or Radians to an angle because the operator overloads handle the necessary conversion for you. The type only matters if you access the internal value and use that. And if you want the end result to be Degrees, you can just convert once at the end (usually).

I think we could just yield the type of the left hand side operand like you're suggesting though. This is what several angle crates like ang and angular_units seem to do.

[pnarimani]

I think the same way that an integer cannot be added to a float in rust, radians also should not be addable to degrees. We should force conversion to the same unit before any operation.

[Jondolf]

Yeah, that'd probably be another reasonable way to handle this using the approach in this PR.

I suppose this is one of the main problems with having separate Degrees and Radians types. If we had a singular Angle type like my initial proposal's option 3, this wouldn't be an issue, as it'd always have the same internal representation, and radians/degrees would just be helpers for constructing and retrieving a specific type of angle. Of course this does come with the downside of assuming a specific angle unit for the internal representation.

[djeedai]

Can't this be impl Add<T: Angle> for dyn Angle? Or is it one of those weird cases in Rust where there's an ambiguity and that's not possible? Because that makes Angle much less easy to work with.

[Jondolf]

That causes a cycle:

cycle detected when computing type of angle::<impl at crates\bevy_math\src\angle\mod.rs:149:1: 149:33>
...which again requires computing type of angle::<impl at crates\bevy_math\src\angle\mod.rs:149:1: 149:33>, completing the cycle

[Jondolf]

Okay, I've made the following changes based on the feedback:

• The Angle trait requires that you can create Radians or Degrees from it using From, which should make separate as_radians and as_degrees methods redundant
• All implicit conversions should be gone; you can't do Degrees(90.0) + 45.0
• Degrees(90.0) * Degrees(90.0) isn't possible, because it doesn't make sense mathematically. The other value must be an f32.
  • Some/Most angle crates I've seen just return Degrees though
• Degrees(90.0) / Degrees(45.0) returns an f32 (same for Rem)
• Adding or subtracting angles always returns the left hand side operand instead of always returning Radians
  • This is unopinionated and more flexible
  • This is a relatively common Rust convention, e.g. Instant + Duration returns Instant like djeedai mentioned
  • This is what several angle crates like ang and angular_units seem to do
• Removed add_angle and sub_angle in favor of the normal Add and Sub like described above

The PR is still definitely controversial and I'm not entirely sure about all of the details myself, but I think it does look more reasonable now.

As for this:

no clear example where some public API is improved; instead PR shows an internal change which doesn't benefit at all from the abstraction

I agree that my examples might not have been particularly convincing, and I think this is also quite subjective.

In my eyes, the main benefit comes from the explicitness. You know that a value is an angle (of a specific unit) without having to look at documentation or surrounding context. "Code is read more than it's written."

You could argue that radians are the unit used everywhere internally, but to know that, you need to know the codebase already. If you don't know that, you need to always double check.

A few examples where angle types make things more explicit and/or ergonomic.

With methods like gizmos.arc_2d, there are just three floats in a row. You can't tell what they represent without looking at the function signature. If the method used angle types, you'd at least know that two of them are angles.

// Before
gizmos.arc_2d(
    Vec2::ZERO,
    0.0,
    std::f32::consts::PI / 4.0,
    1.0,
    Color::GREEN,
);

// After
gizmos.arc_2d(
    Vec2::ZERO,
    Radians::ZERO,
    Radians::PI / 4.0,
    1.0,
    Color::GREEN,
);

Of course, it's still not perfectly clear what each value does if you haven't seen the method before, but it is more explicit.

I have a PR to add more math helpers for primitive shapes (#10632), and it has some methods that return angles. It would be really nice if they could have units.

Currently, they (e.g. this) can use separate _degrees and _radians variants, but if there were units, we could either have just one variant (with a specific unit return type), keep them as they are but specify the return type as an angle, or even unify them using the Angle trait:

// Before
pub fn internal_angle_degrees(&self) -> f32 {
    (self.sides - 2) as f32 * 180.0 / self.sides as f32
}
pub fn internal_angle_radians(&self) -> f32 {
    (self.sides - 2) as f32 * PI / self.sides as f32
}

// After
pub fn internal_angle<T: Angle>(&self) -> T {
    (self.sides - 2) as f32 * Angle::HALF / self.sides as f32
}

// or just keep the separate functions, but return Degrees/Radians

Another ergonomics improvement is that sometimes degrees are just easier to work with. I myself used to struggle getting Transform rotations correct before learning how radians work in high school (not everyone even goes to high school), and it still isn't as intuitive as degrees.

With separate angle types, you also don't need to import PI everywhere. An example brought up on Discord, slightly modified:

// Before
transform.rotate_x(std::f32::consts::PI / 2.0);

// After
transform.rotate_x(Degrees(90.0));

With the old approach, you need to (1) know that the method takes radians, and (2) think of what PI / 2 looks like on a unit circle. Unless you have a strong background in mathematics, I'd argue that just 90 degrees is much more intuitive and quick to understand. You don't need to worry about the unit taken by rotate_x, or import anything (other than the Bevy prelude).

Of course, you could just do 90.0.to_radians() too, but I find it to still be a bit less clear, and you still need to know that the expected value is radians.

[Jondolf]

Closing for now, see #11132 (comment). This can be reopened (or adopted) if there's more consensus that we should move forward with this or some similar approach, but at the moment it seems too controversial and perhaps unnecessary, and I myself am not entirely sure if we need/want this.