LocalToWorld.Rotation or quaternion(float4x4) bug.

LocalToWorld.Rotation returns invalid rotation when it is scaled (uniformly).

Test Code

using Unity.Mathematics;
using Unity.Transforms;
using UnityEngine;
public class L2W_Quaternion_Bug : MonoBehaviour
{
    public float3 EulerDegree = math.float3(90, 90, 0);
    [Range(0, 10)] public float scale = 0.1f;
    public float3 Translate = 0;

    public quaternion rotationIn;
    public quaternion rotationOut;

    [NaughtyAttributes.Button(nameof(TestL2WRotation))]
    public void TestL2WRotation()
    {
        var l2w = new LocalToWorld();
        rotationIn = quaternion.EulerZXY(math.radians(EulerDegree));
        l2w.Value = float4x4.TRS(Translate, rotationIn, scale);
        rotationOut = l2w.Rotation;
        if (math.dot(rotationOut.value, rotationIn.value) < 0) rotationOut.value = -rotationOut.value;
    }
}

due to

        /// <summary>Constructs a unit quaternion from an orthonormal float4x4 matrix.</summary>
        public quaternion(float4x4 m)
        {
            float4 u = m.c0;
            float4 v = m.c1;
            float4 w = m.c2;

            uint u_sign = (asuint(u.x) & 0x80000000);
            float t = v.y + asfloat(asuint(w.z) ^ u_sign);
            uint4 u_mask = uint4((int)u_sign >> 31);
            uint4 t_mask = uint4(asint(t) >> 31);

            float tr = 1.0f + abs(u.x);

            uint4 sign_flips = uint4(0x00000000, 0x80000000, 0x80000000, 0x80000000) ^ (u_mask & uint4(0x00000000, 0x80000000, 0x00000000, 0x80000000)) ^ (t_mask & uint4(0x80000000, 0x80000000, 0x80000000, 0x00000000));

            value = float4(tr, u.y, w.x, v.z) + asfloat(asuint(float4(t, v.x, u.z, w.y)) ^ sign_flips);   // +---, +++-, ++-+, +-++

            value = asfloat((asuint(value) & ~u_mask) | (asuint(value.zwxy) & u_mask));
            value = asfloat((asuint(value.wzyx) & ~t_mask) | (asuint(value) & t_mask));

            value = normalize(value);
        }

summary said orthonormal, but uniform scaled Matrix is orthonormal.
uniform scale should be properly decomposed ahead.
Some thing like:

        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        internal static bool DecomposeRotationScale(this float3x3 rotScSh, out float3 scale, out float3x3 rotation, out float3x3 shearRotation, MainAxisID mainAxis = MainAxisID.ZAxis)
        {
            float3 c0 = rotScSh.c0, c1 = rotScSh.c1, c2 = rotScSh.c2;
            var lengthSq = math.float3(math.dot(c0, c0), math.dot(c1, c1), math.dot(c2, c2));
            bool hasValidScale =
                math.cmin(lengthSq) > Epsilon &&
                math.cmax(lengthSq) < EpsilonRcp &&
                math.isfinite(lengthSq.x) &&
                math.isfinite(lengthSq.y) &&
                math.isfinite(lengthSq.z);
            if (hasValidScale)
            {
                var mainAxisID = (int)mainAxis;
                var secondAxisID = (mainAxisID + 1) % 3;
                var thirdAxisID = (secondAxisID + 1) % 3;
                scale = math.sqrt(lengthSq);
                scale[thirdAxisID] = (math.determinant(rotScSh) < 0f) ? -scale[thirdAxisID] : scale[thirdAxisID];
                var scaleRcp = math.rcp(scale);
                shearRotation = math.float3x3(c0 * scaleRcp.x, c1 * scaleRcp.y, c2 * scaleRcp.z);
                rotation = RotationFromAxisAssumeNormalizedValid(shearRotation[mainAxisID], shearRotation[secondAxisID], mainAxisID);
            }
            else
            {
                scale = 1;
                rotation = float3x3.identity;
                shearRotation = rotScSh;
            }
            return hasValidScale;
        }
        [MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static float3x3 RotationFromAxisAssumeNormalizedValid(float3 mainAxis, float3 secondAxis, int mainAxisID = 2)
        {
            var secondAxisID = (mainAxisID + 1) % 3;
            var thirdAxisID = (mainAxisID + 2) % 3;
            var thirdAxis = cross(mainAxis, secondAxis);
            float3x3 rot = default;
            rot[mainAxisID] = mainAxis;
            rot[secondAxisID] = cross(thirdAxis, mainAxis);
            rot[thirdAxisID] = thirdAxis;
            return rot;
        }

these code is to decompose shear and none-uniform scale.
uniform scale should be much simpler.

Also LocalToWorld.Right/Up/Forward are all not normalized. but that’s fine.
Users still need to know that these values are not ensured to be Unit length.

@Dale_Kim The Issue link
https://github.com/Unity-Technologies/Unity.Mathematics/issues/171#issuecomment-704997545

Orthonormal transformation preserves angles and lengths and thus may not contain any scaling.

Oh, you right. I took Orthonormal transformation as linear transformation. (not native speaker).
So that’s not Unity.Mathematics’s fault. But Unity.Entities’s

    [Serializable]
    [WriteGroup(typeof(WorldToLocal))]
    public struct LocalToWorld : IComponentData
    {
        public float4x4 Value;

        public float3 Right => new float3(Value.c0.x, Value.c0.y, Value.c0.z);
        public float3 Up => new float3(Value.c1.x, Value.c1.y, Value.c1.z);
        public float3 Forward => new float3(Value.c2.x, Value.c2.y, Value.c2.z);
        public float3 Position => new float3(Value.c3.x, Value.c3.y, Value.c3.z);

        public quaternion Rotation => new quaternion(Value);
    }

Looks like Unity.Entities don’t know that this is wrong :public quaternion Rotation => new quaternion(Value);
and Right/Up/Forward are likely to be not normalized.

Translate/Rotation/Scale could be local to parent value.
LocalToWorld is the only place to get world transformation of entity.
LocalToWorld error is just Like Transform.Position/Rotation/Up/Right/Forward return wrong value.
How could this system work for any game?

This is a pretty performance critical function and using the most pessimistic method of extracting the data might not be a good idea either. The simplest solution is to remove these helpers and let everyone extract the data manually where they need it with more knowledge about what values are legit in their project.

Agreed, The reason I made this post is one of my team uses LocalToWorld.Rotation without know any detail. And got the wrong value. It looks pretty convenient but in fact pure evil. they had better not exist at all.

Public API should be something like GetRotationSafe(quaternion defaultRotation)
As zero scale or Shear transform could invalid rotation value.

1 Like

Just wanted to add that I see people using LTW much more than they probably should be due to not understanding bugs related to physics & instantiating entities after the Transform system. I think this will bite a lot of people.
Not sure if removing is the right thing to do here - if there’s an expensive but accurate way to decompose the values I personally think that would be preferable. Even if it divides the api. Clearly the root of the aforementioned issues need addressing but I think there are likely a lot of people that will experience wacky values at some point and will think they’re retrieving world space rather than local space or similar mistakes and waste a lot of time.

Exactly! After figuring out when L2W is updated and what’s wrong with what physics does. When having a litter fragile confidence of knowing what’s going on. He who finally gets a wrong world rotation from L2W will get lost forever…

1 Like