Import: fix skinning (and other node hierarchy problems)

[scurest]

Very large patch, but it's a massive improvement for all node hierarchy problems, most notably skinning.

Fixes every file I tested ＼（Ｔ∇Ｔ）／

Fixes #239.
Fixes #257.
Fixes #483.
Fixes #509.
Fixes #698.
Fixes #699.
Fixes #700.
Fixes #703.
Fixes #847.
Fixes #848.
Fixes #850.
Also fixes the model from #251.

Super big thanks to @fire btw for filing all the issues with test models <3 They were really invaluable, thanks!

Example before/after (#703)

Virtual nodes

The main piece of machinery added is an intermediate graph of "virtual" nodes, or vnodes. The glTF node graph is copied to a vnode graph, and a series of passes are done to transform it into a form we can import into Blender. This gives us a lot of flexibility in doing "global" transforms to the node graph before we commit to it by creating real Blender nodes for everything.

Vnodes are stored in a flat dict and referenced by keys just like glTF nodes are stored in a flat array and referenced by indices. Integer keys correspond to vnodes that came from glTF nodes. A root node is always inserted with key 'root'.

Summary of the vnode passes

Here's a visualization of how the graph gets transformed from "glTF form" into "Blender form"

1. A dummy root is added to turn the forest into a tree. This simplifies a lot of things. The dummy root doesn't actually get created in Blender though.
2. All nodes used by joints are turned into bones and their common ancestor is marked as an armature. (Assume the colored nodes were used by joints.)
3. The skinned mesh is moved to become a child of the armature that modifies it.
4. The node with both a mesh and camera has the camera split off into a separate node.

Joints, skins, and armatures

In master, the importer believes that every node is used by at most one joint, and it uses the inverse bind matrix of that joint to set the bone's local-to-parent. This is obviously not viable if there is not a unique joint that uses a node.

This PR changes our attitude about bones and joints. The importer now thinks each bone should have the same world transform it has in glTF (no dependence on the inverse binds). It's also no longer correct to think of skins as imported as armatures; instead a vnode pass marks nodes that should be armatures or bones for the sole purpose of satisfying the following Blender constraint

The nodes used to skin a mesh must all be bones, and must be proper descendants of a common node that is an armature.

There's no problem with overlapping skins if we think about it this way.

Skinned meshes

Where in the glTF node graph a skinned mesh is instantiated doesn't affect its transform; only the nodes used by its joints do. So we always move a skinned mesh from wherever it was in glTF to (a fresh child of) the armature that contains its bones. The fresh child has the identity transform, so the skinned mesh has the same world transform as the armature, and then there's a clear path bones-to-arma, then arma-to-world, where the mesh isn't affected by anything else.

We also need to calculate the positions of the vertices in the bind pose ie. the pose the edit bones are in. This might not be the same as the glTF, mainly because glTF nodes can be scaled but edit bones can't. Skinning code for the verts was added to primitive creation. This is the place the inverse bind matrices are now used.

Animation

Not much to say here, but it was updated to use vnode ids. The bone code has become simpler; there's a straight-forward formula for the TRS of the pose bones, which is nice.

We do have to be cognizant of objects parented to a bone (we need to translate them back to the head of the bone since they're at the tail by default).

Things I dropped

These are things I stripped out that I'd like to address later

• All scene/collection handling was dropped. The glTF scenes are ignored and the whole node forest is imported into the current Blender scene. This is not actually a big difference from before; in 2.8 everything already went into the current scene.
• We no longer make any attempt to get rid of the Yup2Zup node. It will be much simpler and more foolproof to convert coordinates whenever we read them out of the glTF file, which will get rid of it for good.
• The stuff for selecting objects/setting the active object was dropped.
• The stuff for making bone chains was dropped. You can't change the bone length after import because it has significance in the transform hierarchy (it affects objects parented to the bone). It should be possible to add a vnode pass to rotate/resize bones (see Imported armature issues #324) after this.

[fire]

Thanks for the callout. I was getting frustrated.

What's a good way to test this?

[fire]

I tested #850 and it worked. Thank you so much.

💯

[julienduroure]

Hello @scurest
Thanks for this huge PR.
I didn't go deeply into it yet, but here are some general thoughts (meanly in things you currently dropped):

First, I would like to include guys from sketchfab into testing process. @AurL Aurélien, can you please have a look into it, or forward it to the new responsible og glTF in Sketchfab?

All scene/collection handling was dropped. The glTF scenes are ignored and the whole node forest is imported into the current Blender scene. This is not actually a big difference from before; in 2.8 everything already went into the current scene.

One possible solution is to import all in current scene, and to create (if needed) some collection(s) to reflect glTF scene. That way, no issue with glTF node in multiple glTF scenes, because all Blender objects will be in same scene, but collections can be a good way to display glTF scenes. This can be done later, not directly linked to this PR.

We no longer make any attempt to get rid of the Yup2Zup node. It will be much simpler and more foolproof to convert coordinates whenever we read them out of the glTF file, which will get rid of it for good.

Using debug values to get rid of Yup2Zup is part of the production pipeline in Airbus DS. So this has to be restored, we can't drop it.

The stuff for selecting objects/setting the active object was dropped.

Same here, Airbus DS need to have the root of imported glTF active at end of import. This need to be restored.

The stuff for making bone chains was dropped. You can't change the bone length after import because it has significance in the transform hierarchy (it affects objects parented to the bone). It should be possible to add a vnode pass to rotate/resize bones (see #324) after this.

Yes, confirmed. We can drop it for now (and we need to think how we can manage it later).

[scurest]

Thanks for looking @julienduroure. I know it's huge ><

The active object is easy. Could you clarify what is supposed to be selected though? In master it depends on a lot of different things (like whether there are animations and what the debug_value is).

Should I just select all (and only) the imported objects?

[scurest]

I made the first object in the default glTF scene active and selected. Tell me if you wanted something else.

I also pushed the code for getting rid of the Yup2Zup node by hard-applying the conversion whenever we read. You'll notice that the direction bones point changes and correction nodes are inserted for cameras/lights (just like how the exporter works). I can change these in a later PR (I have the code for it written already).

[julienduroure]

Regarding active object:
At end of import, the active object must be the root object just imported. In most cases, this Blender Object is the glTF tree root (but can be different because of armature). This should be simpler to find it in vnode tree. If there are multiple root objects, pick (randomly) one to be active. Regarding scenes : if there is a scene property in glTF file, use this scene. If there is no scene property, use the scenes[0] scene.

Regarding Yup2Zup node : No selection is required at end of import.

Current rules :

• We don't want to convert Yup to Zup:
  • Because user don't want it (debug = 100)
• We want to convert Yup to Zup, and we will NOT remove it at end of process
  • Because there is some animation on object (check on gltf.animation_object in code), we can't remove it. So the process is to check all imported objected without any parent (most of the time these are root object in glTF tree, but it can be different because of armature), and set all these objects parent to Yup2Zup empty.
• We want to convert Yup to Zup, and this empty will be removed at end of process
  • When there is no animation on object. In that case, We select all root objects (once again, this is most of the time root glTF nodes, but not always), and then apply transforms on all selected objects, and then remove Yup2Zup node

[julienduroure]

@scurest Seems we updated this thread in same time :)
I will have a look on your proposal for active/selection and for Yup2Zup removal.
First remark, as I wrote in my previous post that I wrote during your update: No selection is needed at end of import, only active object is really required. So I think you can remove the selection part (we will save some time).

[scurest]

We did :) I amended the last commit to set the active object like you said and not select anything.

[julienduroure]

Can you please also restore log on debug 101 ?

[scurest]

Sure. I have it print the vnode id now.

[fire]

@scurest I've added another test case. #862

[scurest]

#862 occurs when an armature object has both its loc/rot/scale animated, and also has pose bone animations, and two actions would get created. That didn't ever used to happen since the importer appears to create a new node for the armature that wasn't in the glTF node tree, so it's loc/rot/scale couldn't be animated. In this branch, an existing node is turned into the armature (except when that's not possible and the dummy root is turned into an armature).

So the animation_node code now uses the same cache as the animation_bone code to be sure only one action is created.

Here you can see the difference in the node tree, master/this branch. You can see the extra "6" node in master.

Thanks for catching that @fire :)

[fire]

Is this targeted for 2.82? https://developer.blender.org/project/view/89/

[julienduroure]

@fire No, BCon3 is tomorrow, I don't have time to test it heavily. This is a very good improvement (thanks again scurest :) ), but this is quite a big change, so we need time to test it carefully.
So this will go with 2.83

[scurest]

@julienduroure How's this coming?

[julienduroure]

Tests are in progress.
Target is definitively to merge it as soon as possible, probably in the next two weeks

[julienduroure]

Tests that I/we want/need to perform:

• Airbus DS non regression tests
• Some armature tests ( Cesium man / monster, ...)
• Spot light orientation
• Camera orientation
• no scene in gltf file tests

[scurest]

• I tested CesiumMan, Monster, and Brainstem.

  You might have noticed there's actually some special handling in add_primitive_to_bmesh to keep CesiumMan working because it's skin weights don't sum to 1. Anyway, it looks the same as in master.

  If you check edit mode, Brainstem and Monster are actually both improved by this branch.

  
  

• For light orientation, I made a spotlight whose line-of-sight passes through a plane, exported it, and re-imported it. This is affected by whether we're doing Yup2Zup. It works both ways.

  

• For camera orientation, I used the Cameras sample model. This is affected by whether we're doing Yup2Zup. It works both ways.

  

• I tested no scenes by just deleting the scene and scenes property from CesiumMan and loading.

[scurest]

Here's an example of how multi-type nodes get resolved.

The Top node has both a mesh and a camera. In master the camera gets lost.

[julienduroure]

Thanks a lot @scurest for this huge improvement !

[donmccurdy]

These changes look excellent, thank you @scurest!