-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Angle
type
#11132
Comments
I like option 3. Option 2 seems frustrating and inferior to 3. Option 1 doesn't seem like a meaningful improvement over just using floats. |
What do you think about using the They split the two representations into two structs which implement a common Obvious cons:
Solutions to the cons:
|
IMO we should just write our own rather than worrying about more trivial dependencies. |
Personally i dont like this idea at all, because angles are too common to restrict the ergonomics of calculations with typed float. Rust is a good language to guide its users using its type system, but sometimes they should think about the logic of their application themselves. If community would prefer typed angles, I propose the following:
Working with angles would be like so:
We can also add functions with
No breaking changes should occur (i think, i am writing this on phone right after i saw this) |
I think the current situation is fine actually. Both the unit and the direction are the standard across CS and mathematics. It's something you only need to learn once. I don't want to have to write something like |
Counterclockwise is the convention for radians, but I wouldn't say that radians are necessarily "the standard" in mathematics. For working with pi and trigonometric operations, it is, but a lot of geometry also uses degrees. More importantly though, a very large chunk of game developers don't have a deep background in CS or mathematics. A lot of them are hobbyists and some are artists. In most art programs, clockwise degrees are actually the standard for 2D rotations, and that's also what Godot uses for 2D transforms. While radians are more common, at least in internal computations, there is no singular convention that works for everything. In a lot of cases, degrees can just be more intuitive from the user's perspective. Personally, I still like the third option I proposed, and updating Bevy's examples to use it just convinced me further of its benefits. For me, ergonomics isn't just about saving some typing, but that the code is also as readable as possible. It should be clear that a value is an angle without having to look at docs or the surrounding code for extra context. You can also still do most math operations on That being said, I do really like your proposal. It's pretty similar to the enum approach, but having two separate types like that is more ergonomic and explicit. I'm personally not a fan of the implicitness of automatically converting floats to angles, but if others prefer that, I'm open to it. I'd probably change I'll probably make two PRs, one with the third option I proposed, and one with the |
Main purpose of |
I'm totally on your side alice. I also would rather vote for a reimplementation of the angle crate instead of using it. That's what I wanted to argue for with the second half of my message. I just wanted to say that we can use the crate as a reference. The ideas proposed by aleqdev are exactly the ideas from the I'm also not a fan of the implicit conversions from primitive rust types like Other than that splitting the trait up into two traits for each angle type doesn't really make sense to me yet. I thought that the unified That being said, I probably need to experiment with all of this a bit myself. I'd happily provide/contribute to the PR for this idea as well. |
I've been experimenting with this more, and I think I made a PR for this already: #11148. For now, I didn't add implicit conversion from If people want, I can still make a PR using the third option originally shown in this PR, and that's what I actually started with. I do think the Thanks everyone for the feedback and ideas! Feel free to continue sharing thoughts and alternative API ideas if you have any :) Edit: After further experimentation and feedback, I think the issue of math operations like |
I'm generally all in favor of helping usability and lowering the barrier to entry, but I have to admit here that I'm in favor of, if doing anything at all, at least a solution where the math standard (rad, ccw) is the default everywhere, and can be used implicitly. Degrees are best used in UIs etc. because they yield "round" numbers like 90 degrees, easier to input. But there's no math function using them (look at trig functions in For orientation, supporting both CW and CCW equally is a recipe for confusion and errors. There's a strong argument for having a unique standard internally across all Bevy code, third party plugins included. I'd be a lot warmer to the idea if we were talking about a higher level layer like the Editor UI or some scripting language, which are targeting a less code savvy audience. For Bevy internally, not so much. |
It is in radians because every angle in Bevy is in radians. As far as rotation direction, Calling rotations "clockwise" or "counterclockwise" is ambiguous without specifying the direction you're looking in. This comes up often with yaw if vertical direction is up, since most people instinctively measure direction with respect to the ground, and thus coming up with the exact opposite answer. |
How do you know that from the perspective of a user who doesn't know Bevy's codebase? Godot uses (clockwise) degrees for 2D transform rotation and skew, and most art programs use degrees for rotating things. It's not a fully universal convention to use radians for everything, and even if it was, new users wouldn't necessarily know that. For public APIs, degrees can just be more intuitive and ergonomic in some cases (see my comment here for a bit more examples). This doesn't mean that Bevy's APIs should actually use degrees, but taking an
Not sure about this, but it might just be because I'm used to the current way of thinking about it. I believe Thinking of it in terms of planes is more confusing to me, but I suppose it's mostly a matter of viewpoints. |
Because it's the default convention in almost any math code, everywhere. This goes back again to the fact that standard libraries in most languages only provide radian trig functions. But I agree that a sprinkle of documentation would help remind the users about this.
In code? Or in UI? Because this doc of
Yes, I agree that allowing some API entry points to take degrees via an The drawback of introducing an explicit |
Note that this is copied from |
Because I know that
It is common to see degrees in any kind of user interfaces. If bevy had an editor, it would be reasonable to use degrees there by default (with mandatory In code, it's radians everywhere, because you're supposed to pass them around as a number to quaternions and such without needing to think about whether degree multiplier is attached or not.
Here's usual graph paper from a school. Which direction is "clockwise"? https://graph-paper.net/wp-content/uploads/2019/11/Graph-Paper-with-Axis-.jpg |
That is exactly my argument against it.
Can't change that as long as you use If you decide to change to |
That being said, having It would have:
Is it worth having it as part of bevy? Probably yes, because we want an to have an editor eventually, where degrees are preferable. This is similar to color (where we should use RGB everywhere in code, but in the editor user might prefer to pick HSV). This is similar to rotation (where we should use Quat everywhere in code, but in the editor user might prefer to select yaw-pitch-roll). |
I think I'll close this for now; I myself am unsure if we actually want angle types, at least in the form displayed here. While they would (in my opinion) be more explicit and potentially ergonomic when implemented properly, I agree that in code we should perhaps just use radians consistently for all APIs and leave degrees for very high-level things like editor UI. Radians are the sensible default for almost all code, and if you really wanted to use degrees for clarity, you could just do e.g. Feel free to reopen if you have any new ideas or thoughts on this though! Something that is adjacent to this topic but that we will actually need is some kind of I'll probably make an issue and PR for this soon-ish. A separate rotation type would also allow us to explicitly state a consistent default for the rotation direction and unit, as well as make the API more ergonomic for rotations if possible. |
We do have explicit units in the colour system and in the sizing (e.g. pixels as explicit unit for UI). It would seem inconsistent to disallow the same for angles. From my dayjob in unity, actually 90% of the time I code in neither angles nor radians, but only in full rotations. For me, this is the most intuitive notation (the metric unit of angles), and it plays nicely with all kinds of other procedural maths, as multiplication is neutral with a full rotation. I would love an explicit |
# Objective - Another way of specifying rotations was requested in #11132 (comment) ## Solution - Add methods on `Rot2` - `turn_fraction(fraction: f32) -> Self` - `as_turn_fraction(self) -> f32` - Also add some documentation on range of rotation ## Testing - extended existing tests - added new tests ## Showcase ```rust let rotation1 = Rot2::degrees(90.0); let rotation2 = Rot2::turn_fraction(0.25); // rotations should be equal assert_relative_eq!(rotation1, rotation2); // The rotation should be 90 degrees assert_relative_eq!(rotation2.as_radians(), FRAC_PI_2); assert_relative_eq!(rotation2.as_degrees(), 90.0); ``` --------- Co-authored-by: Joona Aalto <[email protected]> Co-authored-by: Jan Hohenheim <[email protected]>
What problem does this solve or what need does it fill?
Angles come up quite frequently: transform rotations, primitive shapes, physics, spotlights, rendering, and even touch input for stylus altitude angles, just to name a few.
However, angles come with several ambiguities. Consider the following:
Is the angle in degrees or radians? If it represents a rotation, is it clockwise or counterclockwise? 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.
This issue focuses mainly on an
Angle
type, asRotation2d
would have slightly different needs, and I'm not sure if we even need a new type for it. See the "Angle
vs.Rotation2d
" section at the end.Let's establish some goals:
What solution would you like?
There are several options here, and they all have their tradeoffs. I don't think there is an approach that ticks all the goals listed above, so some compromises are necessary.
Option 1: Type aliases
The simplest option is to just add type aliases:
This very straightforward approach accomplishes goals 2, 3, 4, and (decently) 5, but explicitness is lacking.
While it is clear from the function signature or property type what kind of angle it should be, it's not clear from the actual value, which hurts readability. For example, the issue when rotating a transform remains:
Is
1.4
in degrees or radians, and is it clockwise or counterclockwise? You'd need to check the type and/or docs to find out, so this doesn't really help improve code readability that much.There's also a potential footgun here;
Radians + Degrees
does not produce a sensible value, because they are just aliases for normal floats and don't actually treat the values as special angles.Option 2: Enum
The second approach is to use an enum:
Alright! This ticks 3 and 4, and mostly ticks 1 and 2.
It's very explicit and ergonomic to write
Angle::Degrees(90.0)
. However, what should happen if you doAngle::Degrees(90.0) + Angle::Radians(PI)
? Should it return degrees or radians? Mathematical operations would need separate methods that return different types, and it quickly becomes highly unergonomic.Another issue is that a specific variant will always be more efficient. Most internal computations in e.g. physics and rendering use radians, so if the user provides degrees, it might be necessary to always convert it to radians, which is just unnecessary computation. However, I suppose you could internally convert just once and reuse that everywhere, so I'm not sure if this is actually a big issue.
Option 3: Struct with radians internally
The third and perhaps most common option is to have a struct that just stores the angle in radians:
This is very similar to the
Rotation2D
type in theeuclid
crate. I'd say that it ticks goals 1, 2, 3, 4 and mostly ticks 5, so it's quite close to ideal.The API could be roughly like this:
From the outside, this approach doesn't make it clear what unit the value inside
Angle
uses, but it's not necessarily an issue. It simply abstracts the unit away via methods and makes computations between radians and degrees straightforward, unlike the previous approaches.Internally, the angle is stored as radians, because it's needed for trigonometric operations and therefore much more commonly used in actual computations. This does mean that degrees must be converted to radians, but it's only during the construction of the angle, and usually it would need to be done anyway.
Option 4: Struct with sine and cosine of angle
This is a more special approach that attempts to optimize common operations. The angle is represented like this:
The approach and API is nearly identical to Option 3, and at least ticks goals 1, 2, and 5. However, instead of storing the raw angle, we store its sine and cosine.
The reasoning here is that usually when working with angles, we actually want the sin/cos/tan of the angle. By computing it just once, we can make several computations more efficient. This is what my physics engine bevy_xpbd does for the 2D version of the
Rotation
component, and it speeds up e.g. the common task of rotating vectors:However, here come the caveats that probably make this approach undesirable for a general-purpose
Angle
type..sin()
and.cos()
must always be called (unless created using the actual sine and cosine).atan2()
Add
/Sub
/Mul
/Div
are more expensive, e.g. adding angles requires four multiplications, one addition, and one subtraction instead of just a single addition.Rotation
type, but not for an angle type.Extra
Number type
We don't always want just
f32
angles. The Bevy codebase already has somef64
angles.We could add a
DAngle
orAngleF64
type for this, but I believe generics are nicer here. I have a working version ofAngle<T>
withf32
andf64
support already, although it does require a trait likeNum
for generic number types.Angle
vs.Rotation2d
Instead of adding an
Angle
type, we could just add a 2D rotation type. However, I feel like that'd be a bit too niche for now, as it'd only really be used for a rotation inTransform2d
once we have that, and maybe things like rotating colliders and primitive shapes. We don't really have that many proper use cases forRotation2d
in Bevy itself yet.If we added
Rotation2d
, it'd also make sense to have aRotation3d
, but that's just a quaternion, which already has a type, so the naming wouldn't be very consistent.I think a general-purpose angle type would be more valuable for now, and we could always add a rotation type that builds on it if needed in the future.
The needs for
Rotation2d
are also different, so a different representation could make more sense. I believe the options would be:Angle
clamped to the [-pi, pi] rangeMat2
Angle
in Glam?Angle
has the issue that we can't use it for all APIs in Glam types (Bevy's math library). For example,Quat::from_rotation_x
just takes anf32
, and we can't change that with a type in Bevy.As it turns out, Glam did originally have an
Angle
type, but it was removed, see bitshifter/glam-rs#22. The reasoning was basically that it was perceived as too high level for a low-level library like Glam, as it can just assume everything to use radians.I think that this is a valid point, but in practise I'd still like to have an
Angle
type in Bevy due to the explicitness and ergonomics it provides. Our options are to (1) just add it into bevy_math, ignoring Glam's APIs that use angles, or (2) open discussion to add backAngle
to Glam itself.Conclusion
Personally, I think I prefer Option 3, and I have it mostly implemented for Bevy already. However, if we don't want to add a new struct, then the type alias approach could be okay for now, as it'd at least help make function signatures a bit more explicit.
Other opinions on this and the need for a
Rotation2d
type would be appreciated though :)The text was updated successfully, but these errors were encountered: