Skip to content

Fix Euler rotation order to use ZYX (intrinsic) instead of default XYZ (intrinsic)#16

Open
petertheprocess wants to merge 1 commit intofan-ziqi:mainfrom
petertheprocess:euler-order-bug-fix
Open

Fix Euler rotation order to use ZYX (intrinsic) instead of default XYZ (intrinsic)#16
petertheprocess wants to merge 1 commit intofan-ziqi:mainfrom
petertheprocess:euler-order-bug-fix

Conversation

@petertheprocess
Copy link

Summary

  • Fix Euler rotation order to use ZYX (intrinsic) instead of default XYZ to correctly match URDF/MJCF euler angle conventions
  • Update rotation settings in MJCFAdapter.js, USDAdapter.js, and InertialVisualization.js

Problem

Three.js Object3D.rotation.set() uses XYZ euler order by default, which differs from the euler order used in URDF formats, and the self-built quaternionToEuler function https://github.com/petertheprocess/robot_viewer/blob/53d3103c2a818d20358fc2bec423303147212fa0/src/adapters/MJCFAdapter.js#L999-L1024.

This caused incorrect orientation of visual/collision meshes and inertial visualizations.

Solution

Explicitly specify 'ZYX' euler order when setting rotations to ensure consistency with robot model format conventions.

Result

Using agilex_piper mjcf from mujoco_menagerie as demonstration

image 1
Before (Default XYZ)
image 2
After (ZYX)

TODO for improvement

  • parse eulerseq tag for mujoco mjcf, for ref: https://mujoco.readthedocs.io/en/stable/XMLreference.html#compiler-eulerseq
  • Refactor internal rotation representation in UnifiedRobotModel.js: Switch from Euler angles (origin.rpy) to Quaternions during the model load stage. Storing rotations as Quaternions avoids ambiguity in the rendering pipeline and eliminates the risk of missing the explicit 'ZYX' order in Three.js rotation.set() calls.

More ref for euler order

@fan-ziqi
Copy link
Owner

Thank you for fixing it! Will you continue implementing the changes in the TODO list?

@petertheprocess
Copy link
Author

Thank you for fixing it! Will you continue implementing the changes in the TODO list?

For now, I think it’s better to leave it as is. The TODO list requires a bit more refactoring, and I’d prefer to handle that in a separate PR when I have more time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants