FBX IO: Speed up animation import using NumPy #104856

Thomas Barlow · 2023-08-31T18:47:44+02:00

Thomas Barlow commented

2023-08-31 18:47:44 +02:00

This patch updates much of the animation import code to replace
iteration with NumPy vectorized functions and can result in a
significant speedup for files that are mostly animation.

An FBX file containing an armature with 65 bones and a single animation
with 1523 frames (each one keyframed) animating 47 bones (144 animated
channels in the FBX file) goes from a 1.96s import to a 1.01s import for
me with this patch.

The majority of the remaining animation import time after this patch is
spent converting FBX transformation animations to Blender transformation
animations. Much of this can also be sped up with NumPy, but that will
be submitted separately once it is finished. For this reason, the new
_transformation_curves_gen function is only intended as an interim to
facilitate this patch until the transformation animation conversion is
also updated to use NumPy.

Changes to the import of valid animations:

Previously, all imported animation curves for a single Action would
interpolate the keyframes of every other animation curve imported for
that action. This is needed for imported Object/PoseBone transformation
animations, but is unnecessary for other imported animations.

This was unlikely to be noticed because it only affected material color
animations and animations that affected both the lens and
focus_distance properties of a camera.

Now with this patch, only the transformation animation curves
interpolate the keyframes of every other transform animation curve.

This change is a requirement to be able to support importing other
animation curves in the future, such as those for custom properties.

Changes to the import of questionably valid animations:

FBX's default animation system only uses the first animation curve
assigned to a property's channel, but the specification does allow for
more than one curve per channel.

Previously, Blender would combine these curves together, but in such a
way that each additional imported curve would replace all the values of
prior imported curves at times less than the maximum keyframe time of
the additional imported curve (interpolating values for the keyframes
that don't exist in the additional imported curve).

Now when there are multiple animation curves assigned to a channel, the
additional imported curves only replace values at their keyframe times,
no interpolation for other keyframe times takes place.

Changes to the import of invalid animations:

An animation curve is invalid if its keyframe times are not strictly
increasing. Some imported invalid curves result in extrapolating values.
These extrapolated values are still discarded with this patch, but
previously they would be replaced with a default value. With this patch,
the extrapolated values and their keyframe times are discarded entirely.

This patch updates much of the animation import code to replace iteration with NumPy vectorized functions and can result in a significant speedup for files that are mostly animation. An FBX file containing an armature with 65 bones and a single animation with 1523 frames (each one keyframed) animating 47 bones (144 animated channels in the FBX file) goes from a 1.96s import to a 1.01s import for me with this patch. The majority of the remaining animation import time after this patch is spent converting FBX transformation animations to Blender transformation animations. Much of this can also be sped up with NumPy, but that will be submitted separately once it is finished. For this reason, the new `_transformation_curves_gen` function is only intended as an interim to facilitate this patch until the transformation animation conversion is also updated to use NumPy. # Changes to the import of valid animations: Previously, all imported animation curves for a single Action would interpolate the keyframes of every other animation curve imported for that action. This is needed for imported Object/PoseBone transformation animations, but is unnecessary for other imported animations. This was unlikely to be noticed because it only affected material color animations and animations that affected both the `lens` and `focus_distance` properties of a camera. Now with this patch, only the transformation animation curves interpolate the keyframes of every other transform animation curve. This change is a requirement to be able to support importing other animation curves in the future, such as those for custom properties. # Changes to the import of questionably valid animations: FBX's default animation system only uses the first animation curve assigned to a property's channel, but the specification does allow for more than one curve per channel. Previously, Blender would combine these curves together, but in such a way that each additional imported curve would replace all the values of prior imported curves at times less than the maximum keyframe time of the additional imported curve (interpolating values for the keyframes that don't exist in the additional imported curve). Now when there are multiple animation curves assigned to a channel, the additional imported curves only replace values at their keyframe times, no interpolation for other keyframe times takes place. # Changes to the import of invalid animations: An animation curve is invalid if its keyframe times are not strictly increasing. Some imported invalid curves result in extrapolating values. These extrapolated values are still discarded with this patch, but previously they would be replaced with a default value. With this patch, the extrapolated values and their keyframe times are discarded entirely.

👍 1 ❤️ 1 🚀 1

Thomas Barlow added 11 commits 2023-08-31 18:47:47 +02:00

2ffbeeb8e7 Use only the first anim curve per property channel

The FBX SDK documentation specifies that the FBX animation system's default implementation only uses the first curve assigned to a channel. Additional curves per channel are supported by the FBX specification, but the handling of these extra curves is considered the responsibility of the application that created them.

This patch changes animation imports to discard extra animation curves for each property channel after the first curve has been found. When this has occurred, a warning is printed to the system console.

f91d576625 initial implementation (and revert only using the first anim curve per channel)

ff192f6f3f Optimise and tidy up combining curves with interpolation

6124ee60af Remove the expansive information on how other interpolation can be set up

285fea09d6 Faster generator for yielding indices of invalid animation curves

b3c8b483d8 Move fbx to Blender keyframe time conversion into blen_store_keyframes

Add faster blen_store_keyframes_multi for when multiple fcurves have the same keyframe times.

3332d1936a Cleanup

fded8b6eb5 Small optimisations and more documentation for _transformation_curves_gen

d83d71e45b Combine use of _combine_same_property_curves and blen_read_single_animation_curve into a single function

cf782b4b21 Cleanup

ae09d491d8 Cleanup: comments

Thomas Barlow commented

2023-08-31 19:05:33 +02:00

Personally, I wouldn't mind skipping additional curves assigned to the same channel since FBX only uses the first curve, but combining them better matches existing Blender FBX IO behaviour.

I just need to double-check the behaviour of importing animations with multiple curves assigned to the same channel because I've only run the relevant code in parts and not with a full .fbx file with this weird setup (and I need to figure out how to create that weird setup).

The .fbx file that I profiled to get the timings in the description I cannot redistribute, but it is the Bellydancing animation from Mixamo (requires adobe login) at 60 fps, without skin and without any keyframe reduction (all other options set to their defaults). This was the longest animation on the site that I could find.

The change to importing valid animation curves that animate both lens and focus_distance (and material color animations that animate 2 or 3 of the red/green/blue channels):
Before:

After:

fbx_animation_tests_nla_disabled_all_actions_disabled.fbx (this contains valid animations of each of the different kinds of currently supported FBX animations, aside from material color animations)

The change to importing invalid animation curves where some of the values would be discarded because they would be extrapolated:
Before (shape keys would default to zero):

After:

ShapeKeyMoveUp_shuffled_all_but_first_and_last.fbx (the times of each keyframe aside from the first and last were shuffled to create this invalid FBX file, the original animation simply activated a shape key from 0-100 linearly over 100 frames)

Personally, I wouldn't mind skipping additional curves assigned to the same channel since FBX only uses the first curve, but combining them better matches existing Blender FBX IO behaviour. ~~I just need to double-check the behaviour of importing animations with multiple curves assigned to the same channel because I've only run the relevant code in parts and not with a full .fbx file with this weird setup (and I need to figure out how to create that weird setup).~~ --- The .fbx file that I profiled to get the timings in the description I cannot redistribute, but it is the `Bellydancing` animation from Mixamo (requires adobe login) at 60 fps, without skin and without any keyframe reduction (all other options set to their defaults). This was the longest animation on the site that I could find. --- The change to importing valid animation curves that animate both `lens` and `focus_distance` (and material color animations that animate 2 or 3 of the red/green/blue channels): Before: ![image](/attachments/bd44a6b2-e303-4d28-9a89-dd673115efef) After: ![image](/attachments/bd7ffb96-97b6-494e-8c33-e7adc038a45f) [fbx_animation_tests_nla_disabled_all_actions_disabled.fbx](/attachments/d6078b3a-a7dc-42c0-a0cf-60535dd8623e) (this contains valid animations of each of the different kinds of currently supported FBX animations, aside from material color animations) --- The change to importing invalid animation curves where some of the values would be discarded because they would be extrapolated: Before (shape keys would default to zero): ![image](/attachments/efeedb03-05d8-43e5-8ebf-abf17e31425d) After: ![image](/attachments/f692e984-8b77-4280-a5bd-a582d76e7f28) [ShapeKeyMoveUp_shuffled_all_but_first_and_last.fbx](/attachments/86a4bf1b-d3de-4858-80b5-b51a2d254eb8) (the times of each keyframe aside from the first and last were shuffled to create this invalid FBX file, the original animation simply activated a shape key from 0-100 linearly over 100 frames)

ShapeKeyMoveUp_shuffled_all_but_first_and_last.fbx

20 KiB

image.png

39 KiB

image.png

19 KiB

image.png

34 KiB

fbx_animation_tests_nla_disabled_all_actions_disabled.fbx

97 KiB

image.png

20 KiB

Thomas Barlow commented

2023-08-31 22:33:23 +02:00

Nice, the import of multiple curves for the same channel worked/works exactly as I expected.

Note that in Unity, for example, only the curve read first will be used, additional curves are discarded. This matches the FBX SDKs description.

To start with, these are the isolated curves:
Curve 1:

Curve 2:

Old code, Curve 1 read first and then Curve 2

With Curve 2 read second, it replaces every keyframe that was read from Curve 1 because its last keyframe time is greater. The keyframe times from Curve 1 are replaced with interpolated values from Curve 2.

Old code, Curve 2 read first and then Curve 1

With Curve 1 read second, it replaces every keyframe that was read from Curve 2, up until the last keyframe time in Curve 1. The keyframes times from Curve 2 that are replaced are replaced with interpolated values from Curve 1

New code (in this case, the curve read order is irrelevant because they have unique keyframe times)

Because both curves have unique keyframe times, it doesn't matter which order they are read in. The curves get interlaced together.
The intention here is that if there were multiple non-overlapping curves, then they would get combined together in a way that makes sense. The choice of using overlapping curves here is just to better illustrate the behaviour.
In the case of curves that contain the same keyframe times, later curves take precedence and overwrite the existing values at those times.

Nice, the import of multiple curves for the same channel worked/works exactly as I expected. Note that in Unity, for example, only the curve read first will be used, additional curves are discarded. This matches the FBX SDKs description. To start with, these are the isolated curves: Curve 1: ![image](/attachments/a312db46-a770-4f5b-969f-fe33dce81704) Curve 2: ![image](/attachments/b4e0501c-c833-4d9d-9bac-0b143f5d4c9a) ### Old code, Curve 1 read first and then Curve 2 With Curve 2 read second, it replaces every keyframe that was read from Curve 1 because its last keyframe time is greater. The keyframe times from Curve 1 are replaced with interpolated values from Curve 2. ![image](/attachments/ef1e3f26-ddf2-4b11-9a0e-9cab83b7159b) ### Old code, Curve 2 read first and then Curve 1 With Curve 1 read second, it replaces every keyframe that was read from Curve 2, up until the last keyframe time in Curve 1. The keyframes times from Curve 2 that are replaced are replaced with interpolated values from Curve 1 ![image](/attachments/bb62271a-ca48-4e6b-9dff-02306715fe47) ### New code (in this case, the curve read order is irrelevant because they have unique keyframe times) Because both curves have unique keyframe times, it doesn't matter which order they are read in. The curves get interlaced together. The intention here is that if there were multiple non-overlapping curves, then they would get combined together in a way that makes sense. The choice of using overlapping curves here is just to better illustrate the behaviour. In the case of curves that contain the same keyframe times, later curves take precedence and overwrite the existing values at those times. ![image](/attachments/07909958-d0b4-4405-b779-8b3a718ebec3)

image.png

31 KiB

image.png

26 KiB

MultipleShapeKeyCurves_FlippedOrder.fbx

20 KiB

image.png

27 KiB

image.png

39 KiB

MultipleShapeKeyCurves.fbx

20 KiB

image.png

26 KiB

Thomas Barlow changed title from ~~WIP: FBX IO: Speed up animation import using NumPy~~ to FBX IO: Speed up animation import using NumPy

2023-08-31 22:38:45 +02:00

Thomas Barlow requested review from Bastien Montagne 2023-08-31 22:45:39 +02:00

Thomas Barlow added 1 commit 2023-09-01 00:58:05 +02:00

464e036073 Cleanup: Remove explicit iter use in frame_values_it and add comments

Bastien Montagne approved these changes 2023-09-04 16:58:57 +02:00

Bastien Montagne left a comment

Quite a big patch, so fairly hard to do a detailed review of the whole changes. but from a quick overview it looks fine. Complete and detailed description and comments also help here, thanks!

Feel free to commit the change, also remember in this case to increase the version of the add-on (in its __init__.py file), think this is worth a minor version raise! Release notes should also be updated accordingly.

Quite a big patch, so fairly hard to do a detailed review of the whole changes. but from a quick overview it looks fine. Complete and detailed description and comments also help here, thanks! Feel free to commit the change, also remember in this case to increase the version of the add-on (in its `__init__.py` file), think this is worth a minor version raise! [Release notes](https://wiki.blender.org/wiki/Reference/Release_Notes/4.0/Add-ons) should also be updated accordingly.

io_scene_fbx/import_fbx.py

						
				@ -551,0 +557,4 @@

				    }

				    # Create a setter into transform_data for each values array. e.g. a values array for 'Lcl Scaling' with channel == 2

				    # would set transform_data.sca[2].

Bastien Montagne commented

2023-09-04 16:47:38 +02:00

transform_data.scale[2] I believe?

`transform_data.scale[2]` I believe?

Thomas Barlow commented

2023-09-04 17:23:57 +02:00

In this case .sca is correct, the FBXTransformData namedtuple uses rather short attribute names.

In this case `.sca` is correct, the `FBXTransformData` namedtuple uses rather short attribute names.

👍 1

Bastien Montagne commented

2023-09-04 17:00:27 +02:00

Nice, the import of multiple curves for the same channel worked/works exactly as I expected.

Note that in Unity, for example, only the curve read first will be used, additional curves are discarded. This matches the FBX SDKs description.

TBH I think it would be best to also discard any additional animation curve here... This should indeed be a separate commit though.

> Nice, the import of multiple curves for the same channel worked/works exactly as I expected. > > Note that in Unity, for example, only the curve read first will be used, additional curves are discarded. This matches the FBX SDKs description. TBH I think it would be best to also discard any additional animation curve here... This should indeed be a separate commit though.

Thomas Barlow merged commit abab1c9343 into main

2023-09-04 22:07:45 +02:00

Thomas Barlow deleted branch fbx_import_anim_numpy_p1

2023-09-04 22:07:45 +02:00

Thomas Barlow referenced this issue from a commit

2023-09-04 22:07:46 +02:00

FBX IO: Speed up animation import using NumPy

Sign in to join this conversation.

No reviewers