Crash happens in `action_group_colors_set_from_posebone` /
`ANIM_bonecolor_posebone_get` on a `bPoseChannel` without a `bone`.
If I am not mistaken a new `bPoseChannel` (e.g. after duplication) will
only get its `bone` after leaving editmode.
So in a way the situation is similar to 2a8ce1f121
Behavior of `animchan_sync_group` is not reliable in a way that getting
a `bPoseChannel` from an `bActionGroup` will guarantee these are really
corresponding. So usually, if you dulplicate/symmetrize a bone, there
would be no corresponding `bActionGroup` and nothing would happen
really. But you could for example group fcurves from `Bone` under a
group called `Bone.001` and vice versa. This is totally allowed to do.
In this case, `animchan_sync_group` is doing nothing totally helpful, so
it could find the "wrong" `bPoseChannel`. And it could try
`action_group_colors_set_from_posebone` with that `bPoseChannel` which
still does not have a `bone` and then crash.
So now only do this if we have a valid `bone`.
Pull Request: https://projects.blender.org/blender/blender/pulls/118676
The `object_to_world` and `world_to_object` matrices are set during
depsgraph evaluation, calculated from the object's animated location,
rotation, scale, parenting, and constraints. It's confusing and
unnecessary to store them with the original data in DNA.
This commit moves them to `ObjectRuntime` and moves the matrices to
use the C++ `float4x4` type, giving the potential for simplified code
using the C++ abstractions. The matrices are accessible with functions
on `Object` directly since they are used so commonly. Though for write
access, directly using the runtime struct is necessary.
The inverse `world_to_object` matrix is often calculated before it's
used, even though it's calculated as part of depsgraph evaluation.
Long term we might not want to store this in `ObjectRuntime` at all,
and just calculate it on demand. Or at least we should remove the
redundant calculations. That should be done separately though.
Pull Request: https://projects.blender.org/blender/blender/pulls/118210
The new keyframing functions introduced in #113504
didn't call the functions to decompose the NLA stack.
In practice this meant that when inserting keys into strip
that is under an additive strip, it would take the result of the additive Strip and
bake it back into the base. This would double the transform.
The fix is to call `BKE_animsys_nla_remap_keyframe_values`.
Unfortunately to do so, I had to pass through a few more
arguments to the keyframing functions.
Also adds unit tests to cover the caused bug.
Pull Request: https://projects.blender.org/blender/blender/pulls/118053
Caused by #113504
While basic cycle aware keying works, there is a special case
when inserting the first key. In that case, after the key has been inserted,
it is duplicated and moved around so the FCurve range from
first to last key is exactly the range of the action.
It also auto-creates the Cycle modifier on the FCurve .
Fix the issue by calling the function that does the key duplication
and cover with unit tests.
Pull Request: https://projects.blender.org/blender/blender/pulls/116943
Store the 'expanded/collapsed' state of the bone collection tree view in
the DNA data of the bone collections themselves. This way the tree state
is restored when loading the file.
This commit also adds some code to the abstract tree view classes, for
supporting synchronisation of the extended/collapsed state between it
and external data. It follows the same approach as the handling of the
active element.
RNA wrappers have been added to make it possible for Python code to
expand/collapse parts of the tree.
Library overrides are supported for this property, so the
expanded/collapsed state of linked armatures can be locally saved. If
there is no override, the `is_expanded` property is still editable;
changes will not be saved to file in that case, though.
Pull Request: https://projects.blender.org/blender/blender/pulls/116940
This significantly simplifies memory management, mostly by avoiding
the need to free the memory manually. It may also improve performance,
since std::string has an inline buffer that can prevent heap
allocations and it stores the size.
Pull Request: https://projects.blender.org/blender/blender/pulls/117695
After removing a bone collection that was marked as 'solo', refresh
the armature's 'solo active' flag. The deleted bone collection might have
been the last solo'ed one.
Add the 'solo' flag to bone collections, effectively adding another
layer of visibility controls.
If there is _any_ bone collection with this flag enabled, only this
collection (and others with this flag enabled) will be visible.
In RNA, the following properties are exposed:
- `bone_collection.is_solo`: writable property to manage the solo flag.
- `armature.is_solo_active`: read-only property that is `True` when any
bone collection has `is_solo = True`.
The RNA property `bone_collection.is_visible_effectively` now also takes
the solo flag into account.
Pull Request: https://projects.blender.org/blender/blender/pulls/117414
Rename `BoneCollection::is_visible_effectively()` to
`is_visible_with_ancestors()`. Soon a "solo" flag will be introduced,
and the effective visibility of the bone collection will depend on that
too. This particular function doesn't take that into account, though,
and thus needs a rename.
Note that this does NOT rename the RNA property
`is_visible_effectively`. That will be updated when the "solo" flag is
introduced to also take that into account.
No functional changes.
Pull Request: https://projects.blender.org/blender/blender/pulls/117414
Splits the flag `..._FLAG_INSERTNEEDED` between autokey and
manual keying. The fact that this flag was shared between the two
systems has been the cause of issues in the past. It wouldn't
let you insert a keyframe even though you explicitly used an operator
to do so.
In order to be clearer what options are used where, the user preferences
have been reordered.
By default "Only Insert Needed" will be enabled for auto-keying, but not for manual keying.
The versioning code will enable both if it was enabled previously.
# Code side changes
The keying system has flags that define the behavior
when keys are inserted. Some of those flags were shared
between keying and auto-keying. Some were only used for
auto-keying.
To clarify that, prefix flags that used exclusively in one or the other
system with `AUTOKEY`/`MANUALKEY`
Also the flag name on the user preferences and the tool settings was renamed.
Previously it was called `autokey_flag`. To indicated that it is not only used
for autokeying, rename it `keying_flag`.
Fixes: #73773
Pull Request: https://projects.blender.org/blender/blender/pulls/115525
No functional changes.
In #116823 the function `remap_driver_frame` was moved,
but the name doesn't exactly correspond to what the function is doing.
Since it evaluates the driver, with different parameters, it is now called
`evaluate_driver_from_rna_pointer`.
We could maybe remove the function because it is only called once.
Pull Request: https://projects.blender.org/blender/blender/pulls/117294
No functional changes expected.
Before this PR the keyframing code was aware of driver specifics
and changed the frame where a key is inserted to the current driver value.
This is now changed so that the calling code needs to know where on the
FCurve the key should be inserted. This removes the need for the
`INSERTKEY_DRIVER` flag.
Pull Request: https://projects.blender.org/blender/blender/pulls/116823
Fix an issue where, after deleting the active bone collection, the
incorrect bone collection became the active one.
The solution was to store the active bone collection index before
deletion, instead of after.
When autokeying a property with a driver, the code added
the keyframe into the drivers FCurve.
Judging by the code the intention was to be able to quickly
set up driven keys by modifying the driven value and have that
reflected in the driver curve.
However that idea was blocked by the fact that you can't actually
change the value of a property that is driven.
In addition to that it's quite unexpected and the result is hardly
communicated to the user.
The solution is to not insert keyframes to drivers using the
autokeying system.
Also Fixes#95866
This was discussed in the A&R module meeting
https://devtalk.blender.org/t/2024-01-11-animation-rigging-module-meeting/32888#patch-review-decision-time-5
and the consensus was the feature to set up driven keys would be great,
but since it's not working at all currently it's better to get rid of the bugs.
Pull Request: https://projects.blender.org/blender/blender/pulls/116927
The previous commit introduced a new `RPT_()` macro to translate
strings which are not tooltips or regular interface elements, but
longer reports or statuses.
This commit uses the new macro to translate many strings all over the
UI.
Most of it is a simple replace from `TIP_()` or `IFACE_()` to
`RPT_()`, but there are some additional changes:
- A few translations inside `BKE_report()` are removed altogether
because they are already handled by the translation system.
- Messages inside `UI_but_disable()` are no longer translated
manually, but they are handled by a new regex in the translation
system.
Pull Request: https://projects.blender.org/blender/blender/pulls/116804
Pull Request: https://projects.blender.org/blender/blender/pulls/116804
`armature.collections.move(x, y)` now retains the active collection. Due to
the move, the active collection index can change. The index is now updated
to accomodate for this.
Ensure that the active bone collection doesn't change, when adding a new
bone collection via low-level functions (for example
`armature.collections.new()` in Python).
The change to the active bone collection happened because adding a new
child may change the index of the active bone collection.
Instead of moving bone collections by absolute index, they can now be
moved by manipulating `.child_number`. This is the relative index of the
bone collection within the list of its siblings.
This replaces the much more cumbersome `collections.move_to_parent()`
function. Since that function is now no longer necessary (it's been
replaced by assignment to `.parent` and `.child_number`), it's removed
from RNA. Note that this function was never part of even a beta build of
Blender.
The `expect_bcolls()` function now no longer calls the `EXPECT_EQ` macro,
but returns a `testing::AssertionResult` instead. The function call does
need to be wrapped in an `EXPECT_TRUE()` call now, but that also means that
any failure message points directly to the call site.
