Unity-Technologies / Unity.Mathematics

The C# math library used in Unity providing vector types and math functions with a shader like syntax
Other
1.38k stars 156 forks source link

A ton of issues with 'half' (including bugs) #185

Open MrUnbelievable92 opened 3 years ago

MrUnbelievable92 commented 3 years ago

Soooo... ;D

The (not) equals operators are broken: -0 against +0 equality is not being considered Proof:

UnityEngine.Debug.Log(math.asfloat(1u << 31) == math.asfloat(0u));  

// output: TRUE

Also, inequality of NaN against NaN is not being considered. Proof:

float x = float.NaN;
UnityEngine.Debug.Log(x == x); 

// output: FALSE

This also applies to IEquatable.Equals().

The operators '<', '>', '<=' and '>=' are missing (internally, just convert the operands to floats). Those would be user-friendly wrapper functions instead of having to cast two halfs explicitly all the time.

Consequently, an IComparable implementation is missing.

Obviously, all the arithmetic operators are missing, which could also be wrapped. I understand that that's harder to do both for you aswell as the computer (casting back and forth instead of just promoting to floats and returning a boolean evaluation), but numbers of any kind should at the very least be (I)comparable. If the arithmetic operators are implemented, though, they should return floats (identical to (u)short operations).

Typecasts from/to (u)ints are missing. Currently we have to type (half)(float)myInt, which, once again, could be wrapped. If you decide to do this, do it for (u)long in order to cover all possibilites.

MinValue and MaxValue should ONLY be halfs (just like System.Single and System.Double), if anyone references them as floats or doubles they get compile time evaluated anyway; that would be a backwards-compatible change due to implicit casting. Mark "Min/MaxValueAsHalf" as obsolete. Btw the float values, if they have any reason to exist, should be constants so we can see them in the IDE (or you can write XML documentation for the half-only values i.e. "65XXX as a float"). Mathematics is very strange in that regard. Most constants of custom types are static readonly variables, which clutters RAM unnecessarily and causes performance "problems" in managed code (RAM read) whereas they should be static readonly properties i.e. an inlined function call to "XOR regA regA" (mostly talking about "zero" here...), while float values, which can be const, like half.Min/MaxValue are implemented as static readonly properties :D alt text

Missing static properties: Epsilon is missing which is (half)0.000000059604644775390625d or new half { value = 1 } NaN is missing which is new half { value = 0xFE00 } NegativeInfinity is missing which is new half { value = 0xFC00 } PositiveInfinity is missing which is new half { value = 0x7C00 }

More static functions are missing which is not user-friendly (having to call float.IsNaN etc) and also causes software implemented casting all the time (also in Burst, where efficient 16-bit SIMD ops could be used instead of casting and performing a float comparison):

public static bool IsInfinity(half h)
{
    return (h.value & 0x7FFF) == 0x7C00;
}

public static bool IsNaN(half h)
{
    return (h.value & 0x7FFF) > 0x7C00;
}

public static bool IsNegativeInfinity(half h)
{
    return h.value == 0xFC00;
}

public static bool IsPositiveInfinity(half h)
{
    return h.value == 0x7C00;
}

...which would result in the following branch-free, ILP implementations of '==' and '!='

public static bool operator == (half left, half right)
{
     bool nan   = !IsNaN(left) & !IsNaN(right);
     bool zero  = ((left.value & 0x7FFF) == 0) & ((right.value & 0x7FFF) == 0);
     bool value = left.value == right.value;

     return nan & (zero | value);
}

public static bool operator != (half left, half right)
{
     bool nan   = IsNaN(left) | IsNaN(right);
     bool zero  = ((left.value & 0x7FFF) != 0) | ((right.value & 0x7FFF) != 0);
     bool value = left.value != right.value;

     return nan | (zero & value); 
} 

Additionally, there's .NET standard stuff that nobody really needs but which is trivial to implement (especially since it would only have to be done for the half type itself and not the vectors):

public half Parse(string s)
{
    return (half)float.Parse(s);
}

public bool TryParse(string s, out half result)
{
    bool success = float.TryParse(s, out float cvt);

    result = (half)cvt;

    return success && cvt <= MaxValue && cvt >= MinValue;
}

... Aswell as an implementation of the "IConvertible" interface, which is probably more meaningful, since I know a lot of software that uses this among others as a generic constraint to identify numeric types.

Thanks for taking the time to read this! - And sorry for the edits; I'm an iterative guy.