Skip to content

Conversation

@looeee
Copy link
Collaborator

@looeee looeee commented Oct 20, 2017

FBXLoader currently creates a skeleton for all loaded models, whether or not they contain an animation. It then dumps everything (not just bones, but all meshes, groups, lights, cameras etc.) into the skeleton and uses that to link the animation.

As the comment mentions, this is quite a hacky way of doing things and should be rewritten, but in the meantime this at least solves the problem for files that don't contain any animation.

@jbaicoianu
Copy link
Contributor

If this gets merged before #11603 and ends up putting it in conflict, I will be very sad :(

@looeee
Copy link
Collaborator Author

looeee commented Oct 20, 2017

Note that this checks for 'Takes' in FBXTree to tell whether there is an animation.

Actual animation data is stored here

FBXTree.Objects.subNodes.AnimationCurveNode
FBXTree.Objects.subNodes.AnimationCurve
FBXTree.Objects.subNodes.AnimationLayer
FBXTree.Objects.subNodes.AnimationStack

While 'Takes' is used to specify multiple animation takes, so it may be neccesary to check for the presence of the above 4 nodes to know if there is animation.

However as far as I have been able to tell from the models I've tested, if there is an animation there will always be one take called "Take 001" specified, so the simpler test here should be sufficient.

@looeee
Copy link
Collaborator Author

looeee commented Oct 20, 2017

@jbaicoianu these PRs don't overlap so no need to worry about that 😀

@mrdoob
Copy link
Owner

mrdoob commented Oct 23, 2017

@jbaicoianu how should this PR look like if #11603 is merged?

@looeee
Copy link
Collaborator Author

looeee commented Nov 23, 2017

Closing in favour of #12729

@looeee looeee closed this Nov 23, 2017
@looeee looeee deleted the FBXLoader_conditional_animation_parsing branch January 27, 2022 10:21
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.

3 participants