Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FBXLoader: added warning for unsupported rotation orders #12411

Merged
merged 4 commits into from
Nov 23, 2017

Conversation

looeee
Copy link
Collaborator

@looeee looeee commented Oct 16, 2017

See #12405

I've had a look into supporting Euler rotation order other than XYX in the FBXLoader - it will take a bit of work, so in the meantime I've added a warning when unsupported orders are encountered.

If anybody else want to look into this here are some test files.

fbx_rotation_order_test.zip

The main issue I have run into here is that rotation order is hard coded as 'ZYX' in several places throughout the loader - this may mean that FBX is using extrinsic Euler angles since as far as I know the conversion from intrinsic XYZ order will be extrinsic ZYX. I'm not sure how to do this conversion for other orders though.

@WestLangley any ideas if this is correct, and if so how to go about converting the other possible orders?

@WestLangley
Copy link
Collaborator

Conversion from extrinsic to intrinsic via reversal of the order works for any Euler Order.

If you are including animation, make sure your examples are simple enough to understand what is happening.

If only one axis is being rotated, it does not matter what the Euler Order is, so I am not sure your test cases will be very informative.

@ignazioa
Copy link

@WestLangley agree on the reverse order, any idea how eSPHERIC_XYZ works ?

@WestLangley
Copy link
Collaborator

No idea.

@looeee
Copy link
Collaborator Author

looeee commented Oct 16, 2017

@WestLangley thank you - I had tried just reversing the order and it didn't work, so I guess I must have missed something else.

If only one axis is being rotated...

Are you looking at the preRotation? That is (-90, 0, 0), I'm not sure why since the I didn't do any initial rotating of the cube. I'm testing the animation here, which is a rotation defined on multiples axes.

@makc
Copy link
Contributor

makc commented Nov 2, 2017

unsupported rotation orders

wait, but Euler class (which object.rotation is) specifically supports any order? I was under impression that, if you set object rotation order, it sticks? of course if actual animation code is using some quaternion voodoo, it is irrelevant.

@makc
Copy link
Contributor

makc commented Nov 2, 2017

any idea how eSPHERIC_XYZ works ?

google pretends it's the same as euler xyz but I doubt it. you have to ask sdk engineers.

@looeee
Copy link
Collaborator Author

looeee commented Nov 3, 2017

google pretends it's the same as euler xyz

I suspect that they don't know (or don't support it) either and are just defaulting to XYZ in that case.

if actual animation code is using some quaternion voodoo

All rotations in the FBX format are stored as Euler angles. These are converted to quaternions by the loader.

@makc
Copy link
Contributor

makc commented Nov 3, 2017

back in 2012 Jiayang.Xu and Robert.Goulet @autodesk.com were kind enough to answer my fbx format questions. feel free to try your luck )

@makc
Copy link
Contributor

makc commented Nov 3, 2017

oh, one of them CCed [email protected] so maybe that is proper email to direct the questions to.

@looeee
Copy link
Collaborator Author

looeee commented Nov 3, 2017

Thanks for contacts!

With regards to eSPHERIC_XYZ I don't think it's something we need to worry about though.

@WestLangley
Copy link
Collaborator

@looeee
Copy link
Collaborator Author

looeee commented Nov 3, 2017

@WestLangley thanks, that solves that mystery 😁

@looeee
Copy link
Collaborator Author

looeee commented Nov 23, 2017

@mrdoob was there a reason that you didn't want to merge this? Even though it doesn't actually add support for different Euler orders it will at least display a useful warning and sets the ground work for adding support later.

@mrdoob
Copy link
Owner

mrdoob commented Nov 23, 2017

@mrdoob was there a reason that you didn't want to merge this?

I forgot 😇

@mrdoob mrdoob merged commit 9a41ff3 into mrdoob:dev Nov 23, 2017
@mrdoob
Copy link
Owner

mrdoob commented Nov 23, 2017

Thanks!

@looeee looeee deleted the FBXLoader_rotation_order_warning branch January 27, 2022 10:23
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.

5 participants