Check if either the memory is zero or already matches the default value,
and copy. This simplifies a common pattern to a single line.
Preparing for default initializers in DNA (#134531).
Pull Request: https://projects.blender.org/blender/blender/pulls/138830
No functional changes intended.
This moves the functions
* ANIM_bone_is_visible
* ANIM_bone_is_visible_ebone
* ANIM_bone_is_visible_pchan
into new files `ANIM_armature.hh`/`armature.cc`.
They were previously in `ANIM_bone_collections.hh` but don't
directly have anything to do with bone collections.
It also puts the functions into the `blender::animrig::` namespace
and removes the `ANIM_` prefix as is the standard for C++ files.
Part of #138482
Pull Request: https://projects.blender.org/blender/blender/pulls/138833
No functional changes intended.
This just replaces all calls to `PBONE_VISIBLE` and `EBONE_VISIBLE`
with appropriate functions.
In the case of editbones it is just the function that the macro already contained.
For pose bones, a new function was added that mirrors what the macro had.
Using a function will make it easier to change how selection is queried in the future.
part of #138482
Pull Request: https://projects.blender.org/blender/blender/pulls/138819
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
This adds a version of `BKE_id_new_nomain` that takes the ID type parameter as
template argument. This allows the function the return the newly created ID with
the correct type, removing the need to use `static_cast` on the call-site.
To make this work, I added a static `id_type` member to every ID struct. This
can also be used to create a similar API for other id management functions in
future patches.
```cpp
// Old
Mesh *mesh = static_cast<Mesh *>(BKE_id_new_nomain(ID_ME, "Mesh"));
// New
Mesh *mesh = BKE_id_new_nomain<Mesh>("Mesh");
```
Pull Request: https://projects.blender.org/blender/blender/pulls/138383
It's safer to pass a type so that it can be checked if delete should be
used instead. Also changes a few void pointer casts to const_cast so that
if the data becomes typed it's an error.
Pull Request: https://projects.blender.org/blender/blender/pulls/137404
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
The general idea is to keep the 'old', C-style MEM_callocN signature, and slowly
replace most of its usages with the new, C++-style type-safer template version.
* `MEM_cnew<T>` allocation version is renamed to `MEM_callocN<T>`.
* `MEM_cnew_array<T>` allocation version is renamed to `MEM_calloc_arrayN<T>`.
* `MEM_cnew<T>` duplicate version is renamed to `MEM_dupallocN<T>`.
Similar templates type-safe version of `MEM_mallocN` will be added soon
as well.
Following discussions in !134452.
NOTE: For now static type checking in `MEM_callocN` and related are slightly
different for Windows MSVC. This compiler seems to consider structs using the
`DNA_DEFINE_CXX_METHODS` macro as non-trivial (likely because their default
copy constructors are deleted). So using checks on trivially
constructible/destructible instead on this compiler/system.
Pull Request: https://projects.blender.org/blender/blender/pulls/134771
When writing an Action to a blend file, for forward compatability
reasons the animation data in the first slot is also written to the old
Animato Action storage locations. That way, loading the Action in an
older version of Blender will still at least have that animation data.
However, we overlooked `Action.idroot`, which indicates which ID
type the Action is for, and therefore all Actions have an unspecified
`idroot` when loaded in pre-layered-action versions of Blender.
This PR fixes that by also setting the `idroot` to match the target ID
type of the first slot when writing to disk.
Pull Request: https://projects.blender.org/blender/blender/pulls/133819
The code for converting pre-Animato actions was not getting run properly
because `chanbase` (where animation data used to be stored) was
erroneously getting cleared before the relevant versioning code was run.
The root cause was that the code checking whether an action was already
valid as a layered action or not was NOT confirming that `chanbase` was
empty as part of that check (as it is a DNA-deprecated field), which in
turn triggered code that defensively clears `chanbase` (among other
things) when an action is identified as layered.
Note that the conversion of IPO curves and other pre-Animato data
happens quite late in the versioning, even _after_ the "versioning after
linking" stage. This is not introduced in this commit, this is just to
illuminate pre-existing design that might not be entirely obvious.
Pull Request: https://projects.blender.org/blender/blender/pulls/131975
Change how `action_foreach_id()` reports action slot users. This contains
two changes:
- Report the correct pointer reference, so that the ID remapping code
can actually change the pointers in the cache (necessary, for example,
to nil pointers to deleted IDs).
- Never report anything when it's known the slot user cache is dirty.
Pull Request: https://projects.blender.org/blender/blender/pulls/131808
Give the `IDWALK_CB_…` enum an explicit name:
`LibraryForeachIDCallbackFlag`. This way the flags are type-safe, and
it's known where values come from. This is much preferred (at least by
me) to just having `int flags`.
Uses of `0` have been replaced with `IDWALK_CB_NOP` as that has the same
value and is of the right type.
One invalid use of `IDWALK_NOP` was detected by this change, and is
replaced by `IDWALK_CB_NOP`.
This change might be incomplete; I gave the enum a name, fixed the
compiler errors, and then also updated assignments like `int cb_flag =
cb_data->cb_flag`. I might have missed some assignments to `int` though.
No functional changes.
Pull Request: https://projects.blender.org/blender/blender/pulls/131865
Give the `IDWALK_…` enum an explicit name: `LibraryForeachIDFlag`. This way
the flags are type-safe, and it's known where values come from. This is
much preferred (at least by me) to just having `int flags`.
Uses of `0` have been replaced with `IDWALK_NOP` as that has the same value
and is of the right type.
One invalid use of `IDWALK_CB_NOP` was detected by this change, and is
replaced by `IDWALK_NOP`.
This change might be incomplete; I gave the enum a name, and then fixed
the compiler errors.
No functional changes.
Pull Request: https://projects.blender.org/blender/blender/pulls/131865
Two commits that basically do the same thing for two `enum`s: give
them a name.
- the `IDWALK_…` enum → `LibraryForeachIDFlag`.
- the `IDWALK_CB_…` enum → `LibraryForeachIDCallbackFlag`.
This way the flags are type-safe, and it's known where values come
from. This is much preferred (at least by me) to just having `int
flags`.
Uses of `0` have been replaced with `IDWALK_NOP` and `IDWALK_CB_NOP`,
as those have the same value and are of the right type.
One invalid use of `IDWALK_NOP` was detected by this change, and is
replaced by `IDWALK_CB_NOP`. And another one in the opposite
direction.
This change might be incomplete; I gave the enum a name, fixed the
compiler errors, and then also updated assignments like `int cb_flag =
cb_data->cb_flag`. I might have missed some assignments to `int`
though.
No functional changes.
----------
I intend to land this PR as its two separate commits. I just put them in the same PR so the buildbot can handle them in one go, and we don't have a stack of highly relatled PRs.
In the future this could also apply to the `IDWALK_RET_…` enum. This one I left out, though, because a proper cleanup there would also have to include their ambiguity on whether they are bitflags (like the enums in this PR) or not. Their values and the code in `BKE_lib_query_foreachid_process()` implies they are bitflags, but in practice they are never or'ed together and just used as discrete values.
Pull Request: https://projects.blender.org/blender/blender/pulls/131803
The `Action::last_slot_handle` field is set to a high-ish value, to
disambiguate slot handles from array indices.
Slot handles are opaque integers, and are just used to find a slot
with that particular handle. Its exact value is irrelevant; Blender
only ensures that slot handles are never reused within the same
Action.
This particular value was obtained by taking the 31 most significant
bits of the TentHash value (Nathan's hash function) for the string
"Quercus&Laksa" (Sybren's cats).
Pull Request: https://projects.blender.org/blender/blender/pulls/131310
We've had a bunch of inconsistency between `channel_bag` and `channelbag` in the
code base. After discussion with @dr.sybren, we decided to standardize on
`channelbag` and also rename the camelcase `ChannelBag` to `Channelbag` to be
consistent with that.
This PR implements those changes.
Note that the reason we standardized on `channelbag` rather than `channel_bag`
is because it makes things clearer when stringing multiple terms together in
type and function names. For example, in `channelbag_fcurves_move()` it makes
it clear that `channelbag` describes one thing, rather than `channel` and `bag`
being two separate things.
No functional changes intended.
Pull Request: https://projects.blender.org/blender/blender/pulls/130988
When duplicating a bone (has to be done in edit mode)
the pose bone colors were not copied.
This adds the code to do just that
This also fixes it for symmetrising because that uses the
same code path
Pull Request: https://projects.blender.org/blender/blender/pulls/129007
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
# Fix 128078, Part I: Fix missing reverse endian switch of Action's `idroot`.
ID type code stored as ints (or shorts) need their endian switch to be
reverted (in case there is endianess conversion) on file read.
Interestingly, this was done for the deprecated IPO data (among others), but
not for the Action one!
NOTE: There is no versioning fix for this mistake, i.e. old files that
were saved from a BE system, then opened and re-saved from a LE system,
will still have totally invalid ID code values. This is not considered
as necessary currently, given that this `idroot` value is only
'informational' and not relied on by any part of the code.
# Fix 128078, Part II: GPv3 conversion code missing animation of Layers' location.
Also add code to the AnimDataConverter to ensure that actions get the
`idroot` matching their new ID owner type in GP data case.
Pull Request: https://projects.blender.org/blender/blender/pulls/128129
`ActionSlot::idtype` needs some extra care when reading from a blend
file. Blender's generic endian switching needs to be un-done, as the
ID type values are not numerically the same between little and big
endian machines. Due to the way they are defined, they are always in
the same byte order, regardless of hardware/platform endianness.
This is the same for Action Slots as #128129 does for Actions, except
that Action Slots use a `int16_t` instead of `short` and thus don't
need a cast.
Pull Request: https://projects.blender.org/blender/blender/pulls/128438
Refactor `ED_curve_updateAnimPaths()` and the static functions it calls,
separating out:
- the manipulation of the F-Curves themselves, and
- the manipulation of whatever container holds those F-Curves.
This will make it considerably easier to convert the code to deal with a
`Span<FCurve *>` later instead of `ListBase`, which in turn is necessary
to handle layered Actions.
The old code used to move F-Curves from one `ListBase` to another, to
avoid visiting the same F-Curve twice. This is now done by tracking
those F-Curves in a `Set<FCurve *>`, so that it doesn't require
manipulation of the storage itself.
No functional changes.
In `BKE_action.hh`, update the Action Group related functions, so that
either they are legacy-only or can handle layered Actions as well.
Legacy-only functions not only are documented as such, but also assert
that the given Action is a legacy one.
No functional changes for legacy Actions.
Ref: #123424
Pull Request: https://projects.blender.org/blender/blender/pulls/128088
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
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
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
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
For an NLA strip to use a slotted Action, it needs to specify which slot
to use in that action. This is now handled by two new properties on the
strip in DNA & RNA: `action_slot_handle` and `action_slot_name`.
These serve the same purpose as their counterparts on the `AnimData`
struct.
Note that this commit does NOT add NLA evaluation support for slotted
Actions. It merely allows assigning them. Evaluation, tweak mode
support, etc. will be implemented in future commits.
Pull Request: https://projects.blender.org/blender/blender/pulls/127359
Fix a crash that happens when drawing Action groups in the dope sheet, when
Blender is built without experimental features (hence without Action
slots) and it has loaded a blend file with slotted Actions.
Basically a pointer needs to be set to `nullptr` when reading such a file,
as it ensures that the rest of the code treats the group as "from a legacy
Action".
Pull Request: https://projects.blender.org/blender/blender/pulls/127339
Resovle regression in [0].
Pointers in `action.chanbase` were used without first loading the
list-base. In this case the intention was to skip loading legacy
data, so clear the list instead.
[0]: c7bf1a697e
The issue was that the forward compatibility writing code for channel groups in
layered actions was building a temporary legacy listbase, but was not clearing
it properly afterwards. This was then getting caught by an assert that ensured
that layered-action groups didn't have legacy data in them.
The reason the listbase wasn't getting cleared properly is because the
prev/next listbase pointers were getting cleared using a `LISTBASE_FOREACH`
loop, and thus the loop never progressed past the first item.
Additionally, this mistake wasn't just in the channel groups writing code, but
also the forward compatibility writing code for fcurves.
This fixes the issue in both places by switching the loops to
use`LISTBASE_FOREACH_MUTABLE`.
Based on discussion with @dr.sybren, this also removes the assert that caught
the issue. The situation it guards against is actually completely benign, and
the existence of the assert is contrary to the comments in the forward-compat
writing code explaining why its approach is okay.
Pull Request: https://projects.blender.org/blender/blender/pulls/126970
