Stride: Xenko.Core.Mathematics library

[Qibbi]

Color:
Color uses byte for its channels. But in functions like public static Color Add(Color left, Color right) just add them together which returns an integer. These integers are used in the construction of the result, but because of overloads the constructor that is actually called is public Color(float red, float green, float blue, float alpha).
This constructor uses R = ToByte(red); which results in bad values.
So for every method casts to byte should be used.

A lot of other structs:
With the addition of the in keyword a lot of the methods of this library might be faster. Additionally a lot of ref keywords which are used for that can be changed to in to have the const contract.

[SleepyMode]

Let me give this a shot.

[SleepyMode]

Okay, so, quick update.
The in keyword cannot be used due to the fact we're using C# 7.0 and the in argument modifier has only been introduced in C# 7.2.

As for the byte casts, I'm just going over it again and then I'll throw in a pull request.

[aaronfranke]

Color math should ideally be done with float to support arbitrary precision. It's somewhat common to have 10-bpc and 12-bpc displays, and HDR color math often needs at least 16 bits per channel.

Xenko's Color3 and Color4 classes appear to already work this way. Perhaps it would be best to deprecate Color or perhaps rename it to Color8 or Color32?

Side note, what's the point of Color3? You can just have constructors and methods that take the alpha value as an optional parameter for use cases that don't need it.

If it were up to me, I'd rename Color to Color8 and consider deprecating it, and merge Color3 and Color4 into one Color class using optional parameters for the alpha value. That last part (making Color4 alpha optional), isn't a breaking change, but the rest of this very much is.

[SleepyMode]

Color math should ideally be done with float to support arbitrary precision. It's somewhat common to have 10-bpc and 12-bpc displays, and HDR color math often needs at least 16 bits per channel.

Xenko's Color3 and Color4 classes appear to already work this way. Perhaps it would be best to deprecate Color or perhaps rename it to Color8 or Color32?

Side note, what's the point of Color3? You can just have constructors and methods that take the alpha value as an optional parameter for use cases that don't need it.

If it were up to me, I'd rename Color to Color8 and consider deprecating it, and merge Color3 and Color4 into one Color class using optional parameters for the alpha value. That last part (making Color4 alpha optional), isn't a breaking change, but the rest of this very much is.

Does that really belong here though? I'd recommend making a new issue with the suggestion so it doesn't disappear once this one is dealt with.