facebookincubator / FBX2glTF

A command-line tool for the conversion of 3D model assets on the FBX file format to the glTF file format.
Other
2.1k stars 332 forks source link

Missing joints in output GLTF "joints" list #126

Open helikopterodaktyl opened 6 years ago

helikopterodaktyl commented 6 years ago

I am trying to convert this model http://3drt.com/store/characters/felguard-monster.html to GLTF.

I used the commandline ....\FBX2glTF-windows-x64.exe -i fellguard.FBX -o felguard

It looks a bit weird. The ["skins"]["joints"] array seems to be missing several joints, for example node 29 is referred to in node children and in animation channel target node, but is nowhere to be seen on the list

Apparently, only some viewers can view this output GLTF properly. This one can https://gltf-viewer.donmccurdy.com/ but Windows's "Mixed Reality Viewer" and Blender importer are failing miserably on the output GLTF.

zellski commented 6 years ago

Thanks @helikopterodaktyl! I will investigate.

zellski commented 6 years ago

Two months later –– I think what we see here is an example of #1 in Don McCurdy's "skinning scenarios that may be challenging" here: https://github.com/KhronosGroup/glTF/issues/1403

As you say, the generated joints hierarchy is "missing" nodes: not only is it a subset of the scene graph, but it's a disjoint subset, and not actually a tree or even hierarchy at all. I believe this rhymes with others discussions on glTF threads too, perhaps particularly this one: https://github.com/KhronosGroup/glTF/issues/1270

Ultimately I believe FBX2glTF is outputting valid glTF here, and that it's the clients you mention that are doing the "wrong" thing. But what's right and wrong is unusually (for glTF) ambiguous here, and the clarifications those good folks have arrived at came long after the spec itself (and probably long after even the most recent version of Windows 3D Viewer, to take an example.)

FBX2glTF does have at least one related bug; it computes the "skeleton root" incorrectly here. But that's not what's causing the problems you see (confirmed by simply editing the generated glTF). And the "skeleton" property is optional anyway, and perhaps even confounding. It may be deleted from future versions of the spec.

For this app, pragmatically, it may be meaningful to add a command line switching for generating complete joint hierarchies, even though the spec doesn't demand it. It'll increase the size of the output pointlessly, but in reality the size difference may be marginal. I'll play around a bit this morning and see.

zellski commented 6 years ago

image Good news -- exporting the entire relevant subgraph into the skin's "joints" array solves the rendering issues, at least in Windows 3D Viewer. The size increase is tiny, so I may just make this the default mode.

This would be a really great time to have a strong regression suite in place... I do worry that doing this will bump up against some of the other peculiarities that people report. And it still doesn't load in the online Cesium viewer, which is troubling, although it does pass the glTF Validator.

helikopterodaktyl commented 6 years ago

Hmm. I haven't considered that it's a case of non-joint nodes mixed with joint nodes. If my understanding is correct, in such case it's not allowed to add non-joint nodes as joints, because it will break JOINTS_0 attribute, which indexes by joint numbers rather than node numbers?

zellski commented 6 years ago

Correct, but I'm just appending the "fake" joints at the very end of skins.joins, so the entire old range of joint indices is unchanged. There's just a bunch of new ones at the end now, which are unreferenced by any JOINTS_0 attribute but still clearly help some loaders sort out the scene graph.

fire commented 5 years ago

Was your prototype code published? Looking at solving this problem.

helikopterodaktyl commented 5 years ago

I managed to fix the issue for myself, but I use my own GLTF loader/viewer. I just only use the joints array for mapping inverse bind matrices to nodes/joints, outside of that I completely ignore the existence of it and work on nodes directly instead. With such change, the models converted through fbx2gltf work.