From 9ff5b81433c3944f37a65c6246520b3353357239 Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Mon, 16 Oct 2023 05:31:22 +0100 Subject: [PATCH 1/6] Fix error importing BlendShapeChannels with extraneous FullWeights Fixes a rare cause of #84111, but does not fix the main issue. Aside from Shape Keys with Vertex Groups exported by Blender, rarely, a BlendShapeChannel can have more FullWeights than Shapes assigned to it, which would fail an assertion that the number of FullWeights equals the number of vertices moved by the Shape with an allowed exception of when there is only a single FullWeight. This assertion was not correct because each FullWeight is the deformation percentage of the BlendShapeChannel that fully activates each Shape assigned to the BlendShapeChannel. This patch keeps track of the Shapes assigned to each BlendShapeChannel so that the number of FullWeights can be correctly compared against the number of Shapes. FBX appears to ignore any excess FullWeights, so the updated assertion allows this too. Blender exported Shape Keys with Vertex Groups are then handled separately by initially checking if the number of FullWeights equals the number of vertices moved by the imported Shape. The main cause of #84111 is when there is more than one Shape assigned to a BlendShapeChannel. This is still not supported because it requires larger changes and allowing it now would cause more issues. Because the assertion is no longer sufficient to cause an error when there is more than one Shape, an explicitly thrown RuntimeException has been added to fulfil the same role. --- io_scene_fbx/import_fbx.py | 58 ++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/io_scene_fbx/import_fbx.py b/io_scene_fbx/import_fbx.py index 7cb1f3aea..7be02f6ee 100644 --- a/io_scene_fbx/import_fbx.py +++ b/io_scene_fbx/import_fbx.py @@ -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')) @@ -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 + # 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 -- 2.30.2 From 1ea4d1f1101233be7e580cb2837d3d1c509aa234 Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Fri, 20 Oct 2023 01:19:42 +0100 Subject: [PATCH 2/6] Add more checks for determining whether to import FullWeights as vertex weights This should eliminate the case of FBX from Unreal with extraneous FullWeights because those always seem to be negative for some reason. An alternative check that could be made for these extraneous Unreal FullWeights is that their absolute values seem to always be far smaller than the smallest single precision non-zero float (vertex weights are single-precision). The smallest subnormal single-precision float is 1e-45, but these extraneous FullWeights are usually around 1e-200. Though if there are systems running Blender where single precision float is 64 bits, this check wouldn't work. --- io_scene_fbx/import_fbx.py | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/io_scene_fbx/import_fbx.py b/io_scene_fbx/import_fbx.py index 7be02f6ee..a4e496f2b 100644 --- a/io_scene_fbx/import_fbx.py +++ b/io_scene_fbx/import_fbx.py @@ -1962,19 +1962,28 @@ def blen_read_shapes(fbx_tmpl, fbx_data, objects, me, scene): # 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 - # 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. + # shouldn't. This should only be possible when there are extraneous FullWeights or when there is a single + # FullWeight and its value is not 100.0. + if ( + # Blender exported Shape Keys only ever export as 1 Shape per BlendShapeChannel. + num_shapes_assigned_to_channel == 1 + # There should be one vertex weight for each vertex moved by the Shape. + and len(full_weights) == len(indices) + # Skip creating a Vertex Group when all the weights are 100.0 because such a Vertex Group has no effect. + # This also avoids creating a Vertex Group for imported Shapes that only move a single vertex because + # their BlendShapeChannel's singular FullWeight is expected to always be 100.0. + and not np.all(full_weights == 100.0) + # Blender vertex weights are always within the [0.0, 1.0] range (scaled to [0.0, 100.0] when saving to + # FBX). This can eliminate imported BlendShapeChannels from Unreal that have extraneous FullWeights + # because the extraneous values are usually negative. + and np.all((full_weights >= 0.0) & (full_weights <= 100.0)) + ): + # Not doing the division in-place because it's technically possible for FBX BlendShapeChannels to be used by + # more than one FBX BlendShape, 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. + # There must be a FullWeight for each Shape. Any extra FullWeights are ignored. assert(len(full_weights) >= num_shapes_assigned_to_channel) # To add shape keys to the mesh, an Object using the mesh is needed. -- 2.30.2 From c10fea6546a59b50b33866c58d92617209c5bc99 Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Fri, 20 Oct 2023 01:34:34 +0100 Subject: [PATCH 3/6] Include bug report comment on the same line as the exception This will make it show up in tracebacks. --- io_scene_fbx/import_fbx.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/io_scene_fbx/import_fbx.py b/io_scene_fbx/import_fbx.py index a4e496f2b..fb616145d 100644 --- a/io_scene_fbx/import_fbx.py +++ b/io_scene_fbx/import_fbx.py @@ -1931,8 +1931,8 @@ def blen_read_shapes(fbx_tmpl, fbx_data, objects, me, scene): 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") + # Relevant design task: #104698 + raise RuntimeError("FBX in-between Shapes are not currently supported") # See bug report #84111 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')) -- 2.30.2 From f1cb952249cf3a51229e72365713560674047d6a Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Fri, 20 Oct 2023 03:27:39 +0100 Subject: [PATCH 4/6] Missed comment update in the merge --- io_scene_fbx/import_fbx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io_scene_fbx/import_fbx.py b/io_scene_fbx/import_fbx.py index fb616145d..f955e3262 100644 --- a/io_scene_fbx/import_fbx.py +++ b/io_scene_fbx/import_fbx.py @@ -3546,7 +3546,7 @@ 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. + # Track the Shapes connected to each BlendShapeChannel. 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'): -- 2.30.2 From 33e1373492ca897ac508daac8686043eea2158ae Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Sun, 5 Nov 2023 08:01:35 +0000 Subject: [PATCH 5/6] Fix bad merge in rebase --- io_scene_fbx/import_fbx.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/io_scene_fbx/import_fbx.py b/io_scene_fbx/import_fbx.py index f955e3262..044f95d35 100644 --- a/io_scene_fbx/import_fbx.py +++ b/io_scene_fbx/import_fbx.py @@ -3547,8 +3547,8 @@ 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. - shapes_assigned_to_channel = blend_shape_channel_to_shapes.setdefault(bc_uuid, set()) - shapes_assigned_to_channel.add(s_uuid) + shapes_assigned_to_channel = blend_shape_channel_to_shapes.setdefault(bc_uuid, []) + shapes_assigned_to_channel.append(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! @@ -3570,7 +3570,7 @@ def load(operator, context, filepath="", shapes_list = mesh_to_shapes[bl_mdata][1] # 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. + # iterated, so pass the `shapes_assigned_to_channel` list 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. -- 2.30.2 From 4bd757f0dbef2cba79f714efa83373763fa9cf5b Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Sun, 5 Nov 2023 08:53:31 +0000 Subject: [PATCH 6/6] Increase FBX version --- io_scene_fbx/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io_scene_fbx/__init__.py b/io_scene_fbx/__init__.py index 44b2e99eb..7ab7539ad 100644 --- a/io_scene_fbx/__init__.py +++ b/io_scene_fbx/__init__.py @@ -5,7 +5,7 @@ bl_info = { "name": "FBX format", "author": "Campbell Barton, Bastien Montagne, Jens Restemeier, @Mysteryem", - "version": (5, 10, 0), + "version": (5, 10, 1), "blender": (4, 1, 0), "location": "File > Import-Export", "description": "FBX IO meshes, UVs, vertex colors, materials, textures, cameras, lamps and actions", -- 2.30.2