Custom properties on pose markers in Actions were not handled properly
in the following ways:
- They were not written to or read from blend files, which resulted in
crashes (reported in #138201).
- They were not duplicated when the pose marker lists were duplicated
(during Action duplication), which would leave the duplicate marker
*sharing* custom properties with the marker it was duplicated from.
This PR fixes these issues by creating functions to handle the reading,
writing, and copying of marker lists which properly read/write/copy
custom properties as well, and using those functions in the relevant
places.
Pull Request: https://projects.blender.org/blender/blender/pulls/138494
* Remove `DEG_get_evaluated_object` in favor of `DEG_get_evaluated`.
* Remove `DEG_is_original_object` in favor of `DEG_is_original`.
* Remove `DEG_is_evaluated_object` in favor of `DEG_is_evaluated`.
Pull Request: https://projects.blender.org/blender/blender/pulls/138317
Add a new node to the dependency graph, to act as a single entry point
before drivers are evaluated in parallel.
The node will take all the driven RNA properties, and write their
current value to the property again. This ensures that any implicitly
shared data is copied to ensure writability. Subsequent concurrent
writes by the driver evaluation will then be safe, as the
thread-unsafe part has already been performed.
Fixes: #132423
Pull Request: https://projects.blender.org/blender/blender/pulls/135802
The main issue of 'type-less' standard C allocations is that there is no check on
allocated type possible.
This is a serious source of annoyance (and crashes) when making some
low-level structs non-trivial, as tracking down all usages of these
structs in higher-level other structs and their allocation is... really
painful.
MEM_[cm]allocN<T> templates on the other hand do check that the
given type is trivial, at build time (static assert), which makes such issue...
trivial to catch.
NOTE: New code should strive to use MEM_new (i.e. allocation and
construction) as much as possible, even for trivial PoD types.
Pull Request: https://projects.blender.org/blender/blender/pulls/136134
Fix an issue when the NLA is used in conjunction with a directly-
assigned Action.
When the directly-assigned Action is also used in an NLA strip, that
strip would not be evaluated any more. This was even the case when
different slots were used, which entirely muted the strip when there
was no slot directly assigned. Now the "this has been handled already"
logic considers the action and the slot.
Pull Request: https://projects.blender.org/blender/blender/pulls/135911
This fixes the (unreported) issue where solo'ing an NLA track would only
play back animation when _any_ of the tracks were unmuted.
Some code considered a track to be muted when its `NLATRACK_MUTED` flag
was set regardless of the `NLATRACK_SOLO` flag, whereas other code did
consider the `NLATRACK_SOLO` flag.
Now all the code is consistent with what actual animation evaluation is
doing.
Pull Request: https://projects.blender.org/blender/blender/pulls/134500
This commit takes the 'Slotted Actions' out of the experimental phase.
As a result:
- All newly created Actions will be slotted Actions.
- Legacy Actions loaded from disk will be versioned to slotted Actions.
- The new Python API for slots, layers, strips, and channel bags is
available.
- The legacy Python API for accessing F-Curves and Action Groups is
still available, and will operate on the F-Curves/Groups for the first
slot only.
- Creating an Action by keying (via the UI, operators, or the
`rna_struct.keyframe_insert` function) will try and share Actions
between related data-blocks. See !126655 for more info about this.
- Assigning an Action to a data-block will auto-assign a suitable Action
Slot. The logic for this is described below. However, There are cases
where this does _not_ automatically assign a slot, and thus the Action
will effectively _not_ animate the data-block. Effort has been spent
to make Action selection work both reliably for Blender users as well
as keep the behaviour the same for Python scripts. Where these two
goals did not converge, reliability and understandability for users
was prioritised.
Auto-selection of the Action Slot upon assigning the Action works as
follows. The first rule to find a slot wins.
1. The data-block remembers the slot name that was last assigned. If the
newly assigned Action has a slot with that name, it is chosen.
2. If the Action has a slot with the same name as the data-block, it is
chosen.
3. If the Action has only one slot, and it has never been assigned to
anything, it is chosen.
4. If the Action is assigned to an NLA strip or an Action constraint,
and the Action has a single slot, and that slot has a suitable ID
type, it is chosen.
This last step is what I was referring to with "Where these two goals
did not converge, reliability and understandability for users was
prioritised." For regular Action assignments (like via the Action
selectors in the Properties editor) this rule doesn't apply, even though
with legacy Actions the final state ("it is animated by this Action")
differs from the final state with slotted Actions ("it has no slot so is
not animated"). This is done to support the following workflow:
- Create an Action by animating Cube.
- In order to animate Suzanne with that same Action, assign the Action
to Suzanne.
- Start keying Suzanne. This auto-creates and auto-assigns a new slot
for Suzanne.
If rule 4. above would apply in this case, the 2nd step would
automatically select the Cube slot for Suzanne as well, which would
immediately overwrite Suzanne's properties with the Cube animation.
Technically, this commit:
- removes the `WITH_ANIM_BAKLAVA` build flag,
- removes the `use_animation_baklava` experimental flag in preferences,
- updates the code to properly deal with the fact that empty Actions are
now always considered slotted/layered Actions (instead of that relying
on the user preference).
Note that 'slotted Actions' and 'layered Actions' are the exact same
thing, just focusing on different aspects (slot & layers) of the new
data model.
The "Baklava phase 1" assumptions are still asserted. This means that:
- an Action can have zero or one layer,
- that layer can have zero or one strip,
- that strip must be of type 'keyframe' and be infinite with zero
offset.
The code to handle legacy Actions is NOT removed in this commit. It will
be removed later. For now it's likely better to keep it around as
reference to the old behaviour in order to aid in some inevitable
bugfixing.
Ref: #120406
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
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
Move the following BKE functions to the `animrig::Action` class. Some of
those will be extended to support slots in a future commit; for now they
still operate on all F-Curves in the Action.
| Old | New |
|---------------------------------|-------------------------------------|
| `BKE_action_frame_range_calc()` | `Action::get_frame_range_of_keys()` |
| `BKE_action_frame_range_get()` | `Action::get_frame_range()` |
| `BKE_action_has_motion()` | `Action::has_keyframes()` |
| `BKE_action_has_single_frame()` | `Action::has_single_frame()` |
| `BKE_action_is_cyclic()` | `Action::is_cyclic()` |
Implementations have been copied from the BKE functions. The frame range
functions now return `float2` instead of requiring two `float *r_…`
return parameters.
The `has_motion` function is now renamed to `has_keyframes`, as that is
what the implementation was actually testing for.
The functions now no longer are null-safe. The BKE functions handled a
null action pointer, but IMO that doesn't make sense, and in none of the
call sites I could find where this would actually be valid.
No functional changes.
Ref: #127489
Pull Request: https://projects.blender.org/blender/blender/pulls/127512
Duplicating an Action stashes the original one on the NLA. The NLA
evaluation code didn't properly handle the special case of an all-muted
NLA (all stashed actions are muted there) when a layered Action was
assigned directly to the animated ID.
The code is now refactored to remove this special handling from the
`animsys_calculate_nla()` function. Instead it now just returns whether it
did anything. The caller can then decide to evaluate the main Action
instead. This ensures that there is only one "evaluate the main Action"
branch in the `BKE_animsys_evaluate_animdata()` function.
Pull Request: https://projects.blender.org/blender/blender/pulls/127569
Add support for slotted Actions to the NLA evaluation code.
This also affects the pose library code and the Action Constraint. These
both share some Action evaluation logic with the NLA. They now
explicitly looks at only the first Action slot. The Action Constraint will
have to be updated to have an explicit slot selector, but that's for another
commit.
Pull Request: https://projects.blender.org/blender/blender/pulls/127425
Refactor to prepare for slotted action support to the evaluation of
quaternion F-Curves.
Since slotted Actions store F-Curves in an array, you cannot iterate over
them any more via the `ListBase` pointer `fcurve.next`. Quaternion
evaluation code has been refactored to work on a span of F-Curves instead
of just getting the first one.
For now, slotted Actions just evaluate their first slot only. A future
commit will add a slot handle parameter to evaluate the correct slot.
This commit mainly replaces a lot of `memset(0)` by empty value
initialization of `PointerRNA` and related data.
It also moves a few remaining areas from C alloc/free to C++ new/delete
memory hanlding.
Part of the effort to make PointerRNA non-trivial (#122431).
This commit moves generated `RNA_blender.h`, `RNA_prototype.h` and
`RNA_blender_cpp.h` headers to become C++ header files.
It also removes the now useless `RNA_EXTERN_C` defines, and just
directly use the `extern` keyword. We do not need anymore `extern "C"`
declarations here.
Pull Request: https://projects.blender.org/blender/blender/pulls/124469
The option on NLA strips "Sync Length" (in the Action Clip dropdown of the N panel)
stops keys from being inserted if it is disabled.
This is due to the evaluation mode of the strip, which is set
to "Hold" internally but ONLY IF "Sync Length" is enabled.
Removing that condition allows to key in tweak mode regardless of that setting.
## History
This has been put in place by 89ee260ef2
Judging by the commit description, this was put in place to allow keyframing
in tweak mode. However, no explanation is given why this is only allowed
with "Sync Length" enabled. Potentially because there was no special
handling of tweak strips for keying evaluation, which has been put in
place later. (09709a7e64)
Pull Request: https://projects.blender.org/blender/blender/pulls/123902
Rename "Animation data-block" to "Action" or "Layered Action", where
appropriate. Some uses of the term actually refer to the `AnimData`
struct, in which case they were left as-is.
No real functional changes, just changing some messages & descriptions.
Pull Request: https://projects.blender.org/blender/blender/pulls/124170
Rename 'Binding' to 'Slot'. The old term was causing all kind of
confusion, and 'slot' was considered to be a better term for the
intended functionality.
This commit breaks existing blend files that were using the new layered
Action for their animation. The animation data will be lost due to the
rename, as there is no versioning code or DNA renaming logic. At this
time the new system is still marked as experimental, so shouldn't be
used for anything serious anyway.
Pull Request: https://projects.blender.org/blender/blender/pulls/124170
Avoid logging actions about `action->idroot` not matching, when evaluating
the NLA and visiting a layered Action. Layered Actions are not limited to
a single data-block type, and so the code should ignore `action->idroot`.
The documentation of some of `BKE_animsys_nla_remap_keyframe_values()`'s output
parameters were unclear/misleading, in particular making it unclear how those
output parameters should be used/interpreted by calling code.
This commit attempts to improve that, in particular trying to make it clearer
what actions calling code can/should take based on the returned output
parameters.
Pull Request: https://projects.blender.org/blender/blender/pulls/123081
The new/experimental, layered `Animation` data-block is merged with the
existing `bAction` data-block.
The `Animation` data-block is considerably newer than `bAction`, so the
supporting code that was written for it is also more modern. When moving
that code into `bAction`, I chose to keep the modernity where possible,
and thus some of the old code has been updated as well. Things like
preferring references over pointers.
The `Animation` data-block is now gone from DNA, the main database, etc.
As this was still an experimental feature, there is no versioning code
to convert any of that to Actions.
The DNA struct `bAction` now has a C++ wrapper `animrig::Action`, that
can be obtained via `some_action->wrap()`.
`animrig::Action` has functions `is_empty()`, `is_action_legacy()`, and
`is_action_layered()`. They **all** return `true` when the Action is
empty, as in that case none of the data that makes an action either
'legacy' or 'layered' is there.
The 'animation filtering' code (for showing things in the dope sheet,
graph editor, etc) that I wrote for `Animation` is intentionally kept
around. These types now target 'layered actions' and the
already-existing ones 'legacy actions'. A future PR may merge these two
together, but given how much work it was to add something new there, I'd
rather wait until the dust has settled on this commit.
There are plenty of variables (and some comments) named `anim` or
`animation` that now are of type `animrig::Action`. I haven't renamed
them all, to keep the noise level low in this commit (it's already big
enough). This can be done in a followup, non-functional PR.
Related task: #121355
Pull Request: https://projects.blender.org/blender/blender/pulls/121357
This makes the read and write API functions match more closely, and adds
asserts to check that the data size is as expected.
There are still a few places remaining that use BLO_read_data_address
and similar generic functions, these should eventually be replaced as well.
Pull Request: https://projects.blender.org/blender/blender/pulls/120994
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.
Pull Request: https://projects.blender.org/blender/blender/pulls/118677
The issue was that the `PointerRNA` passed to `BKE_animsys_get_nla_keyframing_context`
needs to point to an `ID` which wasn't the case when keying bones.
That is because internally the `FCurve` path is used to resolve the property.
This can only work from the `ID` because the `FCurve` path is always stored relative to that.
While the function doesn't fail when the property can't resolve the path, it won't actually do
the remapping when passing it to `BKE_animsys_nla_remap_keyframe_values` later.
Pull Request: https://projects.blender.org/blender/blender/pulls/120008
No functional changes.
This PR replaces the uses of the C-style `BLI_BITMAP`
with `blender::BitVector` in `keyframing.cc`.
Note that in `BKE_animsys_nla_remap_keyframe_values` I had to
add code that maps from one to the other because I don't feel
comfortable ripping out the `BLI_BITMAP` from
`NlaEvalChannelSnapshot->remap_domain`
Pull Request: https://projects.blender.org/blender/blender/pulls/118957
The depsgraph CoW mechanism is a bit of a misnomer. It creates an
evaluated copy for data-blocks regardless of whether the copy will
actually be written to. The point is to have physical separation between
original and evaluated data. This is in contrast to the commonly used
performance improvement of keeping a user count and copying data
implicitly when it needs to be changed. In Blender code we call this
"implicit sharing" instead. Importantly, the dependency graph has no
idea about the _actual_ CoW behavior in Blender.
Renaming this functionality in the despgraph removes some of the
confusion that comes up when talking about this, and will hopefully
make the depsgraph less confusing to understand initially too. Wording
like "the evaluated copy" (as opposed to the original data-block) has
also become common anyway.
Pull Request: https://projects.blender.org/blender/blender/pulls/118338
Along with the 4.1 libraries upgrade, we are bumping the clang-format
version from 8-12 to 17. This affects quite a few files.
If not already the case, you may consider pointing your IDE to the
clang-format binary bundled with the Blender precompiled libraries.
