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

Skin/armature export produces NODE_SKINNED_MESH_NON_ROOT #1626

Open
emackey opened this issue Apr 14, 2022 · 8 comments · May be fixed by #1627
Open

Skin/armature export produces NODE_SKINNED_MESH_NON_ROOT #1626

emackey opened this issue Apr 14, 2022 · 8 comments · May be fixed by #1627

Comments

@emackey
Copy link
Member

emackey commented Apr 14, 2022

There's been discussion of this before, for example #1538 (comment)

So perhaps GLTF exporter should apply transforms and clear skinned mesh parent during export.

I've come to understand the correct fix is to NOT apply transforms to the skinned mesh. The design of glTF's skinning system is such that any transforms applied to the skinned mesh itself are completely ignored, only joint transforms are considered. Specifically, the final location of each vertex coming out of a glTF skinned vertex shader has influence from joints adding up to 100% strength, with exactly 0% coming from the mesh's own model matrix.

So the correct answer here sounds pretty reckless, but it's true: Just clear the parent of the skinned mesh. As in, just lose the reference to the parent. Don't apply any of the parent's transforms or animations (and you may also clear any transforms or animations that target the skinned mesh itself; they're unused as well).

Essentially glTF's skinning system treats the mesh as always being at the origin, and uses the joints (not parent node transforms) to move each vertex where it belongs. The joints' transforms do the work of inheriting from their own parents, so these are not lost.

@julienduroure
Copy link
Collaborator

As said in comment, now vtree is merged, this change is now quite easy.
PR still need to be fully tested, to be sure this is regression free for all cases.

@donmccurdy
Copy link
Contributor

donmccurdy commented Apr 14, 2022

Don't apply any of the parent's transforms or animations ...

Understanding that applying transforms to the skinned mesh node (as opposed to the armature / joints) does not have an effect in glTF ... don't we still need to apply those transforms somewhere, if they were contributing to the final position of the vertices in Blender? Applying the transform to the inverse bind matrices might be the best approach if that's the case.

@scurest
Copy link
Contributor

scurest commented Apr 14, 2022

Whether the exporter handles skinned meshes correctly is a separate question, since obviously correctness is unchanged by moving skinned meshes to root nodes. The only goal of moving it would be to get rid of NODE_SKINNED_MESH_NON_ROOT warnings in the validator.

@emackey
Copy link
Member Author

emackey commented Apr 14, 2022

don't we still need to apply those transforms somewhere, if they were contributing to the final position of the vertices in Blender?

I think @scurest's understanding here is deeper than mine, but my own take on this is that Blender's insistence on placing the mesh as a child of the armature is to treat the mesh's transform as kind of an unspoken "root bone" of sorts when no other bones have influence. But glTF insists on each vertex having 100% control (no more, no less) from added-up bone weights, so the "root" bone (if there is one) must be explicit, not implicit. So... no mesh transform influence needed.

The only goal of moving it would be to get rid of NODE_SKINNED_MESH_NON_ROOT warnings in the validator.

That's correct, but the validator doesn't issue this warning frivolously. It indicates that client implementations may be wasting GPU cycles applying and then inverting the mesh's node transform hierarchy. So in that light this is... a teeny performance gain, perhaps? For some client implementations at least. Still users seem more vocal about the warning than the performance, as the latter is nearly undetectable.

@fenhelix
Copy link

Apologies if it's been a while in this issue, but I'm working on some asset conversion tooling for VisionOS

The only goal of moving it would be to get rid of NODE_SKINNED_MESH_NON_ROOT warnings in the validator.

@scurest @emackey I disagree with this. The option to set the skinned mesh as a root is essential for certain animation production pipelines.

In particular, tools from Apple's and Pixar's USD pipelines expect the mesh to not be a child of the armature when converting to USDZ from glb/gltf.

If you try and pass an asset with a NODE_SKINNED_MESH_NON_ROOT error, it prevents the animation from playing in certain Apple environments such as Reality Composer Pro.

I will submit appropriate feedback to Apple, however, the option to have this as a flag would be extremely handy 👍

FYI, the reasoning for not exporting directly from Blender to .usd* is due to the lack of support for USD_Skel in Blender's export.

Apple's developer tools are however able to convert glb/gltf with animated skeletons. Unfortunately, certain tools from Apple/Pixar will not detect animations unless NODE_SKINNED_MESH_NON_ROOT is resolved.

@emackey
Copy link
Member Author

emackey commented Jan 3, 2024

however, the option to have this as a flag would be extremely handy

Yes, we should fix this, so skinned meshes are always root nodes in glTF.

But we don't need a flag for it. We don't need to ask the user if they want it broken or fixed. Once fixed it should just stay fixed.

@prominentdetail
Copy link

What if you need the skinned mesh nested inside other objects? How can it be root in that case?

@emackey
Copy link
Member Author

emackey commented Jan 5, 2024

What if you need the skinned mesh nested inside other objects? How can it be root in that case?

Then the mesh's skeleton must be nested inside the other object, not the actual skinned mesh. In glTF, all location information comes from the skeleton's node transforms, not from the skinned mesh's node transform.

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 a pull request may close this issue.

6 participants