Unity-Technologies / Unity.Mathematics

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

Multiplication operator behaves differently between Unity.Mathematics.float4x4 and UnityEngine.Matrix4x4 #83

Open JussiKnuuttila opened 4 years ago

JussiKnuuttila commented 4 years ago

The multiplication operator is defined for both float4x4 and Matrix4x4, but has different semantics. The traditional Unity Matrix4x4 does the intuitive thing, which is a matrix multiplication, whereas float4x4 does a componentwise multiplication (like HLSL) and requires using mul to get the matrix multiplication.

This can lead to bugs that are very hard to spot while porting over math code from C# to HPC#, so it could be worth considering changing this, or at least very clearly highlighting this in the documentation and possible best practice / upgrade guides.

james7132 commented 4 years ago

This is apparently intended behavior. math.mul is how to do matrix-matrix and matrix-vector multiplication in Unity.Mathematics.

The intention is to view float4 and float4x4 first and foremost as packs of floats organized in SIMD compatible formats, and as traditional ND vectors and matrices second. Hence why component-wise multiplication is the default and not matrix multiplies.

There was a good talk about this kind of approach: https://www.youtube.com/watch?v=BpwvXkoFcp8

unpacklo commented 4 years ago

Yes, the multiplication operator as defined is intended. I agree that we could do better here with documentation to make it clear what is going to happen; a documentation pass will be done at some point to make it easier for those using Unity.Mathematics in the future.

jstine35 commented 3 years ago

For the matter of upgrading, the strategy I tend to employ for float4x4 upgrading existing code is cleverly ad-hoc:

  1. temporarily comment/disable the operator * for float4x4, which will cause any usage to surface as a hard compiler error.
  2. rewrite these using math.mul() until compiler errors resolved
  3. re-enable float4x4 operators in the Mathematics package

This works nicely so long as no other libs use the operator. In that case, some error filtering is needed.

That said, I firmly believe the rules of float4x4 operators to be one of the worst aspects of HLSL and GLSL alike, and so if there's any place were I personally wouldn't mind seeing some deviation from the HLSL syntax, it's with regards to matrix operations. The use cases for treating a float4x4 as a pack of floats is extremely rare/limited and largely a throw-back to old shader languages which lacked concepts of arrays. I suggest removing the multiply operator entirely for the multi-dimensional floats, eg float4x4. Generally speaking, the use case for operators is:

float4x4 clearly fails the first use case and also fails the second case as a result: The most common usage of float4x4 is for matrix mul, and so by having the operator perform a SIMD broadcast mul it greatly reduces the use or effectiveness of the operator. At the same time, having it perform a matrix mul is probably non-ambiguous and certainly not matching HLSL expected syntax. Finally, even in the case of matrix multiplication, such operations are some of the least common tasks when authoring SIMD, and are rarely well-suited for compound functional statements like the example above due to various issues with non-communitive behaviors.