WIP: Anim: Implement keyframing functionality for the Animation data-block #119669

Christoph Lendenfeld · 2024-03-19T17:06:16+01:00

Christoph Lendenfeld commented

2024-03-19 17:06:16 +01:00

WIP because #118677: Baklava: minimal data model needs to land first.

This PR modifies the keyframing operators to work with the new Animation data-block.

Most of my changes are in animrig/intern/keyframing.cc.

Known issues

While auto-keying works, the flag check for "Only insert needed" and "Only insert available" does not
Keyingsets do not work yet.
Some Dependency Graph update is missing because you need to move keys before they affect the viewport.
Datablock name should go through translation. Currently it is hardcoded to "Animation"
Create helper functions to get Layer and Strip
Need to check the experimental flag before inserting keys to the Animation datablock

WIP because [#118677: Baklava: minimal data model](https://projects.blender.org/blender/blender/pulls/118677) needs to land first. This PR modifies the keyframing operators to work with the new Animation data-block. Most of my changes are in `animrig/intern/keyframing.cc`. ## Known issues * While auto-keying works, the flag check for "Only insert needed" and "Only insert available" does not * Keyingsets do not work yet. * Some Dependency Graph update is missing because you need to move keys before they affect the viewport. * Datablock name should go through translation. Currently it is hardcoded to "Animation" * Create helper functions to get `Layer` and `Strip` * Need to check the experimental flag before inserting keys to the Animation datablock

Christoph Lendenfeld added the

Module

Animation & Rigging

label 2024-03-19 17:06:16 +01:00

Christoph Lendenfeld added 16 commits 2024-03-19 17:06:27 +01:00

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

f740332275 Refactor: reduce indentation in `ANIM_nla_mapping_get()`

Flip conditions and use early returns to simplify `ANIM_nla_mapping_get()`.

No functional changes.

02f3fad1c0 Anim: add Animation data-block management functions

Add code (including RNA wrappers) for:

- Creating, removing, and accessing `Animation` data-blocks.
- Creating and removing layers, strips, and bindings on those `Animation`
  data-blocks.
- Accessing those via RNA.

Note that this does not include assignment to any animated data-block,
so it is of limited practical use.

For more info, see #113594.

37613cfb12 Anim: allow assigning Animation data-blocks

Expand the `AnimData` struct with an `Animation *` + an
`binding_stable_index` field, and properly handle those relations.

This also adds functionality for actually pointing animated IDs to
`Animation` data-blocks, and automatically hooking up the relevant
`Binding`.

The Depsgraph code is extended to take these new relations into account,
but doesn't trigger any animation evaluation yet.

For more info, see #113594.

d6dc36f48f Anim: Add a simple GUI for assigning Animation data-blocks

Add a 'Baklava' panel to the 3D Viewport side-panel. It's a
developer-GUI, not meant for animators (or for inclusion beyond the
experimental feature, for that matter).

Note that this GUI shows all layer properties, even though the data
model is currently limited to a single layer. This means that things
like 'influence' and 'mix mode' are irrelevant, as there is no
underlying layer to mix with.

Also key insertion and animation evaluation are not implemented yet (but
will be in upcoming commits).

For more info, see #113594.

fc89c10782 Anim: allow inserting keys in Animation data-block

Allow inserting keys into Keyframe strips (which is the only type of
strip that is currently implemented).

Note that the data model is currently limited to a single layer, with a
single infinite strip. Because of this, the strip will not be shown in
any UI, as there is no way to manipulate it anyway.

Note that the inserted keys are not yet evaluated, so the animation
isn't visible in the 3D viewport yet. That's for an upcoming commit.

For more info, see #113594.

e351378c68 Anim: add evaluation of Animation data-blocks

Include Animation data-block handling in Blender's animation evaluation
stack. If an `Animation` is assigned to an `ID`, it will take precedence
over the NLA and/or any `Action` that might be assigned as well.

For more info, see #113594.

f9547aa690 Anim: show animated properties with the appropriate color in the GUI

Use the `Animation` data-block to find F-Curves, for drawing the
background color of properties in the GUI (green for 'animated', yellow
for 'keyed on this frame', etc.).

This assumes (correctly) that the `Animation` is limited to a single
layer with a single strip. As such, there is only one set of F-Curves
for every animated ID, which means that the correct F-Curve will be
found. This will need adjustment when the same property can have
multiple F-Curves (due to layers) or when an F-Curve may not be
applicable for each point in time (due to the use of finite strips).

20ec4906f1 Anim: add Animation data-block to filtering code

It seems to be working like this, but more testing is necessary.

9cb1ea3ffb Refactor: use animfilter_fcurves_span in more places

5099db0f8e Properly tag Animation for updates in `ANIM_list_elem_update()`

This is called by the graph editor transform code, and is necessary to
see the effect of moving keyframes.

b9939d81f4 created an animation datablock

ae58f12c1c inserting "works"

d18af9aa43 split into subfunctions

c26c0aa9d4 fix crash

7388211a5f use existing function to create an animation datablock

25aa82c233 split function

Sybren A. Stüvel reviewed 2024-03-21 12:36:23 +01:00

source/blender/animrig/intern/keyframing.cc Outdated

						
				@ -997,0 +1016,4 @@

				                                                                                        prop);

				    Vector<float> rna_values = get_keyframe_values(&ptr, prop, visual_keyframing);

				    int property_array_index = 0;

				    for (float value : rna_values) {

Sybren A. Stüvel commented

2024-03-21 12:36:23 +01:00

Here you could have

for (int property_array_index : rna_values.index_range()) {
  const float value = rna_values[property_array_index];
  ...
}

This will remove the need to keep track of the array index explicitly, and I think the code will be a bit simpler for it.

Here you could have ```cpp for (int property_array_index : rna_values.index_range()) { const float value = rna_values[property_array_index]; ... } ``` This will remove the need to keep track of the array index explicitly, and I think the code will be a bit simpler for it.

👍 1

Christoph Lendenfeld commented

2024-03-21 16:06:27 +01:00

the code is now in a different place, but I still used that logic. Thanks

Sybren A. Stüvel reviewed 2024-03-21 12:59:00 +01:00

Sybren A. Stüvel left a comment

I think this is a good start. I'm still pondering a bit over the order in which things are done. Currently in this PR:

find the layer, creating one if necessary
find the strip, creating one if necessary
loop over the RNA paths
for each property, find the current values, and use those to insert a key.

It might be interesting to split up that last part, especially since at some point we'll want to pin certain properties to certain layers. How would you feel about this?

Loop over each property, find the current values. Store these in some container with the resolved RNAProperty. Maybe the EvaluationResult could be reused for this?
Use utility function to ensure we have a KeyframeStrip to put keys into.
Use the properties-with-values from step 1 to insert the keys into the strip.

And maybe step 3 could be optimised by first getting the ChannelBag for the Binding, and then inserting into that.

I think this is a good start. I'm still pondering a bit over the order in which things are done. Currently in this PR: 1. find the layer, creating one if necessary 2. find the strip, creating one if necessary 3. loop over the RNA paths 4. for each property, find the current values, and use those to insert a key. It might be interesting to split up that last part, especially since at some point we'll want to pin certain properties to certain layers. How would you feel about this? 1. Loop over each property, find the current values. Store these in some container with the resolved RNAProperty. Maybe the `EvaluationResult` could be reused for this? 2. Use utility function to ensure we have a `KeyframeStrip` to put keys into. 3. Use the properties-with-values from step 1 to insert the keys into the strip. And maybe step 3 could be optimised by first getting the `ChannelBag` for the `Binding`, and then inserting into that.

source/blender/animrig/intern/keyframing.cc

						
				@ -997,0 +1019,4 @@

				    for (float value : rna_values) {

				      /* TODO: convert scene frame to strip time. */

				      const float2 time_value = {scene_frame, value};

				      strip.as<KeyframeStrip>().keyframe_insert(

Sybren A. Stüvel commented

2024-03-21 12:46:34 +01:00

This code assumes that the strip is a KeyframeStrip. Better to just pass the strip parameter as such, and leave the wrapping / assumptions / error handling to the caller.

This code assumes that the strip is a `KeyframeStrip`. Better to just pass the `strip` parameter as such, and leave the wrapping / assumptions / error handling to the caller.

👍 1

source/blender/animrig/intern/keyframing.cc Outdated

						
				@ -997,0 +1069,4 @@

				  }

				  Layer *layer;

				  if (anim.layers().size() == 0) {

Sybren A. Stüvel commented

2024-03-21 12:48:03 +01:00

There's now double code to ensure a layer exists, which means duplication of the default layer name logic as well. Better to move this into some KeyframeStrip &keystrip = animrig::simplifications::ensure_layer_with_keyframe_strip(anim) function.

There's now double code to ensure a layer exists, which means duplication of the default layer name logic as well. Better to move this into some `KeyframeStrip &keystrip = animrig::simplifications::ensure_layer_with_keyframe_strip(anim)` function.

Christoph Lendenfeld commented

2024-03-21 16:01:56 +01:00

where is the duplication of that code? Is that somewhere in your code?

source/blender/animrig/intern/keyframing.cc Outdated

						
				@ -1007,0 +1104,4 @@

				    return;

				  }

				  if (adt->animation != nullptr || adt->action == nullptr) {

Sybren A. Stüvel commented

2024-03-21 12:49:24 +01:00

I think it's fine to write more "boolean" style checks for these, like if (adt->animation || !adt->action).

I used to be of "the other camp", but thinking about things like smart pointers have swayed me in the other direction.

I think it's fine to write more "boolean" style checks for these, like `if (adt->animation || !adt->action)`. I used to be of "the other camp", but thinking about things like smart pointers have swayed me in the other direction.

👍 1

Christoph Lendenfeld added 1 commit 2024-03-21 16:02:54 +01:00

dfe59ce8f8 move object eval to outer level

Christoph Lendenfeld added 1 commit 2024-03-21 16:05:51 +01:00

3039305bdb fix: make work with bones by passing resolved path

Christoph Lendenfeld added 1 commit 2024-03-21 16:07:53 +01:00

7c869ae0b4 use boolean logic to check pointers

Christoph Lendenfeld commented

2024-03-21 16:21:29 +01:00

I've moved the path resolution and evaluation up to insert_key_anim. Good point moving that up because we will need to do NLA like decomposition so we can't resolve the values on strip level.

About using EvaluationResult, This doesn't seem to store a time value. How would I use that. I've made a struct KeyInsertData for now

I've moved the path resolution and evaluation up to `insert_key_anim`. Good point moving that up because we will need to do NLA like decomposition so we can't resolve the values on strip level. About using `EvaluationResult`, This doesn't seem to store a time value. How would I use that. I've made a struct `KeyInsertData` for now

Sybren A. Stüvel referenced this pull request

2024-04-08 17:25:20 +02:00

Animation: Project Baklava phase 1: multi-data-block animation #120406

Nathan Vegdahl referenced this pull request

2024-04-09 14:08:26 +02:00

Anim: implement 3D viewport keyframing functionality for the Animation data-block #120428

Nathan Vegdahl commented

2024-04-09 14:12:09 +02:00

At @ChrisLend 's suggestion, I've continued this in a separate PR (#120428) by just applying the diff of his combined commits against main.

At @ChrisLend 's suggestion, I've continued this in [a separate PR (#120428)](https://projects.blender.org/blender/blender/pulls/120428) by just applying the diff of his combined commits against main.

👍 2

Nathan Vegdahl closed this pull request

2024-04-09 14:12:13 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

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

WIP: Anim: Implement keyframing functionality for the Animation data-block #119669

Known issues

Pull request closed