FBX IO: Fix error importing BlendShapeChannels with extraneous FullWeights #104956

Merged
Thomas Barlow merged 6 commits from Mysteryem/blender-addons:fbx_fix_extra_fullweights_error into main 2023-11-05 10:16:56 +01:00
Showing only changes of commit 9ff5b81433 - Show all commits

View File

@ -1928,7 +1928,11 @@ def blen_read_shapes(fbx_tmpl, fbx_data, objects, me, scene):
# will be clamped, and we'll print a warning message to the console.
shape_key_values_in_range = True
bc_uuid_to_keyblocks = {}
for bc_uuid, fbx_sdata, fbx_bcdata in fbx_data:
for bc_uuid, fbx_sdata, fbx_bcdata, shapes_assigned_to_channel in fbx_data:
num_shapes_assigned_to_channel = len(shapes_assigned_to_channel)
if num_shapes_assigned_to_channel > 1:
# See bug report #84111 and design task #104698.
raise RuntimeError("FBX in-between Shapes are not currently supported")
elem_name_utf8 = elem_name_ensure_class(fbx_sdata, b'Geometry')
indices = elem_prop_first(elem_find_first(fbx_sdata, b'Indexes'))
dvcos = elem_prop_first(elem_find_first(fbx_sdata, b'Vertices'))

The short of it is that while we could allow each in-between shape to be imported as a separate shape key right now, any imported animations of the DeformPercent of the BlendShapeChannel the Shapes belong to will not work correctly and the FullWeights value for each Shape will be lost. I think the minimum for in-between shapes support would be importing them and their animations correctly.

Maybe the error message should include text that makes it clear this is a known issue and doesn't need to be reported.

The short of it is that while we could allow each in-between shape to be imported as a separate shape key right now, any imported animations of the DeformPercent of the BlendShapeChannel the Shapes belong to will not work correctly and the FullWeights value for each Shape will be lost. I think the minimum for in-between shapes support would be importing them and their animations correctly. Maybe the error message should include text that makes it clear this is a known issue and doesn't need to be reported.
@ -1943,22 +1947,35 @@ def blen_read_shapes(fbx_tmpl, fbx_data, objects, me, scene):
dvcos = dvcos[:-remainder]
dvcos = dvcos.reshape(-1, 3)
# There must be the same number of indices as vertex coordinate differences.
assert(len(indices) == len(dvcos))
# We completely ignore normals here!
weight = elem_prop_first(elem_find_first(fbx_bcdata, b'DeformPercent'), default=100.0) / 100.0
vgweights = elem_prop_first(elem_find_first(fbx_bcdata, b'FullWeights'))
vgweights = parray_as_ndarray(vgweights) if vgweights else np.empty(0, dtype=data_types.ARRAY_FLOAT64)
# Not doing the division in-place in-case it's possible for FBX shape keys to be used by more than one mesh.
vgweights = vgweights / 100.0
# The FullWeights array stores the deformation percentages of the BlendShapeChannel that fully activate each
# Shape assigned to the BlendShapeChannel. Blender also uses this array to store Vertex Group weights, but this
# is not part of the FBX standard.
full_weights = elem_prop_first(elem_find_first(fbx_bcdata, b'FullWeights'))
full_weights = parray_as_ndarray(full_weights) if full_weights else np.empty(0, dtype=data_types.ARRAY_FLOAT64)
create_vg = (vgweights != 1.0).any()
# Special case, in case all weights are the same, FullWeight can have only one element - *sigh!*
nbr_indices = len(indices)
if len(vgweights) == 1 and nbr_indices > 1:
vgweights = np.full_like(indices, vgweights[0], dtype=vgweights.dtype)
assert(len(vgweights) == nbr_indices == len(dvcos))
# Special case for Blender exported Shape Keys with a Vertex Group assigned. The Vertex Group weights are stored
# in the FullWeights array.
# XXX - It's possible, though very rare, to get a false positive here and create a Vertex Group when we
# shouldn't. This should only happen if there are extraneous FullWeights or there is a single FullWeight
# and its value is not 100.0.
is_vertex_weights = len(full_weights) == len(indices)
# Skip creating a Vertex Group if all the weights are 100.0 because such a Vertex Group has no effect and this
# avoids creating a Vertex Group for imported Shapes that only move a single vertex because their singular

I'm not very happy about this possible false positive.
It should be noted that most software will truncate the FullWeights array to the same length as the number of Shapes, so the Blender Vertex Group weights will not often be preserved. External software can also read the Vertex Group weights as FullWeights values which can have unintended effects.

A few ideas:

  • Add a new import option that controls whether importing FullWeights as Vertex Group weights is enabled.
    • FullWeights as Vertex Groups is a really niche feature that is only really relevant for importing Blender exported .fbx back into Blender, so adding an importer option sounds like overkill.
  • Only import as Vertex Group weights when the the creator of the .fbx file is Blender.
    • If there is external software that has added support for propagating Blender Vertex Group weights, this could break support
  • Entirely remove support for exporting/importing FullWeights as Vertex Group weights
    • Just because it's a really niche feature doesn't mean nobody has been using it
    • While there's not much code to change, it's probably too big of a change for 4.0 at this point.
I'm not very happy about this possible false positive. It should be noted that most software will truncate the FullWeights array to the same length as the number of Shapes, so the Blender Vertex Group weights will not often be preserved. External software can also read the Vertex Group weights as FullWeights values which can have unintended effects. A few ideas: - Add a new import option that controls whether importing FullWeights as Vertex Group weights is enabled. - FullWeights as Vertex Groups is a really niche feature that is only really relevant for importing Blender exported .fbx back into Blender, so adding an importer option sounds like overkill. - Only import as Vertex Group weights when the the creator of the .fbx file is Blender. - If there is external software that has added support for propagating Blender Vertex Group weights, this could break support - Entirely remove support for exporting/importing FullWeights as Vertex Group weights - Just because it's a really niche feature doesn't mean nobody has been using it - While there's not much code to change, it's probably too big of a change for 4.0 at this point.

I've added an extra check that all the FullWeights are within the [0.0, 100.0] range, which prevents the false positives when importing the .fbx files in #104909

I've added an extra check that all the FullWeights are within the [0.0, 100.0] range, which prevents the false positives when importing the .fbx files in https://projects.blender.org/blender/blender-addons/issues/104909
# FullWeight is expected to always be 100.0.
if is_vertex_weights and np.any(full_weights != 100.0):
# Not doing the division in-place because it's technically possible for FBX Shape Keys to be used by more
# than one mesh, though this shouldn't be the case for Blender exported Shape Keys.
vgweights = full_weights / 100.0
else:
vgweights = None
# There must be a FullWeight for each Shape. Any excess is ignored.
assert(len(full_weights) >= num_shapes_assigned_to_channel)
# To add shape keys to the mesh, an Object using the mesh is needed.
if me.shape_keys is None:
@ -1977,7 +1994,7 @@ def blen_read_shapes(fbx_tmpl, fbx_data, objects, me, scene):
kb.value = weight
# Add vgroup if necessary.
if create_vg:
if vgweights is not None:
# VertexGroup.add only allows sequences of int indices, but iterating the indices array directly would
# produce numpy scalars of types such as np.int32. The underlying memoryview of the indices array, however,
# does produce standard Python ints when iterated, so pass indices.data to add_vgroup_to_objects instead of
@ -3508,6 +3525,11 @@ def load(operator, context, filepath="",
seen_connections.add(connection_key)
yield c_dst_uuid, fbx_data, bl_data
# XXX - Multiple Shapes can be assigned to a single BlendShapeChannel to create a progressive blend between the
# base mesh and the assigned Shapes, with the percentage at which each Shape is fully blended being stored
# in the BlendShapeChannel's FullWeights array. This is also known as 'in-between shapes'.
# We don't have any support for in-between shapes currently.
blend_shape_channel_to_shapes = {}
mesh_to_shapes = {}
for s_uuid, (fbx_sdata, _bl_sdata) in fbx_table_nodes.items():
if fbx_sdata is None or fbx_sdata.id != b'Geometry' or fbx_sdata.props[2] != b'Shape':
@ -3515,6 +3537,9 @@ def load(operator, context, filepath="",
# shape -> blendshapechannel -> blendshape -> mesh.
for bc_uuid, fbx_bcdata, _bl_bcdata in connections_gen(s_uuid, b'Deformer', b'BlendShapeChannel'):
# Track the Shapes connected to each BlendShapeChannel. Duplicates are ignored.
shapes_assigned_to_channel = blend_shape_channel_to_shapes.setdefault(bc_uuid, set())
shapes_assigned_to_channel.add(s_uuid)
for bs_uuid, _fbx_bsdata, _bl_bsdata in connections_gen(bc_uuid, b'Deformer', b'BlendShape'):
for m_uuid, _fbx_mdata, bl_mdata in connections_gen(bs_uuid, b'Geometry', b'Mesh'):
# Blenmeshes are assumed already created at that time!
@ -3534,7 +3559,10 @@ def load(operator, context, filepath="",
mesh_to_shapes[bl_mdata] = (objects, shapes_list)
else:
shapes_list = mesh_to_shapes[bl_mdata][1]
shapes_list.append((bc_uuid, fbx_sdata, fbx_bcdata))
# Only the number of shapes assigned to each BlendShapeChannel needs to be passed through to
# `blen_read_shapes`, but that number isn't known until all the connections have been
# iterated, so pass the `shapes_assigned_to_channel` set instead.
shapes_list.append((bc_uuid, fbx_sdata, fbx_bcdata, shapes_assigned_to_channel))
# BlendShape deformers are only here to connect BlendShapeChannels to meshes, nothing else to do.
# Iterate through each mesh and create its shape keys