Skip to content
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

Maths: struct representing an Angle #1169

Open
vpenades opened this issue Dec 13, 2022 · 24 comments
Open

Maths: struct representing an Angle #1169

vpenades opened this issue Dec 13, 2022 · 24 comments
Assignees
Labels

Comments

@vpenades
Copy link

Summary of feature

In geometry, I work with angles quite often, so I ended having an Angle structure which has proven a lot more useful than I imagined, because it handles stuff like:

  • Radian/Degree conversion
  • Angle between vectors
  • radial arithmetics (an angle can represent a direction, but also the number of turns)
  • Equality (which is tricky; is 0° == 360° ? it depends on the context)

Once you think about it, angles have their own mathematical domain that has always been very underrated.

Taken from my code, it would be something like this:

struct Angle
{
    public float Radians;

    public float Degrees => Radians * 180 / Math.Pi;

    public static implicit operator float(Angle angle) => angle.Radians;

    public static Angle InDegrees(float degrees) ... 
    public static Angle InRadians(float radians) ... 
    public static Angle InClockTime(int hour, int min, int second) ... 
    public static Angle Between(Vector2 a, Vector2 b) ...
    public static Angle Between(Vector3 a, Vector3 b) ...
    public static Angle ArcTan(float y, float x) ...
    public static Angle ArcCos(float cos) ...
    public static Angle ArcSin(float sin) ...
    public static Angle Round(Angle angle) ... rounds any angle to 0°-360° range
    public static operator Angle +(Angle a, Angle b)....
}

Comments

The biggest advantage I had by using such a structure is that it greatly reduces the extremely common case of using radians in place of degrees or the other way around, in other words: it makes the concept of an angle value strong typed.

And it makes the code a lot more pleasant:

var angle = Angle.InDegrees(90) + Angle.Between( vector1, vector2 );

Does this have a proposal?

Check the documentation/proposals folder. If it doesn't have one, you may need to create one if you're making massive breaking changes.

@vpenades vpenades added the enhancement New feature or request label Dec 13, 2022
@Perksey
Copy link
Member

Perksey commented Dec 14, 2022

@HurricanKai @Beyley thoughts?

@Beyley
Copy link
Contributor

Beyley commented Dec 14, 2022

i personally would like it to at least have both a float and a double version, or even generic with all the math types being possible to shove in there, if feasable

@vpenades
Copy link
Author

I think generic would not make much sense, because integer values would be unusable, due to the values ranging around PI values and requiring high precission. This leaves single and double. again, since the values would be around PI, a float has plenty of precission. doubles would only make sense for insane precission, or for counting a very large number of "turns" while retaining precission.

Personally, I think a float implementation would cover most use cases, but if double is requested, then I would do AngleS[ingle] and AngleD[ouble]

@Beyley
Copy link
Contributor

Beyley commented Dec 14, 2022

I think generic would not make much sense, because integer values would be unusable, due to the values ranging around PI values and requiring high precission. This leaves single and double. again, since the values would be around PI, a float has plenty of precission. doubles would only make sense for insane precission, or for counting a very large number of "turns" while retaining precission.

Personally, I think a float implementation would cover most use cases, but if double is requested, then I would do AngleS[ingle] and AngleD[ouble]

im just being hopeful for the day of halfs being less jank in c# :^)
only really doubles and floats would have use yes (is decimal something you can shove into generic math things? that might also have use)

@HurricanKai
Copy link
Member

readonly struct Radians { public readonly float Value } is preferable IMO.
For maths it'd be on theme to make it take , but I don't really mind either option.

@Perksey
Copy link
Member

Perksey commented Dec 15, 2022

I think this is one for the working group to debate about actual API intricacies (i.e. what we want to expose) but the concept here is sound. Concur with generic maths, there are lots of places where the generic APIs are useless without floating point but the ability to specify the floating point precision is valuable nonetheless.

@Perksey Perksey added this to the Next Working Group Meeting milestone Dec 15, 2022
@vpenades
Copy link
Author

vpenades commented Dec 15, 2022

My reasoning to use "Angle" instead of "Radians" is because Angle is a concept, and "Radians" is a specific unit of measurement which yes, it happens to be hardware supported in some architectures. It would be like using "Metres" instead of "Scalar"... The same reasoning as to why TimeSpan is not called "Ticks" even if ticks is the internal representation.

So, although the internal representation is in Radians, I didn't want to favour too much radians over degrees, because degrees are more "human friendly"

Also, I think it's nicer: Angle.InRadians(3.14); and Angle.InDegrees(180) than new Radians(3.14) and Radians.InDegrees(180);

But it's also true that the implicit conversion would be to Radians, because it's what most math libraries expect, so to avoid ambiguities, I am not sure what would be nicer:

Math.Cos( Angle.InDegrees(90) ); // my current implementation
or
Math.Cos( Angle.InDegrees(90).Radians ); // avoiding ambiguity
or
Math.Cos( Radians.InDegrees(90) );

@roeyskoe
Copy link
Contributor

Angle.InRadians(3.14);

I would prefer that this would be Angle.FromRadians(3.14), since after all you are creating a new angle from a radian value.

Equality (which is tricky; is 0° == 360° ? it depends on the context)

Maybe Angle.Radians would always be in range of [0, 2Pi[, so contructing an angle from 2Pi radians (or 360°) would give you an angle with a value of 0. It could have a field like int FullRotations that would have a value of 1 in this case.
Comparison would be between the radian values, so Angle.FromRadians(0) == Angle.FromRadians(2Pi), and of course Angle.FromRadians(0).FullRotations != Angle.FromRadians(2Pi).FullRotations.
And also have something like float AbsoluteRadians that would essentially be FullRotations * 2 * Pi + Radians.

Some other stuff that would also be nice to have:

  • public Vector2 UnitVector: Vector that has a same direction as this angle.
  • public Direction MainDirection: Up/Down/Left/Right. For example Angle.FromDegrees(10).Direction == Direction.Right
  • public static Angle FromVector(Vector2)
  • public Vector2 Rotate(Vector2)

@Perksey
Copy link
Member

Perksey commented Dec 16, 2022

There's also the question of what do we store the value as - if we convert there is a possible loss of precision when round-tripping (e.g. if we store the angle as degrees, Angle.FromRadians(3.1212345).Radians may not equal 3.1212345 exactly given that we have to calculate this.)

@vpenades
Copy link
Author

vpenades commented Dec 16, 2022

just for reference, this is the structure I am using at work:

https://gist.github.com/vpenades/3d0b52e1bfc046191e685473ad006ab7

Some of the methods were a bit rushed because I added them when I needed them, so don't take it as rock solid knowledge.

@Perksey I think the internal representation should be one that can be used straight away with MathF.Cos(value); so the internal representation would be unconstrained radians. I think clamping an angle to the 0-360 range is equivalent to normalizing a quaternion, so it's something that should be explicitly set by the developer, not something automatic. In my code is called "Normalize360", because an angle could also be normalized in the range of -180 to 180 , I don't know which might be better.

To some degree (no put intended), Angle is similar to TimeSpan, in which you have "Seconds" and "TotalSeconds", the equivalent with Angle would be:

Angle.TotalRadians (unbounded) (internal representation)
Angle.Radians (normalized to 0 to 2π, or -π to π)
Angle.TotalDegrees (Unbounded)
Angle.Degrees (normalized to 0-360 or -180 to 180)
Angle.Turns (integer)
Angle.TotalTurns (float, so you could have 2.5 turns, which is 2 and half turns)

@Perksey
Copy link
Member

Perksey commented Jan 7, 2023

  • Useful, we think there's a use case

  • Use Scalar (i.e. generics)

  • Similar to TimeSpan?

  • What is normalized?

  • Don't think caring about integers makes sense

  • pi to -pi

  • We will use Radians internally

  • -pi to pi for radians, 0-360 for degrees always

  • Bunch of APIs, Working Group to take the provided gist and make a formal proposal with an extensive set of APIs

@vpenades
Copy link
Author

vpenades commented Jan 7, 2023

  • Similar to TimeSpan?

TimeSpan internal representation is Ticks, equivalent to Radians, But it also has Hour Minutes and Seconds, so you can have 14:25:10 ... but also TotalMinutes, TotalSeconds and so on.

If we take the analogy to Angle, we could split the value in two parts: Direction (like a compass) and Number of Turns

  • What is normalized?

Don't know exactly how to call this one: it essentially applies the modulus of PI to the value... effectively leaving the "compass direction" and resetting the number of turns to zero. This is usually useful when you have an object state representing the direction to which the object is facing, and you update the direction continuously. You don't want to keep the number of turns to preserve precission,

@Perksey
Copy link
Member

Perksey commented Jan 7, 2023

FYI here's the discussion recording, the notes probably don't go into all of the details of our discussion: https://youtu.be/gjneerMeRVU?t=3720

@vpenades
Copy link
Author

vpenades commented Jan 7, 2023

I've watched the video, and looks like there's a preference to keep the Angle always normalized. But there's some key use cases where it's important to define angles beyond the -pi to pi range. Which is Angular Velocities

Consider simulating a spinning fan. You need it's current angle state and it's angular velocity, which happens to be quite high:

Angle FanAngle;
Angle FanAngularV = Angle.FromDegrees(11880); // angular velocity

void UpdateFan(TimeSpan step)
{
   FanAngle += FanAngularV * step.TotalSeconds;
   FanAngle = FanAngle.Normalize();
}

There's lots of things that spin at very high angular velocities: fans, propellers, saws, gears, engine parts, wheels, black holes, etc

Just the wheel of a car racing at top speed is already spinning several times per second...

@Hanprogramer
Copy link

Hanprogramer commented Jan 9, 2023

Couldn't these be static methods instead? Like in Godot there's deg2rad and rad2deg functions

@Beyley
Copy link
Contributor

Beyley commented Jan 9, 2023

Couldn't these be static methods instead? Like in Godot there's deg2rad and rad2deg functions

This is not just for conversion, its for explicit typing of a concept, with more than just 'degrees to radians' and 'radians to degrees'

@Beyley
Copy link
Contributor

Beyley commented Jan 9, 2023

I've watched the video, and looks like there's a preference to keep the Angle always normalized. But there's some key use cases where it's important to define angles beyond the -pi to pi range. Which is Angular Velocities

Consider simulating a spinning fan. You need it's current angle state and it's angular velocity, which happens to be quite high:

Angle FanAngle;
Angle FanAngularV = Angle.FromDegrees(11880); // angular velocity

void UpdateFan(TimeSpan step)
{
   FanAngle += FanAngularV * step.TotalSeconds;
   FanAngle = FanAngle.Normalize();
}

There's lots of things that spin at very high angular velocities: fans, propellers, saws, gears, engine parts, wheels, black holes, etc

Just the wheel of a car racing at top speed is already spinning several times per second...

I think the sentiment during the meeting was the majority of cases are better off with consistent float accuracy, and the cases it being used for rotational velocity are lower, (and can just be handled by an overload of the + operator for Angle + T)

@Hanprogramer
Copy link

I see. But would this impact memory usage? for example there's 50k objects each have it's own Transform. But now there will be Transform and Angle

@Beyley
Copy link
Contributor

Beyley commented Jan 9, 2023

I see. But would this impact memory usage? for example there's 50k objects each have it's own Transform. But now there will be Transform and Angle

Well as seen in the original proposal these are not objects, they are structures

@Hanprogramer
Copy link

I see. But would this impact memory usage? for example there's 50k objects each have it's own Transform. But now there will be Transform and Angle

Well as seen in the original proposal these are not objects, they are structures

So structures are actually smaller than classes in size?

@Beyley
Copy link
Contributor

Beyley commented Jan 9, 2023

I see. But would this impact memory usage? for example there's 50k objects each have it's own Transform. But now there will be Transform and Angle

Well as seen in the original proposal these are not objects, they are structures

So structures are actually smaller than classes in size?

Google can best explain the nuances, but yes, generally a struct will always be smaller

@Perksey
Copy link
Member

Perksey commented Oct 1, 2023

This has been assigned to @HurricanKai to make a formal proposal for WG discussion

@Perksey
Copy link
Member

Perksey commented Nov 11, 2023

@HurricanKai WG meeting feels like it might be getting close, any progress on this? :)

@Perksey
Copy link
Member

Perksey commented Nov 19, 2023

This has been postponed to the next working group meeting and is currently being championed by @dfkeenan in #1806

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants