FBX Import: Speed up shape keys with numpy #104491

Thomas Barlow · 2023-03-18T02:22:26+01:00

Thomas Barlow commented

2023-03-18 02:22:26 +01:00

Move vg.add outside the hot loop when adding vertex groups for shape key weights. This will also slightly speed up the import of bone weights since that uses the same function.

Group shape keys by mesh so that each mesh only needs to be processed once. This is important for the new code, since it has an upfront cost per mesh. Without the grouping, the same individual cost would be per shape key instead.

The greater the percentage of the mesh that a shape key moves, the greater this patch's speedup.
If the number of indices of an imported shape key is less than about 2% the number of vertices of the mesh, this patch can end up slower due to every vertex in the shape key being set, not only those moved by the imported shape key.
For many shape keys that move only a small portion of the mesh, e.g. character models with shape keys for facial animations, the speedup is often only around 1-3 times and can be slightly slower in some cases.
It's possible to modify the original code to iterate more efficiently, at which point the threshold would change to being less than about 7% the number of vertices.

Move vg.add outside the hot loop when adding vertex groups for shape key weights. This will also slightly speed up the import of bone weights since that uses the same function. Group shape keys by mesh so that each mesh only needs to be processed once. This is important for the new code, since it has an upfront cost per mesh. Without the grouping, the same individual cost would be per shape key instead. The greater the percentage of the mesh that a shape key moves, the greater this patch's speedup. If the number of indices of an imported shape key is less than about 2% the number of vertices of the mesh, this patch can end up slower due to every vertex in the shape key being set, not only those moved by the imported shape key. For many shape keys that move only a small portion of the mesh, e.g. character models with shape keys for facial animations, the speedup is often only around 1-3 times and can be slightly slower in some cases. It's possible to modify the original code to iterate more efficiently, at which point the threshold would change to being less than about 7% the number of vertices.

Thomas Barlow requested review from Bastien Montagne 2023-03-18 02:33:01 +01:00

Bastien Montagne reviewed 2023-03-20 18:04:33 +01:00

io_scene_fbx/import_fbx.py Outdated

						
				@ -1401,3 +1428,2 @@

				        for idx, co in vcos:

				            kb.data[idx].co[:] = co

				        # When only a small part is affected by the shape key it can be faster to index each co than to use foreach_set.

Bastien Montagne commented

2023-03-20 18:04:28 +01:00

Am wondering about how much speed differences we are talking about here... because this double codepath handling for a same thing has a significant impact on readability and maintainability, so would only consider it necessary if we are talking about real use-cases where the difference would be in tens of seconds on edge-cases, or at least seconds in more common cases?

Thomas Barlow reviewed 2023-03-21 05:56:11 +01:00

io_scene_fbx/import_fbx.py Outdated

						
				@ -1401,3 +1428,2 @@

				        for idx, co in vcos:

				            kb.data[idx].co[:] = co

				        # When only a small part is affected by the shape key it can be faster to index each co than to use foreach_set.

Thomas Barlow commented

2023-03-21 05:55:24 +01:00

Edge-cases only in the low single digit seconds slower I would say.
A 500,000 vertex .fbx with 60 shape keys that move only a single vertex each is about 1.9s slower to import without the iteration for me.

If getting rid of the iteration then I would add in a quick dvcos.any() check to only actually do the main work when the shape key contains at least one non-zero dvcos. I probably should have done that to begin with.

Edge-cases only in the low single digit seconds slower I would say. A 500,000 vertex .fbx with 60 shape keys that move only a single vertex each is about 1.9s slower to import without the iteration for me. If getting rid of the iteration then I would add in a quick `dvcos.any()` check to only actually do the main work when the shape key contains at least one non-zero dvcos. I probably should have done that to begin with.

Bastien Montagne requested changes 2023-03-21 12:12:48 +01:00

Bastien Montagne left a comment

Patch look good, would rather have the 'utils' part of it (shared with some other patches) either put in a single one, or even in a separate PR to help with the merging process.

Also think that the imports 'where it's used' can be ignored here, this is supposed to be a startup time optimization (to avoid blender's python to import everything when starting Blender), but in practice doubt this is giving any real improvements here, since numpy is also imported in fbx_utils etc.

Patch look good, would rather have the 'utils' part of it (shared with some other patches) either put in a single one, or even in a separate PR to help with the merging process. Also think that the `import`s 'where it's used' can be ignored here, this is supposed to be a startup time optimization (to avoid blender's python to import everything when starting Blender), but in practice doubt this is giving any real improvements here, since numpy is also imported in `fbx_utils` etc.

io_scene_fbx/import_fbx.py Outdated

						
				@ -1401,3 +1428,2 @@

				        for idx, co in vcos:

				            kb.data[idx].co[:] = co

				        # When only a small part is affected by the shape key it can be faster to index each co than to use foreach_set.

Bastien Montagne commented

2023-03-21 12:08:57 +01:00

Thanks, then indeed I think keeping a single codepath here is better overall.

Thomas Barlow force-pushed fbx_import_shape_keys_np_pr from 413a5d98fd to 849751471b

2023-03-22 05:37:47 +01:00

Compare

Thomas Barlow force-pushed fbx_import_shape_keys_np_pr from 849751471b to bccbd85735

2023-03-22 07:01:57 +01:00

Compare

Thomas Barlow commented

2023-03-22 07:08:59 +01:00

The imports and shared parray_as_ndarray utility function have been moved to a separate commit that has its own PR: #104499

I updated the commit message for the effect of removing the iteration codepath.

The imports and shared `parray_as_ndarray` utility function have been moved to a separate commit that has its own PR: https://projects.blender.org/blender/blender-addons/issues/104499 I updated the commit message for the effect of removing the iteration codepath.

Thomas Barlow requested review from Bastien Montagne 2023-03-22 07:09:09 +01:00

Bastien Montagne force-pushed fbx_import_shape_keys_np_pr from bccbd85735 to 66390ced12

2023-04-03 14:43:25 +02:00

Compare

Bastien Montagne approved these changes 2023-04-03 14:48:54 +02:00

Bastien Montagne merged commit 66390ced12 into main

2023-04-03 14:49:04 +02:00

Bastien Montagne deleted branch fbx_import_shape_keys_np_pr

2023-04-03 14:49:04 +02:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

FBX Import: Speed up shape keys with numpy #104491