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.07k stars 331 forks source link

Blend shape animation converted to morph targets but not playing (Blender -> FBX 6.1 -> glTF) #56

Open artemistint opened 6 years ago

artemistint commented 6 years ago

I think the problem is that the model was split to four submeshes(and the names of those submeshes are even not listed in the gltf file). The morph targets are working, but the animation is not playing (the intention is to play through morph targets as a sequence.

Model: https://drive.google.com/drive/folders/1NE8uqjBPg-g0d8A5pUJ_sJW680cC9A5E?usp=sharing

zellski commented 6 years ago

Hey @artemistint, thanks for the report. I probably never tested morph targets with meshes that require splitting. I appreciate the test model and will look into as soon as I can.

Usually the reason a mesh gets split up is if parts of it use one material (or material variation, e.g. transparent vs not) and another part another. It will also happen if you have > 65535 vertices in a mesh; this harkens back from glTF 1.0 and should probably be made optional; it has some pros and some cons.

In any case, hopefully we can make morph targets work with any kind of splitting.

chipweinberger commented 6 years ago

Zellski,

are you familiar with this SDK function? I noticed in your code you do not use it.

FbxGeometryConverter converter(fbxMgr);; converter.Triangulate(fbxScene, true); // glTF supports triangles only converter.SplitMeshesPerMaterial(fbxScene, true);

zellski commented 6 years ago

converter.SplitMeshesPerMaterial(fbxScene, true);

Yeah, it'd be redundant in our code because it happens automatically in RawMaterial::CreateMaterialModels, after all the identical-vertex-elimination code has run and all the meshes have been torn apart and reconstructed, with the key test here:

https://github.com/facebookincubator/FBX2glTF/blob/a2d5c7d87b1b61572d2ea40afe9575075276cbb0/src/RawModel.cpp#L440-L448

It's possible that under some circumstances we could just use converter.SplitMeshesPerMaterial early on, but right now I don't see an obvious gain.

zellski commented 6 years ago

Apologies for the delay. Other duties have taken all my time.

Taking a step back, the real answer here is obviously to add logic that uses 16-bit or 32-bit indices by default depending on the size of the model's mesh(es). That way this model will work right out of the box.

(I'll add command-line override switches as well.)

chipweinberger commented 6 years ago

Zellski, on the topic of FbxGeometryConverter converter(fbxMgr) I actually don't recommend you use it.

I have a couple meshes that crash in SplitMeshesPerMaterial() and the only straightforward easy way to fix it was to build that functionality into my converter my self like you have.

As a plus to this approach you can also more easily group the split meshes as primitives in the same mesh which is nice.

zellski commented 6 years ago

@chipweinberger Not to be too mean to Autodesk, but this seems about par with the SDK in general. Lots of competent code in there, but it kind of feels like development stalled around 2013 and since then it's mostly been layers of new functionality rather than muscular moving-forward of the platform and doing due diligence on engineering quality.

zellski commented 6 years ago

Actually @artemistint do you by any chance have the original .fbx file that caused this issue? I understand if it's proprietary or private, but if not -- makes it a lot easier to debug what's going on!

zellski commented 6 years ago

In the short term, I will implement 32-bit indices (as per https://github.com/facebookincubator/FBX2glTF/issues/68); that will work around this bug until I have time to investigate in more detail.