Remove some `#ifdef WITH_ANIM_BAKLAVA` guards to make a unit test
succeed. The code is handling slots when assigning Actions to IDs.
Non-experimental builds will only deal with legacy Actions, and thus the
slot assignment is a no-op anyway.
The unit test is explicitly creating layered Actions and running tests
on them. Since this also covers some new code introduced for Action
assignments (unified code for both legacy & layered Actions), I'd feel
more comfortable keeping the test enabled.
Note that when loading a blend file with layered Actions in a
non-experimental build will actually load them as legacy Actions.
Pull Request: https://projects.blender.org/blender/blender/pulls/128483
Mark these copy constructors as 'explicit' in `blender::animrig`:
- `Slot`
- `StripKeyframeData`
- `ChannelBag`
The copy constructors for the other related classes were already
`explicit` or `deleted`.
This prevents bugs by disallowing implicit copies. For example:
```cpp
ChannelBag cbag = agrp->channel_bag->wrap();
```
This should have been a reference (`ChannelBag &cbag`), an easy mistake
which is now caught by the compiler (and fixed in this commit).
No functional changes. The implicit copy that was removed was just
inefficient, but didn't produce the wrong results.
Pull Request: https://projects.blender.org/blender/blender/pulls/128424
Previously it only filtered on a specific rna path-based criteria.
This commit changes it to take a predicate function instead, splitting
off its previously fixed criteria into a separate function.
This also renames it to `fcurves_in_action_slot_filtered()` to better
reflect its new generalized functionality.
No functional changes intended.
Pull Request: https://projects.blender.org/blender/blender/pulls/128423
Add support for `rna_struct.keyframe_insert(…, group="name")` parameter,
when inserting keys into a layered Action.
This simply was never implemented, and the default channel group name
was always used.
Pull Request: https://projects.blender.org/blender/blender/pulls/128383
Calling `action.fcurves.clear()` would clear the F-Curves, but didn't
update the F-Curve groups. This meant that the groups data still had their
original length & offsets into the F-Curves array, causing subsequent
F-Curve creation to crash Blender.
The unit test for this also covers the previous commit.
Pull Request: https://projects.blender.org/blender/blender/pulls/128375
Fix an issue where Python's `rna_struct.keyframe_insert(path, index=-1)`
would not create any keys. This was caused by the new layered animation
code not taking -1 as an 'all array elements' wild-card.
The accompanying unit test will follow in another commit, as its success
depends on another bugfix as well.
Users of the layered Actions API should never mutate the data via these
spans, and so the functions should always return just a `Span<>`.
No functional changes.
Pull Request: https://projects.blender.org/blender/blender/pulls/128264
`BKE_animdata_fix_paths_remove()` now calls into the new function
`animrig::legacy::action_fcurves_remove()`, which works for both
legacy and slotted Actions.
Also I updated the documentation of the BKE function so that it's clear
what it's actually doing.
No functional changes for legacy Actions.
Ref: #123424
Pull Request: https://projects.blender.org/blender/blender/pulls/128252
When assigning an Action with the new API:
- check the Actions ID type (if it is a legacy Action), and
- check whether the ID is in NLA tweak mode.
This means that action assignment can fail, so the
`animrig::assign_action()` and `animrig::unassign_action()` functions
now return a `[[nodiscard]] bool`.
Part of `generic_assign_action()` has now also been shielded with an
`#ifdef WITH_ANIM_BAKLAVA` just to be sure.
This also includes a change in `BKE_animdata_free()`. That function now
first exits NLA tweak mode (if enabled) before freeing the Actions. This
makes it possible to simply un-assign the assigned Action, as all the
NLA tweakmode stuff has already been taken care of by the responsible
function.
Pull Request: https://projects.blender.org/blender/blender/pulls/128199
This adds an operator that moves slots of slot channels selected in
the channel box to a new action.
All slots are moved together into that new action instead of
moving them into separate actions.
I think that's more reasonable because that way the "move slots into separate actions" is
still possible by selecting the slots one by one.
The "explode action" operator that does just that can be a separate operator.
The new action is named after the slot for the case when only one slot is moved.
When multiple slots are moved, the name "CombinedAction" is used.
This also adds a menu entry "Action" to the action editor.
Pull Request: https://projects.blender.org/blender/blender/pulls/128171
Previously we were using a bespoke hodgepodge of
`Action::is_action_legacy()` and `Action::is_action_layered()`,
sometimes in combination with checking for the Baklava feature flag,
when what we really meant is "Should this action be treated as legacy
or not?"
This commit changes the places where that's semantically what we meant
to use `action_treat_as_legacy()`. Some of those places were already
correct, using a compound conditional, but some of them weren't, and
thus were not always branching correctly. For those latter cases,
this commit is a bug fix.
Importantly, not all uses of bare `Action::is_action_legacy()` or
`Action::is_action_layered()` are semantically incorrect: there are many
places where that is the right thing to do. This commit takes care not
to touch those places.
Pull Request: https://projects.blender.org/blender/blender/pulls/128174
Use the `foreach_action_slot_use(id, callback)` iterator function to find
all action/slot uses of an ID. That already handles the action constraint,
and thus solves the issue.
Pull Request: https://projects.blender.org/blender/blender/pulls/128090
Add slotted/layered Action support to `get_active_actiongroup()` and
`set_active_action_group()`.
Note that there is still a bunch of code around that directly manipulates
the action group flags, instead of using these functions. That's for
another commit to address.
This commit also introduces the functions `action_treat_as_legacy()` and
`channel_groups_all(action)` utility functions in the `animrig::legacy`
namespace.
Ref: #123424
Pull Request: https://projects.blender.org/blender/blender/pulls/128084
The collada export code was directly using `action->curves` in its
export code, which doesn't work with slotted actions. This commit
updates that code to use wrapper functions that provide access to the
correct fcurves regardless of whether the action is slotted or not.
Note that the import code has not yet been updated, and is left for
a future PR.
Ref: #123424
Pull Request: https://projects.blender.org/blender/blender/pulls/128061
Convert the Grease Pencil legacy converter code to the new layered
Action API.
Unfortunately this cannot just work on legacy Actions (and then rely on
the versioning to layered Actions we'll land soon), as the GP conversion
runs as the final step of the versioning process. This cannot be helped,
as the usual versioning is not allowed to create new IDs and this does.
Ref: #123424
Pull Request: https://projects.blender.org/blender/blender/pulls/128064
Rename `action_foreach_fcurve` to `foreach_fcurve_in_action_slot`. This
matches the functionality better, and opens up the floor for an iterator
that actually does loop over all F-Curves in an Action.
No functional changes.
Pull Request: https://projects.blender.org/blender/blender/pulls/128065
`BKE_fcurves_filter()` was only used by the Symmetrize Armature
operator, when dealing with Action Constraints on the bones.
`BKE_fcurves_filter()` is now removed, and has been replaced by the new
`animrig::fcurve_find_in_action_slot_filtered()`. It's functionally
pretty much the same, except that it returns a `Vector<FCurve *>`
instead of manipulating a `ListBase` parameter.
Because it's now implemented using an iterator in
`ANIM_action_iterators.hh` it didn't feel right to keep the function in
BKE.
No functional changes.
Fix the code that decides whether to create a new F-Curve in a legacy or
layered style. The old code went for layered if:
- The experimental feature is enabled AND the Action is already layered.
The new code goes for layered if:
- The experimental feature is enabled AND the Action is empty, OR
- The Action is already layered.
Pull Request: https://projects.blender.org/blender/blender/pulls/128036
Replace those calls to `BKE_fcurve_find()` that are searching in the
curves of an Action with their corresponding call in `animrig`:
- `animrig::fcurve_find_in_action()` when it should really search
through the entire Action,
- `animrig::fcurve_find_in_action_slot()` when only the F-Curves for a
specific slot should be searched, or
- `animrig::fcurve_find_in_assigned_slot()` same as above, searching
through the action slot that is assigned to the given ADT.
This also makes `animrig::fcurve_find_in_action()` compatible with both
layered and legacy Actions.
This makes the naming a bit closer to the `BKE_fcurve_find()` function,
and opens op the naming for two upcoming functions
`fcurve_find_in_action_slot` and `fcurve_find_in_assigned_slot`.
No functional changes.
Pull Request: https://projects.blender.org/blender/blender/pulls/128036
Remove the declaration of `blender::animrig::legacy::action_has_fcurves()`.
Its implementation was never committed to `main`, and the declaration also
shouldn't have.
No functional changes.
Instead of assigning Actions by direct pointer manipulation (and the
corresponding juggling of user counts), call `animrig::assign_action()`
and `animrig::unassign_action()`.
These functions not only correctly handle user counts, but also ensure
that slot assignments & user tracking works. The former always happens,
the latter only when building with experimental features enabled.
Because (un)assigning slotted Actions need the animated ID (instead of
just the `AnimData *`), more functions now require an `OwnedAnimData`.
Note that there is still some user count juggling. This is caused by
`BKE_id_new()`, and by extension `BKE_action_add`, returning an ID with
user count = 1, even though that ID is not yet used. A todo task #128017
has been made to change `BKE_action_add()` so that the Action it returns
can be directly fed into `animrig::assign_action()`.
This PR updates the following areas:
- Creating a node group by grouping animated nodes, as this has to move
the animation data. This PR just handles the assignment of a new
Action.
- Temporary Action creation for ungrouping node groups.
- Versioning of pre-2.5 animation data.
No functional changes.
Ref: #123424
Pull Request: https://projects.blender.org/blender/blender/pulls/128026
Fix two bugs by replacing direct assignment to `adt->action` with a call
to `animrig::assign_action()`:
- If a related Action was found, its user count was not incremented.
- If a new Action is created, its `idroot` was not properly set.
Pull Request: https://projects.blender.org/blender/blender/pulls/128030
The `animrig::assign_action(action, id)` function now takes a `bAction`
pointer (instead of its C++ wrapper `animrig::Action`). This makes it
easier to call from code that just deals with the DNA/C struct.
Also an override was added that takes an `OwnedAnimData` struct instead
of just the ID. This saves the lookup of the AnimData, to be used in cases
where the caller already has it.
Pull Request: https://projects.blender.org/blender/blender/pulls/128030
Instead of simply reassigning the `adt->action` and `adt->tmpact` pointers,
the code now uses the `animrig::assign_action()` and `assign_tmpaction()`.
This way the slot user maps should be properly updated.
Pull Request: https://projects.blender.org/blender/blender/pulls/128030
This adds the "Merge Animation" (`ANIM_OT_merge_animation`) operator to the 3D viewport.
It merges the animation of all selected objects into the animation of the active object.
On a technical level this moves the `ChannelBag` of the source action into a new slot
on the target action. The Slot name is copied but might differ due to name clashes.
Due to this, the operator is named "Merge Animation" and not "Merge Actions" since
it might not merge the complete actions but only the slots actually used by the selection
in the 3D viewport
This operator will find all related IDs to the selection, i.e. object data, shape keys etc. and merge those as well.
That means this operator can also be used to JUST merge the actions of the active object and its data.
### Caveats
Because there may be more than 1 slot user this might reassign an object that isn't actually selected.
The other option is that when there is more than 1 slot user, the `ChannelBag` is copied instead of moved.
That means moving objects that share a slot one by one would result in a loss of slot sharing and
data duplication. I think it's better to respect the artist decision of "these things use the same animation"
in this case. (open to opinions though)
Design task: #126937
Pull Request: https://projects.blender.org/blender/blender/pulls/127414
This is a follow up to #126559. In that PR, strip data is never deleted even
when the strip that uses it is. Thus, the strip data array just keeps growing as
more strips are added, and never shrinks.
This PR implements that deletion. The approach is simple: remove the strip data
from the array using a swap-remove that swaps in the last item in the data
array, and then loop over the strips in the action to update any that were
referencing that swapped-in item. Additionally, before removal we check to make
sure the data item isn't still being used by any strips, and if it is then we
don't remove it.
Pull Request: https://projects.blender.org/blender/blender/pulls/127837
Add an iterator `foreach_action_slot_use_with_references(ID, callback)`
that provides references to the found `bAction` pointer and slot handle.
That way the callback can assign another Action or slot when it sees
fit, without having to know exactly what the source of this info is
(could be a direct assignment, but also an NLA strip or Action
Constraint).
Ref: #127844
Pull Request: https://projects.blender.org/blender/blender/pulls/127871
Add some F-Curve getter functions that work in all these situations:
- Built without experimental features.
- Built with experimental features, and called with legacy Action.
- Built with experimental features, and called with layered Action.
No functional changes, just useful tools for migrating to the new
Actions API.
Ref: #120406
Pull Request: https://projects.blender.org/blender/blender/pulls/127841
Also return a pointer from `Layer::duplicate_with_shallow_strip_copies()`
rather than a reference, since it doesn't maintain ownership of the
returned item.
This updates the layered action data model to store strip data differently. Specifically:
- `Strip` is now just a single, POD type that only stores the data common to all
strips, such as start/end frames.
- The data that might be of a completely different nature between strips (e.g.
keyframe data vs modifier data) is now stored in arrays on the action itself.
- `Strip`s indicate their type with an enum, and specify their data with an
index into the array on the action that stores data for that type.
This approach requires a little more data juggling, but has the advantage of
making `Strip`s themselves super simple POD types, and also opening the door to
trivial strip instancing later on: instances are just strips that point at the
same data.
The intention is that the RNA API remains the same: from RNA's perspective there
is no data storage separate from the strips, and a strip's data is presented as
fields and methods directly on the strip itself. Different strip types will be
presented as different subtypes of `ActionStrip`, each with their own fields and
methods specific to their underlying data's type. However, this PR doesn't
implement that sub-typing, leaving it for a future PR. It does, however, put the
fields and methods of the one strip type we have so far directly on the strip,
which avoids changing the APIs we have so far.
This PR implements the bulk of this new approach, and everything should be
functional and working correctly. However, there are two TODO items left over
that will be implemented in forthcoming PRs:
- Type refinement in the RNA api. This PR actually removes the existing type
refinement code that was implemented in terms of the inheritance tree of the
actual C++ types, and this will need to be reimplemented in terms of the new
data model. The RNA API still works without the type refinement since there
are only keyframe strips right now, but it will be needed in preparation for
more strip types down the road.
- Strip data deletion. This PR only deletes data from the strip data arrays when
the whole action is deleted, and otherwise just accumulates strip data as more
and more strips are added, never removing the data when the corresponding
strips get removed. That's fine in the short term, especially since we only
support single strips right now. But it does need to be implemented in
preparation for proper layered actions.
Pull Request: https://projects.blender.org/blender/blender/pulls/126559
Refactor the slot assignment & related properties to use as much generic
code as possible. This'll make it much easier to add Action+Slot
properties to other data-blocks in the future (currently just planned
for the Action Constraint).
Pull Request: https://projects.blender.org/blender/blender/pulls/127720
In the `blender::animrig` namespace, add two new functions
`generic_assign_action()` and `generic_assign_action_slot()`. These are
now used as basis for (un)assigning Actions and Action slots, both
directly on the ID and for NLA strips.
The method `Action::assign_id()` has been removed, in favour of free
functions for (un)assigning Actions and slots.
This is almost a non-functional change. The only thing that's different
is when both an Action and a slot should get assigned in one go.
Old behaviour: the slot would be inspected, and if not suitable for the
animated ID, the entire operation would be aborted. In other words, any
previously-assigned Action would still be assigned, making this an
atomic operation.
New behaviour: assigning an Action + a slot is no longer atomic. First
the new Action is assigned (which uses the automatic slot selection
based on the name). If after this the given slot cannot be assigned, the
new Action + the auto-selected slot are still kept.
This makes the Action & slot assignment more uniform in behaviour, where
"assign this Action + this slot" as one operation behaves the same as
two sequential operations "assign this Action" + "assign this slot". If
it turns out to be confusing, we can always improve things.
The return code for slot assignment is now more explicit (enum instead
of boolean), so that the caller can decide what to do in case of which
error (like unassigning the slot if the automatic behaviour is
unwanted).
Pull Request: https://projects.blender.org/blender/blender/pulls/127712
When inserting keys, look on related IDs for an action to reuse that.
This will make use of the slot system on the new layered action to ensure
the animation data doesn't collide.
This is done on the `id_action_ensure` function since that is the common
function to get an action off an `ID`.
IDs with more than 1 user will be skipped.
"Related ID" in this case is hardcoded with a `switch` statement for each ID type.
The system builds a list starting from the ID that should be keyed and
keeps expanding the list until an action is found or no more
non-duplicate IDs can be added. (This is using linear search for duplicate checks,
but I don't think we will deal with a lot of IDs in this case)
Pull Request: https://projects.blender.org/blender/blender/pulls/126655
