1635363 - Invert orientation after calling decompose() from XRRigidTransform

Status: RESOLVED FIXED

(Core :: WebVR, defect)

Description

[Daosheng Mu[:daoshengmu]]

We might have a potential bug in Matrix4x4::Decompose(). We refer the formula from [1], but when I was trying to

transformMatrix.RotateX(1.2f);
transformMatrix.Decompose(position, orientation, scale);

The orientation here is [-0.564642, 0.000000, 0.000000, 0.825336]. IIUC, the orientation.x should be positive because we rotate on +x.

I also notice Chromium is using the same formula of Matrix.Decompose() [2] when getting a XRPose, that makes they have to do this orientation invert [3] as well.

[1] http://www.euclideanspace.com/maths/geometry/rotations/conversions/matrixToQuaternion/index.htm
[2] https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/platform/transforms/transformation_matrix.cc;l=567;bpv=1;bpt=1?originalUrl=https:%2F%2Fcs.chromium.org%2F
[3] https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/modules/xr/xr_rigid_transform.cc;l=37;bpv=0;bpt=1?originalUrl=https:%2F%2Fcs.chromium.org%2F

Comment 1

[Daosheng Mu[:daoshengmu]]

I would prefer to merge this fix first for the coming FxR v.10 release. We can open a follow up bug to investigate this root cause more.

Comment 2

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1635363 - Invert orientation after matrix.decompose from XRRigidTransform.

Comment 3

[Daosheng Mu[:daoshengmu]]

The test case is https://playcanv.as/e/p/TAYVQgU2/.
It is only happened in PlayCanvas is because they use position and orientation DomPoints to generate a local matrix by their own [1]. Other WebXR examples like input-profile, they use transform.matrix [2]. So, I believe we did it correctly in Matrix4x4, and there is something wrong when getting the orientation from Matrix4x4.

[1] https://github.com/playcanvas/engine/blob/d9b54249828b016338e5ae23ab5bbfd0a69138d2/src/xr/xr-input-source.js#L230
[2] https://github.com/immersive-web/webxr-samples/blob/eea396e20366dd896532a8cf51537552fabc971f/input-profiles.html#L216

Comment 4

[Daosheng Mu[:daoshengmu]]

(In reply to Daosheng Mu[:daoshengmu] from comment #0)

We might have a potential bug in Matrix4x4::Decompose(). We refer the formula from [1], but when I was trying to

transformMatrix.RotateX(1.2f);
transformMatrix.Decompose(position, orientation, scale);

The orientation is [-0.564642, 0.000000, 0.000000, 0.825336]. IIRC, the orientation.x should be positive because we rotate degrees on +x.

I also notice Chromium is using the same formula of Matrix.Decompose() [2] when getting a XRPose, that makes they have to do this orientation invert [3] as well.

[1] http://www.euclideanspace.com/maths/geometry/rotations/conversions/matrixToQuaternion/index.htm
[2] https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/platform/transforms/transformation_matrix.cc;l=567;bpv=1;bpt=1?originalUrl=https:%2F%2Fcs.chromium.org%2F
[3] https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/modules/xr/xr_rigid_transform.cc;l=37;bpv=0;bpt=1?originalUrl=https:%2F%2Fcs.chromium.org%2F

According to the discussion here, https://stackoverflow.com/questions/4436764/rotating-a-quaternion-on-1-axis

The proper result of a quaternion rotates x-axis with 1.2 radian should be [0.564642, 0, 0, 0.825336].

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/247592e4f235
Invert orientation after matrix.decompose from XRRigidTransform. r=kip,imanol

Comment 6

[Daosheng Mu[:daoshengmu]]

The Quaternion.SetFromRotationMatrix() from Blender [1] is inverted when compared to Gecko's [2]. I guess we need to do some changes in our function.

[1] https://github.com/blender/blender/blob/2d1cce8331f3ecdfb8cb0c651e111ffac5dc7153/source/blender/blenlib/intern/math_rotation.c#L276
[2] https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/gfx/2d/Quaternion.h#49

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/247592e4f235