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

Discussion: for skinning, verts not assigned to a bone are exported wrong #1151

Closed
scurest opened this issue Jul 28, 2020 · 6 comments
Closed
Labels
bug Something isn't working exporter This involves or affects the export process Skinning_&_Rigging

Comments

@scurest
Copy link
Contributor

scurest commented Jul 28, 2020

In Blender, vertices that aren't assigned to any bones act as if they weren't skinned. In glTF however, every vert needs to be assigned to at least one bone. Currently these verts are being exported with zero weights, which is invalid. #1150 adds a stopgap against invalid files by assigning them to the 0th joint, but doesn't fix the real issue.

UnassignedVertTest.zip

cc

^^^ The two lower verts on the quad are unassigned.

Solution

To really fix this, we need to insert a new bone into the armature at the root with the default TRS. Any vert assigned to this bone would act as if it weren't skinned, so let's provisionally call this a "neutral bone". Then we just need to treat unassigned verts as if they were assigned to the neutral bone.

dd

We also only want to insert the neutral bone when necessary, ie. when there are unassigned verts.

Alternatives

I believe this (or minor variations on this) are the only possible solution. We obviously need a new bone to assign these verts to. The only real variation is where to put the neutral bone (as a new root of all the other bone trees, as the root of a new parallel one-bone tree).

We could also try to detect if an existing bone could serve as the neutral bone. This would be nice because files that happened to be fixed by #1150 like the tree from #308 wouldn't need to get a new bone. It's kind of ambitious though, so maybe a future enhancement.

The other thing we could do is declare this unsupported and print a warning for unassigned verts. We could probably avoid the warning in some cases (again, assuming we can detect an existing neutral bone). This is nice because its simpler.

@scurest
Copy link
Contributor Author

scurest commented Jul 28, 2020

So, how to actually implement the solution?

Prototype patch (against 6786505)
diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_extract.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_extract.py
index c605a60..e296d00 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_extract.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_extract.py
@@ -185,6 +185,8 @@ def extract_primitives(glTF, blender_mesh, library, blender_object, blender_vert
                 joint_name_to_index = {joint.name: index for index, joint in enumerate(skin.joints)}
                 group_to_joint = [joint_name_to_index.get(g.name) for g in blender_vertex_groups]
 
+                needs_neutral_bone = False
+
                 # Find out max number of bone influences
                 for blender_polygon in blender_mesh.polygons:
                     for loop_index in blender_polygon.loop_indices:
@@ -286,6 +288,10 @@ def extract_primitives(glTF, blender_mesh, library, blender_object, blender_vert
                         bones.append((joint, weight))
                 bones.sort(key=lambda x: x[1], reverse=True)
                 bones = tuple(bones)
+                if not bones:
+                    # Assign unassigned verts to a neutral bone (inserted later).
+                    needs_neutral_bone = True
+                    bones = ((len(skin.joints), 1.0),)
                 vert += (bones,)
 
             for shape_key in shape_keys:
@@ -302,6 +308,10 @@ def extract_primitives(glTF, blender_mesh, library, blender_object, blender_vert
             vert_idx = prim.verts.setdefault(vert, len(prim.verts))
             prim.indices.append(vert_idx)
 
+    if armature and needs_neutral_bone:
+        skin.__needs_neutral_bone = True
+        skin.__arma_name = armature.data.name
+
     #
     # Put the verts into attribute arrays.
     #
diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather.py
index 8d9e5e4..29612f5 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather.py
@@ -61,6 +61,8 @@ def __gather_scene(blender_scene, export_settings):
             if node is not None:
                 scene.nodes.append(node)
 
+    gltf2_blender_gather_nodes.add_neutral_bones_to_node_tree(scene.nodes, export_settings)
+
     export_user_extensions('gather_scene_hook', export_settings, scene, blender_scene)
 
     return scene
diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_joints.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_joints.py
index dff55d1..9e4a9fe 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_joints.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_joints.py
@@ -92,7 +92,42 @@ def gather_joint(blender_object, blender_bone, export_settings):
 
     return node
 
+
 def __gather_extras(blender_bone, export_settings):
     if export_settings['gltf_extras']:
         return generate_extras(blender_bone.bone)
     return None
+
+
+def create_neutral_joint(export_settings):
+    # Create neutral joint; acts like a root bone with identity TRS.
+    # Assigning verts to this joint should act as if they weren't skinned.
+
+    axis_basis_change = mathutils.Matrix.Identity(4)
+    if export_settings[gltf2_blender_export_keys.YUP]:
+        axis_basis_change = mathutils.Matrix(
+            ((1.0, 0.0, 0.0, 0.0), (0.0, 0.0, 1.0, 0.0), (0.0, -1.0, 0.0, 0.0), (0.0, 0.0, 0.0, 1.0)))
+
+    trans, rot, sca = axis_basis_change.decompose()
+    translation, rotation, scale = (None, None, None)
+    if trans[0] != 0.0 or trans[1] != 0.0 or trans[2] != 0.0:
+        translation = [trans[0], trans[1], trans[2]]
+    if rot[0] != 1.0 or rot[1] != 0.0 or rot[2] != 0.0 or rot[3] != 0.0:
+        rotation = [rot[1], rot[2], rot[3], rot[0]]
+    if sca[0] != 1.0 or sca[1] != 1.0 or sca[2] != 1.0:
+        scale = [sca[0], sca[1], sca[2]]
+
+    return gltf2_io.Node(
+        camera=None,
+        children=[],
+        extensions=None,
+        extras=None,
+        matrix=None,
+        mesh=None,
+        name='Neutral',
+        rotation=rotation,
+        scale=scale,
+        skin=None,
+        translation=translation,
+        weights=None,
+    )
diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_nodes.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_nodes.py
index b09e7aa..fe58193 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_nodes.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_nodes.py
@@ -88,6 +88,10 @@ def __gather_node(blender_object, library, blender_scene, dupli_object_parent, e
             node.children.append(correction_node)
         node.camera = None
 
+    # Used by add_neutral_bones_to_node_tree to find armatures
+    if blender_object.type == "ARMATURE":
+        node.__arma_name = blender_object.data.name
+
     export_user_extensions('gather_node_hook', export_settings, node, blender_object)
 
     return node
@@ -494,3 +498,36 @@ def __get_correction_node(blender_object, export_settings):
         translation=None,
         weights=None
     )
+
+
+def add_neutral_bones_to_node_tree(roots, export_settings):
+    # For skinning: glTF requires all verts be assigned to at least one
+    # bone. In Blender, unassigned verts act as if they weren't skinned. To
+    # handle this, unassigned verts are assigned to a "neutral bone" that has
+    # no transform.
+    #
+    # This function makes a post-processing pass over the nodes, inserting
+    # neutral bones anywhere they were marked as needed.
+
+    # Find what needs a neutral bone
+    skins = set()
+    armas = {}
+    def visit(node):
+        nonlocal skins
+        nonlocal armas
+        if node.skin is not None:
+            if getattr(node.skin, '__needs_neutral_bone', False):
+                skins.add(node.skin)
+        if hasattr(node, '__arma_name'):
+            armas[node.__arma_name] = node
+        for child in node.children or []:
+            visit(child)
+    for root in roots:
+        visit(root)
+
+    for skin in skins:
+        arma_node = armas[skin.__arma_name]
+        joint_node = gltf2_blender_gather_joints.create_neutral_joint(export_settings)
+        arma_node.children.append(joint_node)
+        gltf2_blender_gather_skins.add_neutral_bone_to_skin(skin, joint_node, export_settings)
+        skin.__needs_neutral_bone = False
diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_skins.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_skins.py
index 7f64527..c58fdfd 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_skins.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_skins.py
@@ -44,6 +44,9 @@ def gather_skin(blender_object, export_settings):
         skeleton=__gather_skeleton(blender_object, export_settings)
     )
 
+    # Used by add_neutral_bone_to_skin
+    skin.__matrix_world = blender_object.matrix_world
+
     export_user_extensions('gather_skin_hook', export_settings, skin, blender_object)
 
     return skin
@@ -188,3 +191,29 @@ def get_bone_tree(blender_dummy, blender_object):
     list_ = list(set(bones))
     root_ = list(set(root_bones))
     return [blender_object.data.bones[b] for b in list_], children, [blender_object.pose.bones[b] for b in root_]
+
+
+def add_neutral_bone_to_skin(skin, node, export_settings):
+    skin.joints.append(node)
+    skin.inverse_bind_matrices.count += 1
+
+    # Calculate inverse bind of neutral bone
+    axis_basis_change = mathutils.Matrix.Identity(4)
+    if export_settings[gltf2_blender_export_keys.YUP]:
+        axis_basis_change = mathutils.Matrix(
+            ((1.0, 0.0, 0.0, 0.0), (0.0, 0.0, 1.0, 0.0), (0.0, -1.0, 0.0, 0.0), (0.0, 0.0, 0.0, 1.0)))
+    inverse_bind_matrix = (
+        axis_basis_change @
+        skin.__matrix_world
+    ).inverted()
+    # flatten the matrix
+    data = []
+    for column in range(0, 4):
+        for row in range(0, 4):
+            data.append(inverse_bind_matrix[row][column])
+
+    inv_matrix_bin = gltf2_io_binary_data.BinaryData.from_list(data, gltf2_io_constants.ComponentType.Float)
+    skin.inverse_bind_matrices.buffer_view = gltf2_io_binary_data.BinaryData(
+        skin.inverse_bind_matrices.buffer_view.data +
+        inv_matrix_bin.data
+    )

The implementation is kind of strange though.

We find out if we need a neutral bone in extract_primitive. We can't add the neutral bone there though, so we just put a mark (__needs_neutral_bone) on the skin that we needed it and go ahead and act as if it were there.

Then after a whole scene has been collected, we make a second pass over the nodes with add_neutral_bones_to_node_tree. It looks for skins with that mark and their armature nodes. Then it creates a new Node for the neutral bone, inserts it into the arma node's child list, inserts it into the skin's joint list, and adds a new inverse bind matrix for it to the skin.

This is kinda unusual and nothing else in the exporter works like this, but I can't really think of how else to do it.

What does everyone think?

@julienduroure
Copy link
Collaborator

Approve #1150 for 2.90, and let's discuss the solution for 2.91
Your solution about creating a neutral bone when needed was what I had in mind when discussing #308, so let's go with this solution.
I will test your prototype soon. Thanks!

@julienduroure julienduroure added bug Something isn't working exporter This involves or affects the export process Skinning_&_Rigging labels Aug 8, 2020
@fire
Copy link

fire commented Dec 19, 2020

Poke. What is the status of this?

@scurest
Copy link
Contributor Author

scurest commented Dec 19, 2020

I'm not planning on working on this.

@scurest
Copy link
Contributor Author

scurest commented Jan 12, 2021

Think it would be better to add the armature node to the skin.joints array as the "neutral bone" (when necessary) and not add a new node. But I have no idea how to do this because the gather_node code is so incredibly convoluted.

@fire
Copy link

fire commented Jan 29, 2022

@julienduroure Can you prioritize this? I guess for 3.2? I don't know the roadmap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working exporter This involves or affects the export process Skinning_&_Rigging
Projects
None yet
Development

No branches or pull requests

3 participants