dotnet / Silk.NET

The high-speed OpenGL, OpenCL, OpenAL, OpenXR, GLFW, SDL, Vulkan, Assimp, WebGPU, and DirectX bindings library your mother warned you about.
https://dotnet.github.io/Silk.NET
MIT License
4.06k stars 391 forks source link

Maths: struct representing an Angle #1169

Open vpenades opened 1 year ago

vpenades commented 1 year ago

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:

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.

Perksey commented 1 year ago

@HurricanKai @Beyley thoughts?

Beyley commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.

vpenades commented 1 year ago

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 commented 1 year ago

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:

Perksey commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago
vpenades commented 1 year ago
  • 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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Beyley commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

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

Perksey commented 10 months ago

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

Perksey commented 10 months ago

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