diff --git a/source/blender/animrig/ANIM_action.hh b/source/blender/animrig/ANIM_action.hh index 455d7f32739..30905c7a9a9 100644 --- a/source/blender/animrig/ANIM_action.hh +++ b/source/blender/animrig/ANIM_action.hh @@ -753,6 +753,73 @@ class ChannelBag : public ::ActionChannelBag { */ void fcurves_clear(); + /* Channel group access. */ + blender::Span channel_groups() const; + blender::MutableSpan channel_groups(); + const bActionGroup *channel_group(int64_t index) const; + bActionGroup *channel_group(int64_t index); + + /** + * Find the first bActionGroup (channel group) with the given name. + * + * Note that channel groups with the same name are allowed, and this simply + * returns the first match. + * + * If no matching group is found, `nullptr` is returned. + */ + const bActionGroup *channel_group_find(StringRef name) const; + bActionGroup *channel_group_find(StringRef name); + + /** + * Find the channel group that contains the fcurve at `fcurve_array_index` as + * a member. + * + * \return The index of the channel group if found, or -1 if no such group is + * found. + */ + int channel_group_containing_index(int fcurve_array_index); + + /** + * Create a new empty channel group with the given name. + * + * The new group is added to the end of the channel group array of the + * ChannelBag. + * + * \return A reference to the new channel group. + */ + bActionGroup &channel_group_create(StringRefNull name); + + /** + * Find a channel group with the given name, or if none exists create one. + * + * If a new group is created, it's added to the end of the channel group array + * of the ChannelBag. + * + * \return A reference to the channel group. + */ + bActionGroup &channel_group_ensure(StringRefNull name); + + /** + * Remove the given channel group from the channel bag. + * + * Any fcurves that were part of this group will me moved to just after all + * grouped fcurves. + * + * \return true when the channel group was found & removed, false if it wasn't + * found. + */ + bool channel_group_remove(bActionGroup &group); + + /** + * Assigns the given FCurve to the given channel group. + * + * Fails if either doesn't belong to this channel bag, but otherwise always + * succeeds. + * + * \return True on success, false on failure. + */ + bool fcurve_assign_to_channel_group(FCurve &fcurve, bActionGroup &group); + protected: /** * Create an F-Curve. @@ -767,6 +834,79 @@ class ChannelBag : public ::ActionChannelBag { * responsibility of the caller. */ FCurve &fcurve_create(Main *bmain, FCurveDescriptor fcurve_descriptor); + + private: + /** + * Remove the channel group at `channel_group_index` from the channel group + * array. + * + * This is a low-level function that *only* manipulates the channel group + * array in the most basic way. It literally just removes the given item from + * the array and frees it, just like `erase()` on `std::vector`. + * + * It specifically does *not* maintain any of the semantic invariants of the + * group array or its relationship to the fcurves. + * + * Both `collapse_channel_group_gaps()` and + * `update_fcurve_channel_group_pointers()` should be called at some point + * after this to restore the semantic invariants. + * + * \see `collapse_channel_group_gaps()` + * + * \see `update_fcurve_channel_group_pointers()` + */ + void channel_group_remove_raw(int channel_group_index); + + /** + * Move channel groups' fcurve spans so that there are no gaps between them, + * and to start at the first fcurve. + * + * This does *not* alter the order of the channel groups nor the number of + * fcurves in each group. It simply changes the start indices of each group so + * that the groups are packed together at the start of the fcurves. + * + * For example, if the mapping of groups to fcurves looks like this (g* are + * the groups, dots indicate ungrouped areas, and f* are the fcurves, so e.g. + * f1 and f2 are part of group g0): + * + * ``` + * ..| g0 |..|g1|.....| g2 |.. + * |f0|f1|f2|f3|f4|f5|f6|f7|f8|f9| + * ``` + * + * Then after calling this function they will look like this: + * + * ``` + * | g0 |g1| g2 |.............. + * |f0|f1|f2|f3|f4|f5|f6|f7|f8|f9| + * ``` + * + * Note that this specifically does *not* move the fcurves, and therefore this + * alters fcurve membership in a way that depends on how the groups are + * shifted. It also does not update the group pointers inside the fcurves, so + * `update_fcurve_channel_group_pointers()` must be called at some point after + * this to fix those up. + * + * This upholds critical invariants and should be called any time gaps might + * be introduced (changing the fcurve span of or removing a group). + * + * \see `update_fcurve_channel_group_pointers()` + */ + void collapse_channel_group_gaps(); + + /** + * Updates all fcurves to point at the channel group that they belong to. + * + * The indices in the channel groups are considered the source of truth, and + * the pointers in the fcurves are simply updated to match those. Fcurves + * that don't belong to a group will have their group pointer set to null. + * + * This upholds critical invariants and should always be called after + * modifications to either the array of fcurves (changing the array position + * of, adding, or removing fcurves) or to the array of groups (changing the + * fcurve span of, removing, or adding groups). + */ + void update_fcurve_channel_group_pointers(); }; static_assert(sizeof(ChannelBag) == sizeof(::ActionChannelBag), "DNA struct and its C++ wrapper must have the same size"); @@ -832,6 +972,24 @@ Action *get_action(ID &animated_id); */ std::optional> get_action_slot_pair(ID &animated_id); +const animrig::ChannelBag *channelbag_for_action_slot(const Action &action, + slot_handle_t slot_handle); +animrig::ChannelBag *channelbag_for_action_slot(Action &action, slot_handle_t slot_handle); + +/** + * Return the channel groups for this specific slot handle. + * + * This is just a utility function, that's intended to become obsolete when multi-layer Actions + * are introduced. However, since Blender currently only supports a single layer with a single + * strip, of a single type, this function can be used. + * + * The use of this function is also an indicator for code that will have to be altered when + * multi-layered Actions are getting implemented. + */ +Span channel_groups_for_action_slot(Action &action, slot_handle_t slot_handle); +Span channel_groups_for_action_slot(const Action &action, + slot_handle_t slot_handle); + /** * Return the F-Curves for this specific slot handle. * diff --git a/source/blender/animrig/ANIM_fcurve.hh b/source/blender/animrig/ANIM_fcurve.hh index 6afdfd24b09..21840cf330a 100644 --- a/source/blender/animrig/ANIM_fcurve.hh +++ b/source/blender/animrig/ANIM_fcurve.hh @@ -28,6 +28,7 @@ struct FCurveDescriptor { StringRefNull rna_path; int array_index; std::optional prop_subtype; + std::optional channel_group; }; /* This is used to pass in the settings for a keyframe into a function. */ diff --git a/source/blender/animrig/intern/action.cc b/source/blender/animrig/intern/action.cc index c7ebd6e34c9..34e82834baf 100644 --- a/source/blender/animrig/intern/action.cc +++ b/source/blender/animrig/intern/action.cc @@ -111,6 +111,23 @@ template static void grow_array_and_append(T **array, int *num, T it (*array)[*num - 1] = item; } +template +static void grow_array_and_insert(T **array, int *num, const int index, T item) +{ + BLI_assert(index >= 0 && index <= *num); + const int new_array_num = *num + 1; + T *new_array = MEM_cnew_array(new_array_num, __func__); + + blender::uninitialized_relocate_n(*array, index, new_array); + new_array[index] = item; + blender::uninitialized_relocate_n(*array + index, *num - index, new_array + index + 1); + + MEM_SAFE_FREE(*array); + + *array = new_array; + *num = new_array_num; +} + template static void shrink_array(T **array, int *num, const int shrink_num) { BLI_assert(shrink_num > 0); @@ -124,6 +141,55 @@ template static void shrink_array(T **array, int *num, const int shr *num = new_array_num; } +template static void shrink_array_and_remove(T **array, int *num, const int index) +{ + BLI_assert(index >= 0 && index < *num); + const int new_array_num = *num - 1; + T *new_array = MEM_cnew_array(new_array_num, __func__); + + blender::uninitialized_move_n(*array, index, new_array); + blender::uninitialized_move_n(*array + index + 1, *num - index - 1, new_array + index); + MEM_freeN(*array); + + *array = new_array; + *num = new_array_num; +} + +/** + * Moves the given (end exclusive) range to index `to`, shifting other items + * before/after to make room. + * + * The range is moved such that the *start* ends up at `to`. + * + * `to` *must* be far away enough from the end of the array for the entire range + * to be moved there without spilling over the end of the array. + */ +template +static void array_shift_range( + T *array, const int num, const int range_start, const int range_end, const int to) +{ + BLI_assert(range_start <= range_end); + BLI_assert(range_end <= num); + BLI_assert(to <= num + range_start - range_end); + + if (range_start == range_end || range_start == to) { + return; + } + + if (to < range_start) { + T *start = array + to; + T *mid = array + range_start; + T *end = array + range_end; + std::rotate(start, mid, end); + } + else { + T *start = array + range_start; + T *mid = array + range_end; + T *end = array + to + range_end - range_start; + std::rotate(start, mid, end); + } +} + /* ----- Action implementation ----------- */ bool Action::is_empty() const @@ -1163,7 +1229,19 @@ FCurve &ChannelBag::fcurve_create(Main *bmain, FCurveDescriptor fcurve_descripto new_fcurve->flag |= FCURVE_ACTIVE; /* First curve is added active. */ } - grow_array_and_append(&this->fcurve_array, &this->fcurve_array_num, new_fcurve); + bActionGroup *group = fcurve_descriptor.channel_group.has_value() ? + &this->channel_group_ensure(*fcurve_descriptor.channel_group) : + nullptr; + int insert_index = group ? group->fcurve_range_start + group->fcurve_range_length : + this->fcurve_array_num; + BLI_assert(insert_index <= this->fcurve_array_num); + + grow_array_and_insert(&this->fcurve_array, &this->fcurve_array_num, insert_index, new_fcurve); + if (group) { + group->fcurve_range_length += 1; + this->collapse_channel_group_gaps(); + this->update_fcurve_channel_group_pointers(); + } if (bmain) { DEG_relations_tag_update(bmain); @@ -1184,6 +1262,19 @@ bool ChannelBag::fcurve_remove(FCurve &fcurve_to_remove) return false; } + const int group_index = this->channel_group_containing_index(fcurve_index); + if (group_index != -1) { + bActionGroup *group = this->channel_groups()[group_index]; + + group->fcurve_range_length -= 1; + if (group->fcurve_range_length <= 0) { + const int group_index = this->channel_groups().as_span().first_index_try(group); + this->channel_group_remove_raw(group_index); + } + this->collapse_channel_group_gaps(); + this->update_fcurve_channel_group_pointers(); + } + dna::array::remove_index( &this->fcurve_array, &this->fcurve_array_num, nullptr, fcurve_index, fcurve_ptr_destructor); @@ -1259,13 +1350,20 @@ SingleKeyingResult KeyframeStrip::keyframe_insert(Main *bmain, ChannelBag::ChannelBag(const ChannelBag &other) { this->slot_handle = other.slot_handle; - this->fcurve_array_num = other.fcurve_array_num; + this->fcurve_array_num = other.fcurve_array_num; this->fcurve_array = MEM_cnew_array(other.fcurve_array_num, __func__); for (int i = 0; i < other.fcurve_array_num; i++) { const FCurve *fcu_src = other.fcurve_array[i]; this->fcurve_array[i] = BKE_fcurve_copy(fcu_src); } + + this->group_array_num = other.group_array_num; + this->group_array = MEM_cnew_array(other.group_array_num, __func__); + for (int i = 0; i < other.group_array_num; i++) { + const bActionGroup *group_src = other.group_array[i]; + this->group_array[i] = static_cast(MEM_dupallocN(group_src)); + } } ChannelBag::~ChannelBag() @@ -1275,6 +1373,12 @@ ChannelBag::~ChannelBag() } MEM_SAFE_FREE(this->fcurve_array); this->fcurve_array_num = 0; + + for (bActionGroup *group : this->channel_groups()) { + MEM_SAFE_FREE(group); + } + MEM_SAFE_FREE(this->group_array); + this->group_array_num = 0; } blender::Span ChannelBag::fcurves() const @@ -1294,11 +1398,189 @@ FCurve *ChannelBag::fcurve(const int64_t index) return this->fcurve_array[index]; } +blender::Span ChannelBag::channel_groups() const +{ + return blender::Span{this->group_array, this->group_array_num}; +} +blender::MutableSpan ChannelBag::channel_groups() +{ + return blender::MutableSpan{this->group_array, this->group_array_num}; +} +const bActionGroup *ChannelBag::channel_group(const int64_t index) const +{ + BLI_assert(index < this->group_array_num); + return this->group_array[index]; +} +bActionGroup *ChannelBag::channel_group(const int64_t index) +{ + BLI_assert(index < this->group_array_num); + return this->group_array[index]; +} + +const bActionGroup *ChannelBag::channel_group_find(const StringRef name) const +{ + for (const bActionGroup *group : this->channel_groups()) { + if (name == StringRef{group->name}) { + return group; + } + } + + return nullptr; +} + +bActionGroup *ChannelBag::channel_group_find(const StringRef name) +{ + /* Intermediate variable needed to disambiguate const/non-const overloads. */ + Span groups = this->channel_groups(); + for (bActionGroup *group : groups) { + if (name == StringRef{group->name}) { + return group; + } + } + + return nullptr; +} + +int ChannelBag::channel_group_containing_index(const int fcurve_array_index) +{ + int i = 0; + for (bActionGroup *group : this->channel_groups()) { + if (fcurve_array_index >= group->fcurve_range_start && + fcurve_array_index < (group->fcurve_range_start + group->fcurve_range_length)) + { + return i; + } + i++; + } + + return -1; +} + +bActionGroup &ChannelBag::channel_group_create(StringRefNull name) +{ + bActionGroup *new_group = static_cast( + MEM_callocN(sizeof(bActionGroup), __func__)); + + /* Find the end fcurve index of the current channel groups, to be used as the + * start of the new channel group. */ + int fcurve_index = 0; + const int length = this->channel_groups().size(); + if (length > 0) { + bActionGroup *last = this->channel_group(length - 1); + fcurve_index = last->fcurve_range_start + last->fcurve_range_length; + } + new_group->fcurve_range_start = fcurve_index; + + new_group->channel_bag = this; + + /* Make it selected. */ + new_group->flag = AGRP_SELECTED; + + /* Ensure it has a unique name. */ + std::string unique_name = BLI_uniquename_cb( + [&](const StringRef name) { + for (bActionGroup *group : this->channel_groups()) { + if (STREQ(group->name, name.data())) { + return true; + } + } + return false; + }, + '.', + name[0] == '\0' ? DATA_("Group") : name); + + STRNCPY_UTF8(new_group->name, unique_name.c_str()); + + grow_array_and_append(&this->group_array, &this->group_array_num, new_group); + + return *new_group; +} + +bActionGroup &ChannelBag::channel_group_ensure(StringRefNull name) +{ + bActionGroup *group = this->channel_group_find(name); + if (group) { + return *group; + } + + return this->channel_group_create(name); +} + +bool ChannelBag::channel_group_remove(bActionGroup &group) +{ + const int group_index = this->channel_groups().as_span().first_index_try(&group); + if (group_index == -1) { + return false; + } + + /* Move the group's fcurves to just past the end of where the grouped + * fcurves will be after this group is removed. */ + bActionGroup *last_group = this->channel_groups().last(); + BLI_assert(last_group != nullptr); + const int to_index = last_group->fcurve_range_start + last_group->fcurve_range_length - + group.fcurve_range_length; + array_shift_range(this->fcurve_array, + this->fcurve_array_num, + group.fcurve_range_start, + group.fcurve_range_start + group.fcurve_range_length, + to_index); + + this->channel_group_remove_raw(group_index); + this->collapse_channel_group_gaps(); + this->update_fcurve_channel_group_pointers(); + + return true; +} + +void ChannelBag::channel_group_remove_raw(const int group_index) +{ + BLI_assert(group_index >= 0 && group_index < this->channel_groups().size()); + + MEM_SAFE_FREE(this->group_array[group_index]); + shrink_array_and_remove(&this->group_array, &this->group_array_num, group_index); +} + +void ChannelBag::collapse_channel_group_gaps() +{ + int index = 0; + + for (bActionGroup *group : this->channel_groups()) { + group->fcurve_range_start = index; + index += group->fcurve_range_length; + } + + BLI_assert(index <= this->fcurve_array_num); +} + +void ChannelBag::update_fcurve_channel_group_pointers() +{ + Span groups = this->channel_groups(); + for (bActionGroup *group : groups) { + for (FCurve *fcurve : + this->fcurves().slice(group->fcurve_range_start, group->fcurve_range_length)) + { + fcurve->grp = group; + } + } + + int first_ungrouped_fcurve_index = 0; + if (!groups.is_empty()) { + first_ungrouped_fcurve_index = groups.last()->fcurve_range_start + + groups.last()->fcurve_range_length; + } + + for (FCurve *fcurve : this->fcurves().drop_front(first_ungrouped_fcurve_index)) { + fcurve->grp = nullptr; + } +} + /* Utility function implementations. */ -static const animrig::ChannelBag *channelbag_for_action_slot(const Action &action, - const slot_handle_t slot_handle) +const animrig::ChannelBag *channelbag_for_action_slot(const Action &action, + const slot_handle_t slot_handle) { + assert_baklava_phase_1_invariants(action); + if (slot_handle == Slot::unassigned) { return nullptr; } @@ -1320,14 +1602,35 @@ static const animrig::ChannelBag *channelbag_for_action_slot(const Action &actio return nullptr; } -static animrig::ChannelBag *channelbag_for_action_slot(Action &action, - const slot_handle_t slot_handle) +animrig::ChannelBag *channelbag_for_action_slot(Action &action, const slot_handle_t slot_handle) { const animrig::ChannelBag *const_bag = channelbag_for_action_slot( const_cast(action), slot_handle); return const_cast(const_bag); } +Span channel_groups_for_action_slot(Action &action, + const slot_handle_t slot_handle) +{ + assert_baklava_phase_1_invariants(action); + animrig::ChannelBag *bag = channelbag_for_action_slot(action, slot_handle); + if (!bag) { + return {}; + } + return bag->channel_groups(); +} + +Span channel_groups_for_action_slot(const Action &action, + const slot_handle_t slot_handle) +{ + assert_baklava_phase_1_invariants(action); + const animrig::ChannelBag *bag = channelbag_for_action_slot(action, slot_handle); + if (!bag) { + return {}; + } + return bag->channel_groups(); +} + Span fcurves_for_action_slot(Action &action, const slot_handle_t slot_handle) { assert_baklava_phase_1_invariants(action); @@ -1549,6 +1852,44 @@ bool action_fcurve_remove(Action &action, FCurve &fcu) return false; } +bool ChannelBag::fcurve_assign_to_channel_group(FCurve &fcurve, bActionGroup &to_group) +{ + + if (this->channel_groups().as_span().first_index_try(&to_group) == -1) { + return false; + } + + const int fcurve_index = this->fcurves().as_span().first_index_try(&fcurve); + if (fcurve_index == -1) { + return false; + } + + const int from_group_index = this->channel_group_containing_index(fcurve_index); + if (from_group_index != -1) { + bActionGroup *from_group = this->channel_groups()[from_group_index]; + if (from_group == &to_group) { + return true; + } + + from_group->fcurve_range_length--; + if (from_group->fcurve_range_length == 0) { + this->channel_group_remove_raw(from_group_index); + } + } + + array_shift_range(this->fcurve_array, + this->fcurve_array_num, + fcurve_index, + fcurve_index + 1, + to_group.fcurve_range_start + to_group.fcurve_range_length); + to_group.fcurve_range_length++; + + this->collapse_channel_group_gaps(); + this->update_fcurve_channel_group_pointers(); + + return true; +} + ID *action_slot_get_id_for_keying(Main &bmain, Action &action, const slot_handle_t slot_handle, diff --git a/source/blender/animrig/intern/action_test.cc b/source/blender/animrig/intern/action_test.cc index 246f434dbe9..72c57dd25dd 100644 --- a/source/blender/animrig/intern/action_test.cc +++ b/source/blender/animrig/intern/action_test.cc @@ -734,4 +734,317 @@ TEST_F(ActionLayersTest, empty_to_layered) ASSERT_FALSE(converted->is_action_legacy()); } +/*-----------------------------------------------------------*/ + +class ChannelBagTest : public testing::Test { + public: + ChannelBag *channel_bag; + + static void SetUpTestSuite() {} + + static void TearDownTestSuite() {} + + void SetUp() override + { + channel_bag = new ChannelBag(); + } + + void TearDown() override + { + delete channel_bag; + } +}; + +TEST_F(ChannelBagTest, channel_group_create) +{ + ASSERT_TRUE(channel_bag->channel_groups().is_empty()); + + bActionGroup *group0 = &channel_bag->channel_group_create("Foo"); + ASSERT_EQ(channel_bag->channel_groups().size(), 1); + EXPECT_EQ(StringRef{group0->name}, StringRef{"Foo"}); + EXPECT_EQ(group0->fcurve_range_start, 0); + EXPECT_EQ(group0->fcurve_range_length, 0); + EXPECT_EQ(group0, channel_bag->channel_group(0)); + + /* Set for testing purposes. Does not reflect actual fcurves in this test. */ + group0->fcurve_range_length = 2; + + bActionGroup *group1 = &channel_bag->channel_group_create("Bar"); + ASSERT_EQ(channel_bag->channel_groups().size(), 2); + EXPECT_EQ(StringRef{group1->name}, StringRef{"Bar"}); + EXPECT_EQ(group1->fcurve_range_start, 2); + EXPECT_EQ(group1->fcurve_range_length, 0); + EXPECT_EQ(group0, channel_bag->channel_group(0)); + EXPECT_EQ(group1, channel_bag->channel_group(1)); + + /* Set for testing purposes. Does not reflect actual fcurves in this test. */ + group1->fcurve_range_length = 1; + + bActionGroup *group2 = &channel_bag->channel_group_create("Yar"); + ASSERT_EQ(channel_bag->channel_groups().size(), 3); + EXPECT_EQ(StringRef{group2->name}, StringRef{"Yar"}); + EXPECT_EQ(group2->fcurve_range_start, 3); + EXPECT_EQ(group2->fcurve_range_length, 0); + EXPECT_EQ(group0, channel_bag->channel_group(0)); + EXPECT_EQ(group1, channel_bag->channel_group(1)); + EXPECT_EQ(group2, channel_bag->channel_group(2)); +} + +TEST_F(ChannelBagTest, channel_group_remove) +{ + bActionGroup *group0 = &channel_bag->channel_group_create("Group0"); + bActionGroup *group1 = &channel_bag->channel_group_create("Group1"); + bActionGroup *group2 = &channel_bag->channel_group_create("Group2"); + + FCurve *fcu0 = &channel_bag->fcurve_ensure(nullptr, {"fcu0", 0, std::nullopt, "Group0"}); + FCurve *fcu1 = &channel_bag->fcurve_ensure(nullptr, {"fcu1", 0, std::nullopt, "Group0"}); + FCurve *fcu2 = &channel_bag->fcurve_ensure(nullptr, {"fcu2", 0, std::nullopt, "Group2"}); + FCurve *fcu3 = &channel_bag->fcurve_ensure(nullptr, {"fcu3", 0, std::nullopt, "Group2"}); + FCurve *fcu4 = &channel_bag->fcurve_ensure(nullptr, {"fcu4", 0, std::nullopt, std::nullopt}); + + ASSERT_EQ(3, channel_bag->channel_groups().size()); + ASSERT_EQ(5, channel_bag->fcurves().size()); + + /* Attempt to remove a group that's not in the channel bag. Shouldn't do + * anything. */ + bActionGroup bogus; + EXPECT_EQ(false, channel_bag->channel_group_remove(bogus)); + ASSERT_EQ(3, channel_bag->channel_groups().size()); + ASSERT_EQ(5, channel_bag->fcurves().size()); + EXPECT_EQ(group0, channel_bag->channel_group(0)); + EXPECT_EQ(group1, channel_bag->channel_group(1)); + EXPECT_EQ(group2, channel_bag->channel_group(2)); + EXPECT_EQ(fcu0, channel_bag->fcurve(0)); + EXPECT_EQ(fcu1, channel_bag->fcurve(1)); + EXPECT_EQ(fcu2, channel_bag->fcurve(2)); + EXPECT_EQ(fcu3, channel_bag->fcurve(3)); + EXPECT_EQ(fcu4, channel_bag->fcurve(4)); + EXPECT_EQ(group0, fcu0->grp); + EXPECT_EQ(group0, fcu1->grp); + EXPECT_EQ(group2, fcu2->grp); + EXPECT_EQ(group2, fcu3->grp); + EXPECT_EQ(nullptr, fcu4->grp); + + /* Removing an empty group shouldn't affect the fcurves at all. */ + EXPECT_EQ(true, channel_bag->channel_group_remove(*group1)); + ASSERT_EQ(2, channel_bag->channel_groups().size()); + ASSERT_EQ(5, channel_bag->fcurves().size()); + EXPECT_EQ(group0, channel_bag->channel_group(0)); + EXPECT_EQ(group2, channel_bag->channel_group(1)); + EXPECT_EQ(fcu0, channel_bag->fcurve(0)); + EXPECT_EQ(fcu1, channel_bag->fcurve(1)); + EXPECT_EQ(fcu2, channel_bag->fcurve(2)); + EXPECT_EQ(fcu3, channel_bag->fcurve(3)); + EXPECT_EQ(fcu4, channel_bag->fcurve(4)); + EXPECT_EQ(group0, fcu0->grp); + EXPECT_EQ(group0, fcu1->grp); + EXPECT_EQ(group2, fcu2->grp); + EXPECT_EQ(group2, fcu3->grp); + EXPECT_EQ(nullptr, fcu4->grp); + + /* Removing a group that's not at the end of the group array should move its + * fcurves to be just after the grouped fcurves. */ + EXPECT_EQ(true, channel_bag->channel_group_remove(*group0)); + ASSERT_EQ(1, channel_bag->channel_groups().size()); + ASSERT_EQ(5, channel_bag->fcurves().size()); + EXPECT_EQ(group2, channel_bag->channel_group(0)); + EXPECT_EQ(fcu2, channel_bag->fcurve(0)); + EXPECT_EQ(fcu3, channel_bag->fcurve(1)); + EXPECT_EQ(fcu0, channel_bag->fcurve(2)); + EXPECT_EQ(fcu1, channel_bag->fcurve(3)); + EXPECT_EQ(fcu4, channel_bag->fcurve(4)); + EXPECT_EQ(nullptr, fcu0->grp); + EXPECT_EQ(nullptr, fcu1->grp); + EXPECT_EQ(group2, fcu2->grp); + EXPECT_EQ(group2, fcu3->grp); + EXPECT_EQ(nullptr, fcu4->grp); + + /* Removing a group at the end of the group array shouldn't move its + * fcurves. */ + EXPECT_EQ(true, channel_bag->channel_group_remove(*group2)); + ASSERT_EQ(0, channel_bag->channel_groups().size()); + ASSERT_EQ(5, channel_bag->fcurves().size()); + EXPECT_EQ(fcu2, channel_bag->fcurve(0)); + EXPECT_EQ(fcu3, channel_bag->fcurve(1)); + EXPECT_EQ(fcu0, channel_bag->fcurve(2)); + EXPECT_EQ(fcu1, channel_bag->fcurve(3)); + EXPECT_EQ(fcu4, channel_bag->fcurve(4)); + EXPECT_EQ(nullptr, fcu0->grp); + EXPECT_EQ(nullptr, fcu1->grp); + EXPECT_EQ(nullptr, fcu2->grp); + EXPECT_EQ(nullptr, fcu3->grp); + EXPECT_EQ(nullptr, fcu4->grp); +} + +TEST_F(ChannelBagTest, channel_group_find) +{ + bActionGroup *group0a = &channel_bag->channel_group_create("Foo"); + bActionGroup *group1a = &channel_bag->channel_group_create("Bar"); + bActionGroup *group2a = &channel_bag->channel_group_create("Yar"); + + bActionGroup *group0b = channel_bag->channel_group_find("Foo"); + bActionGroup *group1b = channel_bag->channel_group_find("Bar"); + bActionGroup *group2b = channel_bag->channel_group_find("Yar"); + + EXPECT_EQ(group0a, group0b); + EXPECT_EQ(group1a, group1b); + EXPECT_EQ(group2a, group2b); + + EXPECT_EQ(nullptr, channel_bag->channel_group_find("Wat")); +} + +TEST_F(ChannelBagTest, channel_group_ensure) +{ + bActionGroup *group0 = &channel_bag->channel_group_create("Foo"); + bActionGroup *group1 = &channel_bag->channel_group_create("Bar"); + EXPECT_EQ(channel_bag->channel_groups().size(), 2); + + EXPECT_EQ(group0, &channel_bag->channel_group_ensure("Foo")); + EXPECT_EQ(channel_bag->channel_groups().size(), 2); + + EXPECT_EQ(group1, &channel_bag->channel_group_ensure("Bar")); + EXPECT_EQ(channel_bag->channel_groups().size(), 2); + + bActionGroup *group2 = &channel_bag->channel_group_ensure("Yar"); + ASSERT_EQ(channel_bag->channel_groups().size(), 3); + EXPECT_EQ(group2, channel_bag->channel_group(2)); +} + +TEST_F(ChannelBagTest, channel_group_fcurve_creation) +{ + FCurve *fcu0 = &channel_bag->fcurve_ensure(nullptr, {"fcu0", 0, std::nullopt, std::nullopt}); + EXPECT_EQ(1, channel_bag->fcurves().size()); + EXPECT_TRUE(channel_bag->channel_groups().is_empty()); + + /* If an fcurve already exists, then ensuring it with a channel group in the + * fcurve descriptor should NOT add it that group, nor should the group be + * created if it doesn't already exist. */ + channel_bag->fcurve_ensure(nullptr, {"fcu0", 0, std::nullopt, "group0"}); + EXPECT_EQ(1, channel_bag->fcurves().size()); + EXPECT_EQ(nullptr, fcu0->grp); + EXPECT_TRUE(channel_bag->channel_groups().is_empty()); + + /* Creating a new fcurve with a channel group in the fcurve descriptor should + * create the group and put the fcurve in it. This also implies that the + * fcurve will be added before any non-grouped fcurves in the array. */ + FCurve *fcu1 = &channel_bag->fcurve_ensure(nullptr, {"fcu1", 0, std::nullopt, "group0"}); + ASSERT_EQ(2, channel_bag->fcurves().size()); + ASSERT_EQ(1, channel_bag->channel_groups().size()); + bActionGroup *group0 = channel_bag->channel_group(0); + EXPECT_EQ(fcu1, channel_bag->fcurve(0)); + EXPECT_EQ(fcu0, channel_bag->fcurve(1)); + EXPECT_EQ(group0, fcu1->grp); + EXPECT_EQ(nullptr, fcu0->grp); + EXPECT_EQ(0, group0->fcurve_range_start); + EXPECT_EQ(1, group0->fcurve_range_length); + + /* Creating a new fcurve with a second channel group in the fcurve descriptor + * should create the group and put the fcurve in it. This also implies that + * the fcurve will be added before non-grouped fcurves, but after other + * grouped ones. */ + FCurve *fcu2 = &channel_bag->fcurve_ensure(nullptr, {"fcu2", 0, std::nullopt, "group1"}); + ASSERT_EQ(3, channel_bag->fcurves().size()); + ASSERT_EQ(2, channel_bag->channel_groups().size()); + EXPECT_EQ(group0, channel_bag->channel_group(0)); + bActionGroup *group1 = channel_bag->channel_group(1); + EXPECT_EQ(fcu1, channel_bag->fcurve(0)); + EXPECT_EQ(fcu2, channel_bag->fcurve(1)); + EXPECT_EQ(fcu0, channel_bag->fcurve(2)); + EXPECT_EQ(group0, fcu1->grp); + EXPECT_EQ(group1, fcu2->grp); + EXPECT_EQ(nullptr, fcu0->grp); + EXPECT_EQ(0, group0->fcurve_range_start); + EXPECT_EQ(1, group0->fcurve_range_length); + EXPECT_EQ(1, group1->fcurve_range_start); + EXPECT_EQ(1, group1->fcurve_range_length); + + /* Creating a new fcurve with the first channel group again should put it at + * the end of that group. */ + FCurve *fcu3 = &channel_bag->fcurve_ensure(nullptr, {"fcu3", 0, std::nullopt, "group0"}); + ASSERT_EQ(4, channel_bag->fcurves().size()); + ASSERT_EQ(2, channel_bag->channel_groups().size()); + EXPECT_EQ(group0, channel_bag->channel_group(0)); + EXPECT_EQ(group1, channel_bag->channel_group(1)); + EXPECT_EQ(fcu1, channel_bag->fcurve(0)); + EXPECT_EQ(fcu3, channel_bag->fcurve(1)); + EXPECT_EQ(fcu2, channel_bag->fcurve(2)); + EXPECT_EQ(fcu0, channel_bag->fcurve(3)); + EXPECT_EQ(group0, fcu1->grp); + EXPECT_EQ(group0, fcu3->grp); + EXPECT_EQ(group1, fcu2->grp); + EXPECT_EQ(nullptr, fcu0->grp); + EXPECT_EQ(0, group0->fcurve_range_start); + EXPECT_EQ(2, group0->fcurve_range_length); + EXPECT_EQ(2, group1->fcurve_range_start); + EXPECT_EQ(1, group1->fcurve_range_length); + + /* Finally, creating a new fcurve with the second channel group again should + * also put it at the end of that group. */ + FCurve *fcu4 = &channel_bag->fcurve_ensure(nullptr, {"fcu4", 0, std::nullopt, "group1"}); + ASSERT_EQ(5, channel_bag->fcurves().size()); + ASSERT_EQ(2, channel_bag->channel_groups().size()); + EXPECT_EQ(group0, channel_bag->channel_group(0)); + EXPECT_EQ(group1, channel_bag->channel_group(1)); + EXPECT_EQ(fcu1, channel_bag->fcurve(0)); + EXPECT_EQ(fcu3, channel_bag->fcurve(1)); + EXPECT_EQ(fcu2, channel_bag->fcurve(2)); + EXPECT_EQ(fcu4, channel_bag->fcurve(3)); + EXPECT_EQ(fcu0, channel_bag->fcurve(4)); + EXPECT_EQ(0, group0->fcurve_range_start); + EXPECT_EQ(2, group0->fcurve_range_length); + EXPECT_EQ(2, group1->fcurve_range_start); + EXPECT_EQ(2, group1->fcurve_range_length); +} + +TEST_F(ChannelBagTest, channel_group_fcurve_removal) +{ + FCurve *fcu0 = &channel_bag->fcurve_ensure(nullptr, {"fcu0", 0, std::nullopt, "group0"}); + FCurve *fcu1 = &channel_bag->fcurve_ensure(nullptr, {"fcu1", 0, std::nullopt, "group0"}); + FCurve *fcu2 = &channel_bag->fcurve_ensure(nullptr, {"fcu2", 0, std::nullopt, "group1"}); + FCurve *fcu3 = &channel_bag->fcurve_ensure(nullptr, {"fcu3", 0, std::nullopt, "group1"}); + FCurve *fcu4 = &channel_bag->fcurve_ensure(nullptr, {"fcu4", 0, std::nullopt, std::nullopt}); + + ASSERT_EQ(5, channel_bag->fcurves().size()); + ASSERT_EQ(2, channel_bag->channel_groups().size()); + EXPECT_EQ(0, channel_bag->channel_group(0)->fcurve_range_start); + EXPECT_EQ(2, channel_bag->channel_group(0)->fcurve_range_length); + EXPECT_EQ(2, channel_bag->channel_group(1)->fcurve_range_start); + EXPECT_EQ(2, channel_bag->channel_group(1)->fcurve_range_length); + + channel_bag->fcurve_remove(*fcu3); + ASSERT_EQ(4, channel_bag->fcurves().size()); + ASSERT_EQ(2, channel_bag->channel_groups().size()); + EXPECT_EQ(0, channel_bag->channel_group(0)->fcurve_range_start); + EXPECT_EQ(2, channel_bag->channel_group(0)->fcurve_range_length); + EXPECT_EQ(2, channel_bag->channel_group(1)->fcurve_range_start); + EXPECT_EQ(1, channel_bag->channel_group(1)->fcurve_range_length); + + channel_bag->fcurve_remove(*fcu0); + ASSERT_EQ(3, channel_bag->fcurves().size()); + ASSERT_EQ(2, channel_bag->channel_groups().size()); + EXPECT_EQ(0, channel_bag->channel_group(0)->fcurve_range_start); + EXPECT_EQ(1, channel_bag->channel_group(0)->fcurve_range_length); + EXPECT_EQ(1, channel_bag->channel_group(1)->fcurve_range_start); + EXPECT_EQ(1, channel_bag->channel_group(1)->fcurve_range_length); + + bActionGroup *group1 = channel_bag->channel_group(1); + + channel_bag->fcurve_remove(*fcu1); + ASSERT_EQ(2, channel_bag->fcurves().size()); + ASSERT_EQ(1, channel_bag->channel_groups().size()); + EXPECT_EQ(group1, channel_bag->channel_group(0)); + EXPECT_EQ(0, channel_bag->channel_group(0)->fcurve_range_start); + EXPECT_EQ(1, channel_bag->channel_group(0)->fcurve_range_length); + + channel_bag->fcurve_remove(*fcu4); + ASSERT_EQ(1, channel_bag->fcurves().size()); + ASSERT_EQ(1, channel_bag->channel_groups().size()); + EXPECT_EQ(0, channel_bag->channel_group(0)->fcurve_range_start); + EXPECT_EQ(1, channel_bag->channel_group(0)->fcurve_range_length); + + channel_bag->fcurve_remove(*fcu2); + ASSERT_EQ(0, channel_bag->fcurves().size()); + ASSERT_EQ(0, channel_bag->channel_groups().size()); +} + } // namespace blender::animrig::tests diff --git a/source/blender/animrig/intern/keyframing.cc b/source/blender/animrig/intern/keyframing.cc index e2dfa17eb82..e7b8d6ac725 100644 --- a/source/blender/animrig/intern/keyframing.cc +++ b/source/blender/animrig/intern/keyframing.cc @@ -835,25 +835,28 @@ struct KeyInsertData { int array_index; }; -static SingleKeyingResult insert_key_layer(Main *bmain, - Layer &layer, - const Slot &slot, - const std::string &rna_path, - const std::optional prop_subtype, - const KeyInsertData &key_data, - const KeyframeSettings &key_settings, - const eInsertKeyFlags insert_key_flags) +static SingleKeyingResult insert_key_layer( + Main *bmain, + Layer &layer, + const Slot &slot, + const std::string &rna_path, + const std::optional prop_subtype, + const std::optional channel_group, + const KeyInsertData &key_data, + const KeyframeSettings &key_settings, + const eInsertKeyFlags insert_key_flags) { assert_baklava_phase_1_invariants(layer); BLI_assert(layer.strips().size() == 1); Strip *strip = layer.strip(0); - return strip->as().keyframe_insert(bmain, - slot, - {rna_path, key_data.array_index, prop_subtype}, - key_data.position, - key_settings, - insert_key_flags); + return strip->as().keyframe_insert( + bmain, + slot, + {rna_path, key_data.array_index, prop_subtype, channel_group}, + key_data.position, + key_settings, + insert_key_flags); } static CombinedKeyingResult insert_key_layered_action(Main *bmain, @@ -903,6 +906,10 @@ static CombinedKeyingResult insert_key_layered_action(Main *bmain, const std::optional rna_path_id_to_prop = RNA_path_from_ID_to_property(&ptr, prop); BLI_assert(rna_path_id_to_prop.has_value()); + + const std::optional channel_group = default_channel_group_for_path( + &ptr, *rna_path_id_to_prop); + const PropertySubType prop_subtype = RNA_property_subtype(prop); Vector rna_values = get_keyframe_values(&ptr, prop, use_visual_keyframing); @@ -919,6 +926,7 @@ static CombinedKeyingResult insert_key_layered_action(Main *bmain, slot, *rna_path_id_to_prop, prop_subtype, + channel_group, key_data, key_settings, insert_key_flags); diff --git a/source/blender/blenkernel/intern/action.cc b/source/blender/blenkernel/intern/action.cc index e1a7d644dc0..b8b6e88fac0 100644 --- a/source/blender/blenkernel/intern/action.cc +++ b/source/blender/blenkernel/intern/action.cc @@ -294,9 +294,14 @@ static void write_channelbag(BlendWriter *writer, animrig::ChannelBag &channelba { BLO_write_struct(writer, ActionChannelBag, &channelbag); + Span groups = channelbag.channel_groups(); + BLO_write_pointer_array(writer, groups.size(), groups.data()); + for (bActionGroup *group : groups) { + BLO_write_struct(writer, bActionGroup, group); + } + Span fcurves = channelbag.fcurves(); BLO_write_pointer_array(writer, fcurves.size(), fcurves.data()); - for (FCurve *fcurve : fcurves) { BLO_write_struct(writer, FCurve, fcurve); BKE_fcurve_blend_write_data(writer, fcurve); @@ -352,6 +357,67 @@ static void write_slots(BlendWriter *writer, Span slots) } } +/** + * Create a listbase from a Span of channel groups. + * + * \note this does NOT transfer ownership of the pointers. The ListBase should + * not be freed, but given to + * `action_blend_write_clear_legacy_channel_groups_listbase()` below. + * + * \warning This code is modifying actual '`Main`' data in-place, which is + * usually not acceptable (due to risks of unsafe concurrent accesses mainly). + * The reasons why this is currently seen as 'reasonably safe' are: + * - Current blender code is _not_ expected to access the affected bActionGroup data + * (`prev`/`next` listbase pointers) in any way, as they are stored in an array. + * - The `action.groups` listbase modification is safe/valid, as this is a member of + * the Action ID, which is a shallow copy of the actual ID data from Main. + */ +static void action_blend_write_make_legacy_channel_groups_listbase( + ListBase &listbase, const Span channel_groups) +{ + if (channel_groups.is_empty()) { + BLI_listbase_clear(&listbase); + return; + } + + /* Set the fcurve listbase pointers. + * + * Note that the fcurves' own prev/next pointers are hooked up by + * `action_blend_write_make_legacy_fcurves_listbase()`, so that they function + * properly as a list. */ + for (bActionGroup *group : channel_groups) { + if (group->fcurve_range_length == 0) { + group->channels = {nullptr, nullptr}; + continue; + } + Span fcurves = group->channel_bag->wrap().fcurves(); + group->channels = { + fcurves[group->fcurve_range_start], + fcurves[group->fcurve_range_start + group->fcurve_range_length - 1], + }; + } + + /* Determine the prev/next pointers on the elements. */ + const int last_index = channel_groups.size() - 1; + for (int index : channel_groups.index_range()) { + channel_groups[index]->prev = (index > 0) ? channel_groups[index - 1] : nullptr; + channel_groups[index]->next = (index < last_index) ? channel_groups[index + 1] : nullptr; + } + + listbase.first = channel_groups[0]; + listbase.last = channel_groups[last_index]; +} + +static void action_blend_write_clear_legacy_channel_groups_listbase(ListBase &listbase) +{ + LISTBASE_FOREACH (bActionGroup *, group, &listbase) { + group->prev = nullptr; + group->next = nullptr; + } + + BLI_listbase_clear(&listbase); +} + /** * Create a listbase from a Span of F-Curves. * @@ -413,8 +479,16 @@ static void action_blend_write(BlendWriter *writer, ID *id, const void *id_addre "Layered Action should not have legacy data"); const animrig::Slot &first_slot = *action.slot(0); + + /* Note: channel group forward-compat data requires that fcurve + * forward-compat legacy data is also written, and vice-versa. Both have + * pointers to each other that won't resolve properly when loaded in older + * Blender versions if only one is written. */ Span fcurves = fcurves_for_action_slot(action, first_slot.handle); action_blend_write_make_legacy_fcurves_listbase(action.curves, fcurves); + Span channel_groups = channel_groups_for_action_slot(action, + first_slot.handle); + action_blend_write_make_legacy_channel_groups_listbase(action.groups, channel_groups); } #else /* Built without Baklava, so ensure that the written data is clean. This should not change @@ -455,6 +529,7 @@ static void action_blend_write(BlendWriter *writer, ID *id, const void *id_addre * blend-file by generating two `BHead` `DATA` blocks with the same old * address for the same ID. */ + action_blend_write_clear_legacy_channel_groups_listbase(action.groups); action_blend_write_clear_legacy_fcurves_listbase(action.curves); } #endif /* WITH_ANIM_BAKLAVA */ @@ -473,11 +548,25 @@ static void action_blend_write(BlendWriter *writer, ID *id, const void *id_addre } #ifdef WITH_ANIM_BAKLAVA + +static void read_channel_group(BlendDataReader *reader, bActionGroup &channel_group) +{ + /* Remap non-owning pointer. */ + channel_group.channel_bag = static_cast(BLO_read_get_new_data_address_no_us( + reader, channel_group.channel_bag, sizeof(ActionChannelBag))); +} + static void read_channelbag(BlendDataReader *reader, animrig::ChannelBag &channelbag) { BLO_read_pointer_array( - reader, channelbag.fcurve_array_num, reinterpret_cast(&channelbag.fcurve_array)); + reader, channelbag.group_array_num, reinterpret_cast(&channelbag.group_array)); + for (int i = 0; i < channelbag.group_array_num; i++) { + BLO_read_struct(reader, bActionGroup, &channelbag.group_array[i]); + read_channel_group(reader, *channelbag.group_array[i]); + } + BLO_read_pointer_array( + reader, channelbag.fcurve_array_num, reinterpret_cast(&channelbag.fcurve_array)); for (int i = 0; i < channelbag.fcurve_array_num; i++) { BLO_read_struct(reader, FCurve, &channelbag.fcurve_array[i]); FCurve *fcurve = channelbag.fcurve_array[i]; @@ -560,7 +649,7 @@ static void action_blend_read_data(BlendDataReader *reader, ID *id) if (action.is_action_layered()) { /* Clear the forward-compatible storage (see action_blend_write_data()). */ BLI_listbase_clear(&action.curves); - BLI_assert(BLI_listbase_is_empty(&action.groups)); + BLI_listbase_clear(&action.groups); } else { /* Read legacy data. */ diff --git a/source/blender/editors/animation/anim_channels_edit.cc b/source/blender/editors/animation/anim_channels_edit.cc index 7fe0c95ca32..833ba2cf211 100644 --- a/source/blender/editors/animation/anim_channels_edit.cc +++ b/source/blender/editors/animation/anim_channels_edit.cc @@ -1679,6 +1679,13 @@ static void rearrange_action_channels(bAnimContext *ac, bAction *act, eRearrange ListBase anim_data_visible = {nullptr, nullptr}; bool do_channels; + BLI_assert(act != nullptr); + + /* TODO: layered actions not yet supported. */ + if (!act->wrap().is_action_legacy()) { + return; + } + /* get rearranging function */ AnimChanRearrangeFp rearrange_func = rearrange_get_mode_func(mode); @@ -2036,42 +2043,49 @@ static void animchannels_group_channels(bAnimContext *ac, AnimData *adt = adt_ref->adt; bAction *act = adt->action; - if (act) { - ListBase anim_data = {nullptr, nullptr}; - int filter; - - /* find selected F-Curves to re-group */ - filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE | ANIMFILTER_SEL | - ANIMFILTER_FCURVESONLY); - ANIM_animdata_filter(ac, &anim_data, eAnimFilter_Flags(filter), adt_ref, ANIMCONT_CHANNEL); - - if (anim_data.first) { - bActionGroup *agrp; - - /* create new group, which should now be part of the action */ - agrp = action_groups_add_new(act, name); - BLI_assert(agrp != nullptr); - - /* Transfer selected F-Curves across to new group. */ - LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { - FCurve *fcu = (FCurve *)ale->data; - bActionGroup *grp = fcu->grp; - - /* remove F-Curve from group, then group too if it is now empty */ - action_groups_remove_channel(act, fcu); - - if ((grp) && BLI_listbase_is_empty(&grp->channels)) { - BLI_freelinkN(&act->groups, grp); - } - - /* add F-Curve to group */ - action_groups_add_channel(act, agrp, fcu); - } - } - - /* cleanup */ - ANIM_animdata_freelist(&anim_data); + if (act == nullptr) { + return; } + + /* TODO: layered actions not yet supported. */ + if (!act->wrap().is_action_legacy()) { + return; + } + + ListBase anim_data = {nullptr, nullptr}; + int filter; + + /* find selected F-Curves to re-group */ + filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE | ANIMFILTER_SEL | + ANIMFILTER_FCURVESONLY); + ANIM_animdata_filter(ac, &anim_data, eAnimFilter_Flags(filter), adt_ref, ANIMCONT_CHANNEL); + + if (anim_data.first) { + bActionGroup *agrp; + + /* create new group, which should now be part of the action */ + agrp = action_groups_add_new(act, name); + BLI_assert(agrp != nullptr); + + /* Transfer selected F-Curves across to new group. */ + LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { + FCurve *fcu = (FCurve *)ale->data; + bActionGroup *grp = fcu->grp; + + /* remove F-Curve from group, then group too if it is now empty */ + action_groups_remove_channel(act, fcu); + + if ((grp) && BLI_listbase_is_empty(&grp->channels)) { + BLI_freelinkN(&act->groups, grp); + } + + /* add F-Curve to group */ + action_groups_add_channel(act, agrp, fcu); + } + } + + /* cleanup */ + ANIM_animdata_freelist(&anim_data); } static int animchannels_group_exec(bContext *C, wmOperator *op) @@ -2164,22 +2178,29 @@ static int animchannels_ungroup_exec(bContext *C, wmOperator * /*op*/) LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { /* find action for this F-Curve... */ - if (ale->adt && ale->adt->action) { - FCurve *fcu = (FCurve *)ale->data; - bAction *act = ale->adt->action; + if (!ale->adt || !ale->adt->action) { + continue; + } - /* only proceed to remove if F-Curve is in a group... */ - if (fcu->grp) { - bActionGroup *agrp = fcu->grp; + FCurve *fcu = (FCurve *)ale->data; + bAction *act = ale->adt->action; - /* remove F-Curve from group and add at tail (ungrouped) */ - action_groups_remove_channel(act, fcu); - BLI_addtail(&act->curves, fcu); + /* TODO: layered actions not yet supported. */ + if (!act->wrap().is_action_legacy()) { + continue; + } - /* delete group if it is now empty */ - if (BLI_listbase_is_empty(&agrp->channels)) { - BLI_freelinkN(&act->groups, agrp); - } + /* only proceed to remove if F-Curve is in a group... */ + if (fcu->grp) { + bActionGroup *agrp = fcu->grp; + + /* remove F-Curve from group and add at tail (ungrouped) */ + action_groups_remove_channel(act, fcu); + BLI_addtail(&act->curves, fcu); + + /* delete group if it is now empty */ + if (BLI_listbase_is_empty(&agrp->channels)) { + BLI_freelinkN(&act->groups, agrp); } } } diff --git a/source/blender/editors/animation/anim_filter.cc b/source/blender/editors/animation/anim_filter.cc index c87f589aa93..6fbf0ee91e9 100644 --- a/source/blender/editors/animation/anim_filter.cc +++ b/source/blender/editors/animation/anim_filter.cc @@ -543,8 +543,6 @@ bool ANIM_animdata_can_have_greasepencil(const eAnimCont_Types type) * whether any channels will be added (but without needing them to actually get created). * * \warning This causes the calling function to return early if we're only "peeking" for channels. - * - * XXX: ale_statement stuff is really a hack for one special case. It shouldn't really be needed. */ #define ANIMCHANNEL_NEW_CHANNEL_FULL( \ bmain, channel_data, channel_type, owner_id, fcurve_owner_id, ale_statement) \ @@ -1434,17 +1432,30 @@ static size_t animfilter_fcurves_span(bAnimContext *ac, return num_items; } +/** + * Filters a channel group and its children. + * + * This works both for channel groups in legacy and in layered actions. + * + * Note: `slot_handle` is only used for layered actions, and is ignored for + * legacy actions. + */ static size_t animfilter_act_group(bAnimContext *ac, ListBase *anim_data, bAction *act, + animrig::slot_handle_t slot_handle, bActionGroup *agrp, eAnimFilter_Flags filter_mode, ID *owner_id) { + BLI_assert(act != nullptr); + BLI_assert(agrp != nullptr); + ListBase tmp_data = {nullptr, nullptr}; size_t tmp_items = 0; size_t items = 0; - // int ofilter = filter_mode; + + animrig::Action &action = act->wrap(); /* if we care about the selection status of the channels, * but the group isn't expanded (1)... @@ -1487,17 +1498,28 @@ static size_t animfilter_act_group(bAnimContext *ac, if (!(filter_mode & ANIMFILTER_CURVE_VISIBLE) || !(agrp->flag & AGRP_NOTVISIBLE)) { /* group must be editable for its children to be editable (if we care about this) */ if (!(filter_mode & ANIMFILTER_FOREDIT) || EDITABLE_AGRP(agrp)) { - /* get first F-Curve which can be used here */ - FCurve *first_fcu = animfilter_fcurve_next(ac, - static_cast(agrp->channels.first), - ANIMTYPE_FCURVE, - filter_mode, - agrp, - owner_id); + /* Filter the fcurves in this group, adding them to the temporary + * filter list. */ + if (action.is_action_legacy()) { + /* get first F-Curve which can be used here */ + FCurve *first_fcu = animfilter_fcurve_next(ac, + static_cast(agrp->channels.first), + ANIMTYPE_FCURVE, + filter_mode, + agrp, + owner_id); - /* filter list, starting from this F-Curve */ - tmp_items += animfilter_fcurves( - ac, &tmp_data, first_fcu, ANIMTYPE_FCURVE, filter_mode, agrp, owner_id, &act->id); + /* filter list, starting from this F-Curve */ + tmp_items += animfilter_fcurves( + ac, &tmp_data, first_fcu, ANIMTYPE_FCURVE, filter_mode, agrp, owner_id, &act->id); + } + else { + BLI_assert(agrp->channel_bag != nullptr); + Span fcurves = agrp->channel_bag->wrap().fcurves().slice( + agrp->fcurve_range_start, agrp->fcurve_range_length); + tmp_items += animfilter_fcurves_span( + ac, &tmp_data, fcurves, slot_handle, filter_mode, owner_id, &act->id); + } } } } @@ -1508,13 +1530,17 @@ static size_t animfilter_act_group(bAnimContext *ac, if (tmp_items) { /* add this group as a channel first */ if (filter_mode & ANIMFILTER_LIST_CHANNELS) { - /* restore original filter mode so that this next step works ok... */ - // filter_mode = ofilter; - /* filter selection of channel specially here again, * since may be open and not subject to previous test */ if (ANIMCHANNEL_SELOK(SEL_AGRP(agrp))) { - ANIMCHANNEL_NEW_CHANNEL(ac->bmain, agrp, ANIMTYPE_GROUP, owner_id, &act->id); + if (action.is_action_legacy()) { + ANIMCHANNEL_NEW_CHANNEL(ac->bmain, agrp, ANIMTYPE_GROUP, owner_id, &act->id); + } + else { + ANIMCHANNEL_NEW_CHANNEL_FULL(ac->bmain, agrp, ANIMTYPE_GROUP, owner_id, &act->id, { + ale->slot_handle = slot_handle; + }); + } } } @@ -1572,10 +1598,30 @@ static size_t animfilter_action_slot(bAnimContext *ac, const bool expansion_is_ok = !visible_only || !show_slot_channel || slot.is_expanded(); if (show_fcurves_only || expansion_is_ok) { - /* Add list elements for the F-Curves for this Slot. */ - Span fcurves = animrig::fcurves_for_action_slot(action, slot.handle); - items += animfilter_fcurves_span( - ac, anim_data, fcurves, slot.handle, filter_mode, animated_id, &action.id); + animrig::ChannelBag *channel_bag = animrig::channelbag_for_action_slot(action, slot.handle); + if (channel_bag == nullptr) { + return items; + } + + /* Add channel groups and their member channels. */ + for (bActionGroup *group : channel_bag->channel_groups()) { + items += animfilter_act_group( + ac, anim_data, &action, slot.handle, group, filter_mode, animated_id); + } + + /* Add ungrouped channels. */ + if (!(filter_mode & ANIMFILTER_ACTGROUPED)) { + int first_ungrouped_fcurve_index = 0; + if (!channel_bag->channel_groups().is_empty()) { + const bActionGroup *last_group = channel_bag->channel_groups().last(); + first_ungrouped_fcurve_index = last_group->fcurve_range_start + + last_group->fcurve_range_length; + } + + Span fcurves = channel_bag->fcurves().drop_front(first_ungrouped_fcurve_index); + items += animfilter_fcurves_span( + ac, anim_data, fcurves, slot.handle, filter_mode, animated_id, &action.id); + } } return items; @@ -1650,7 +1696,8 @@ static size_t animfilter_action(bAnimContext *ac, lastchan = static_cast(agrp->channels.last); } - items += animfilter_act_group(ac, anim_data, &action, agrp, filter_mode, owner_id); + items += animfilter_act_group( + ac, anim_data, &action, animrig::Slot::unassigned, agrp, filter_mode, owner_id); } /* Un-grouped F-Curves (only if we're not only considering those channels in diff --git a/source/blender/editors/animation/keyframes_edit.cc b/source/blender/editors/animation/keyframes_edit.cc index 491b4855d83..1a6a961df72 100644 --- a/source/blender/editors/animation/keyframes_edit.cc +++ b/source/blender/editors/animation/keyframes_edit.cc @@ -154,13 +154,29 @@ static short agrp_keyframes_loop(KeyframeEditData *ked, return 0; } - /* only iterate over the F-Curves that are in this group */ - LISTBASE_FOREACH (FCurve *, fcu, &agrp->channels) { - if (fcu->grp == agrp) { - if (ANIM_fcurve_keyframes_loop(ked, fcu, key_ok, key_cb, fcu_cb)) { - return 1; + /* Legacy actions. */ + if (agrp->channels.first && agrp->channels.last) { + LISTBASE_FOREACH (FCurve *, fcu, &agrp->channels) { + if (fcu->grp == agrp) { + if (ANIM_fcurve_keyframes_loop(ked, fcu, key_ok, key_cb, fcu_cb)) { + return 1; + } } } + return 0; + } + + /* Layered actions. */ + if (agrp->channel_bag == nullptr) { + return 0; + } + animrig::ChannelBag channel_bag = agrp->channel_bag->wrap(); + Span fcurves = channel_bag.fcurves().slice(agrp->fcurve_range_start, + agrp->fcurve_range_length); + for (FCurve *fcurve : fcurves) { + if (ANIM_fcurve_keyframes_loop(ked, fcurve, key_ok, key_cb, fcu_cb)) { + return 1; + } } return 0; diff --git a/source/blender/editors/animation/keyframes_keylist.cc b/source/blender/editors/animation/keyframes_keylist.cc index cc6d945bceb..61afc2941d0 100644 --- a/source/blender/editors/animation/keyframes_keylist.cc +++ b/source/blender/editors/animation/keyframes_keylist.cc @@ -1200,11 +1200,26 @@ void action_group_to_keylist(AnimData *adt, return; } - LISTBASE_FOREACH (FCurve *, fcu, &agrp->channels) { - if (fcu->grp != agrp) { - break; + /* Legacy actions. */ + if (agrp->channels.first && agrp->channels.last) { + LISTBASE_FOREACH (FCurve *, fcu, &agrp->channels) { + if (fcu->grp != agrp) { + break; + } + fcurve_to_keylist(adt, fcu, keylist, saction_flag, range); } - fcurve_to_keylist(adt, fcu, keylist, saction_flag, range); + return; + } + + /* Layered actions. */ + if (agrp->channel_bag == nullptr) { + return; + } + animrig::ChannelBag channel_bag = agrp->channel_bag->wrap(); + Span fcurves = channel_bag.fcurves().slice(agrp->fcurve_range_start, + agrp->fcurve_range_length); + for (FCurve *fcurve : fcurves) { + fcurve_to_keylist(adt, fcurve, keylist, saction_flag, range); } } diff --git a/source/blender/editors/include/ED_keyframes_draw.hh b/source/blender/editors/include/ED_keyframes_draw.hh index 21ff14060f7..30c807c7a4b 100644 --- a/source/blender/editors/include/ED_keyframes_draw.hh +++ b/source/blender/editors/include/ED_keyframes_draw.hh @@ -152,5 +152,7 @@ void ED_add_mask_layer_channel(ChannelDrawList *draw_list, int saction_flag); ChannelDrawList *ED_channel_draw_list_create(); + void ED_channel_list_flush(ChannelDrawList *draw_list, View2D *v2d); + void ED_channel_list_free(ChannelDrawList *draw_list); diff --git a/source/blender/makesdna/DNA_action_types.h b/source/blender/makesdna/DNA_action_types.h index 8768d1dae71..574d4d47ab5 100644 --- a/source/blender/makesdna/DNA_action_types.h +++ b/source/blender/makesdna/DNA_action_types.h @@ -677,11 +677,30 @@ typedef struct bActionGroup { struct bActionGroup *next, *prev; /** + * List of channels in this group for legacy actions. + * * NOTE: this must not be touched by standard listbase functions * which would clear links to other channels. */ ListBase channels; + /** + * Span of channels in this group for layered actions. + * + * This specifies that span as a range of items in a ChannelBag's fcurve + * array. + */ + int fcurve_range_start; + int fcurve_range_length; + + /** + * For layered actions: the ChannelBag this group belongs to. + * + * This is needed in the keyframe drawing code, etc., to give direct access to + * the fcurves in this group. + */ + struct ActionChannelBag *channel_bag; + /** Settings for this action-group. */ int flag; /** @@ -1217,6 +1236,24 @@ typedef struct KeyframeActionStrip { typedef struct ActionChannelBag { int32_t slot_handle; + /* Channel groups. These index into the `fcurve_array` below to specify group + * membership of the fcurves. + * + * Note that although the fcurves also have pointers back to the groups they + * belong to, those pointers are not the source of truth. The source of truth + * for membership is the information in the channel groups here. + * + * Invariants: + * 1. The groups are stored in this array in the same order as their indices + * into the fcurve array. + * 2. The grouped fcurves are tightly packed, starting at the first fcurve and + * having no gaps of ungrouped fcurves between them. Ungrouped fcurves come + * at the end, after all of the grouped fcurves. */ + int group_array_num; + struct bActionGroup **group_array; + + uint8_t _pad[4]; + int fcurve_array_num; struct FCurve **fcurve_array; /* Array of 'fcurve_array_num' FCurves. */ diff --git a/source/blender/makesrna/intern/rna_action.cc b/source/blender/makesrna/intern/rna_action.cc index 371fb79cd7a..8a9df2cd72c 100644 --- a/source/blender/makesrna/intern/rna_action.cc +++ b/source/blender/makesrna/intern/rna_action.cc @@ -631,6 +631,46 @@ static void rna_ChannelBag_fcurve_clear(ID *dna_action_id, WM_event_add_notifier(C, NC_ANIMATION | ND_KEYFRAME | NA_EDITED, nullptr); } +static void rna_iterator_ChannelBag_groups_begin(CollectionPropertyIterator *iter, PointerRNA *ptr) +{ + animrig::ChannelBag &bag = rna_data_channelbag(ptr); + rna_iterator_array_begin(iter, bag.channel_groups()); +} + +static int rna_iterator_ChannelBag_groups_length(PointerRNA *ptr) +{ + animrig::ChannelBag &bag = rna_data_channelbag(ptr); + return bag.channel_groups().size(); +} + +static bActionGroup *rna_ChannelBag_group_new(ActionChannelBag *dna_channelbag, const char *name) +{ + BLI_assert(name != nullptr); + + animrig::ChannelBag &self = dna_channelbag->wrap(); + return &self.channel_group_create(name); +} + +static void rna_ChannelBag_group_remove(ActionChannelBag *dna_channelbag, + ReportList *reports, + PointerRNA *agrp_ptr) +{ + animrig::ChannelBag &self = dna_channelbag->wrap(); + + bActionGroup *agrp = static_cast(agrp_ptr->data); + + if (!self.channel_group_remove(*agrp)) { + BKE_report(reports, + RPT_ERROR, + "Could not remove the F-Curve Group from the collection because it doesn't exist " + "in the collection"); + return; + } + + RNA_POINTER_INVALIDATE(agrp_ptr); + WM_main_add_notifier(NC_ANIMATION | ND_KEYFRAME | NA_EDITED, nullptr); +} + static ActionChannelBag *rna_KeyframeActionStrip_channels(KeyframeActionStrip *self, const animrig::slot_handle_t slot_handle) { @@ -640,21 +680,115 @@ static ActionChannelBag *rna_KeyframeActionStrip_channels(KeyframeActionStrip *s # endif // WITH_ANIM_BAKLAVA +/** + * Iterator for the fcurves in a channel group. + * + * We need a custom iterator for this because legacy actions store their fcurves + * in a listbase, whereas layered actions store them in an array. Therefore + * this iterator needs to handle both kinds of iteration. + * + * In the future when legacy actions are fully deprecated this can be changed to + * a simple array iterator. + */ +struct ActionGroupChannelsIterator { + /* Which kind of iterator it is. */ + enum { + ARRAY, + LISTBASE, + } tag; + + union { + ArrayIterator array; + ListBaseIterator listbase; + }; +}; + +static void rna_ActionGroup_channels_begin(CollectionPropertyIterator *iter, PointerRNA *ptr) +{ + bActionGroup *group = (bActionGroup *)ptr->data; + + ActionGroupChannelsIterator *custom_iter = MEM_cnew(__func__); + + iter->internal.custom = custom_iter; + + /* Group from a layered action. */ + if (group->channel_bag != nullptr) { + MutableSpan fcurves = group->channel_bag->wrap().fcurves(); + + custom_iter->tag = ActionGroupChannelsIterator::ARRAY; + custom_iter->array.ptr = reinterpret_cast(fcurves.data() + group->fcurve_range_start); + custom_iter->array.endptr = reinterpret_cast( + fcurves.data() + group->fcurve_range_start + group->fcurve_range_length); + custom_iter->array.itemsize = sizeof(FCurve *); + custom_iter->array.length = group->fcurve_range_length; + + iter->valid = group->fcurve_range_length != 0; + + return; + } + + /* Group from a legacy action. */ + custom_iter->tag = ActionGroupChannelsIterator::LISTBASE; + custom_iter->listbase.link = static_cast(group->channels.first); + + iter->valid = custom_iter->listbase.link != nullptr; +} + +static void rna_ActionGroup_channels_end(CollectionPropertyIterator *iter) +{ + MEM_freeN(iter->internal.custom); +} + static void rna_ActionGroup_channels_next(CollectionPropertyIterator *iter) { - ListBaseIterator *internal = &iter->internal.listbase; - FCurve *fcu = (FCurve *)internal->link; - bActionGroup *grp = fcu->grp; + BLI_assert(iter->internal.custom != nullptr); + BLI_assert(iter->valid); - /* only continue if the next F-Curve (if existent) belongs in the same group */ - if ((fcu->next) && (fcu->next->grp == grp)) { - internal->link = (Link *)fcu->next; + ActionGroupChannelsIterator *custom_iter = static_cast( + iter->internal.custom); + + switch (custom_iter->tag) { + case ActionGroupChannelsIterator::ARRAY: { + custom_iter->array.ptr += custom_iter->array.itemsize; + iter->valid = (custom_iter->array.ptr != custom_iter->array.endptr); + break; + } + case ActionGroupChannelsIterator::LISTBASE: { + FCurve *fcurve = (FCurve *)custom_iter->listbase.link; + bActionGroup *grp = fcurve->grp; + /* Only continue if the next F-Curve (if existent) belongs in the same + * group. */ + if ((fcurve->next) && (fcurve->next->grp == grp)) { + custom_iter->listbase.link = custom_iter->listbase.link->next; + iter->valid = (custom_iter->listbase.link != nullptr); + } + else { + custom_iter->listbase.link = nullptr; + iter->valid = false; + } + break; + } } - else { - internal->link = nullptr; +} + +static PointerRNA rna_ActionGroup_channels_get(CollectionPropertyIterator *iter) +{ + BLI_assert(iter->internal.custom != nullptr); + BLI_assert(iter->valid); + ActionGroupChannelsIterator *custom_iter = static_cast( + iter->internal.custom); + + FCurve *fcurve; + switch (custom_iter->tag) { + case ActionGroupChannelsIterator::ARRAY: + fcurve = *reinterpret_cast(custom_iter->array.ptr); + break; + case ActionGroupChannelsIterator::LISTBASE: + fcurve = reinterpret_cast(custom_iter->listbase.link); + break; } - iter->valid = (internal->link != nullptr); + return rna_pointer_inherit_refine(&iter->parent, &RNA_FCurve, fcurve); } static bActionGroup *rna_Action_groups_new(bAction *act, ReportList *reports, const char name[]) @@ -1953,6 +2087,34 @@ static void rna_def_channelbag_fcurves(BlenderRNA *brna, PropertyRNA *cprop) RNA_def_function_ui_description(func, "Remove all F-Curves from this channelbag"); } +static void rna_def_channelbag_groups(BlenderRNA *brna, PropertyRNA *cprop) +{ + StructRNA *srna; + + FunctionRNA *func; + PropertyRNA *parm; + + RNA_def_property_srna(cprop, "ActionChannelBagGroups"); + srna = RNA_def_struct(brna, "ActionChannelBagGroups", nullptr); + RNA_def_struct_sdna(srna, "ActionChannelBag"); + RNA_def_struct_ui_text(srna, "F-Curve Groups", "Collection of f-curve groups"); + + func = RNA_def_function(srna, "new", "rna_ChannelBag_group_new"); + RNA_def_function_flag(func, FunctionFlag(0)); + RNA_def_function_ui_description(func, "Create a new action group and add it to the action"); + parm = RNA_def_string(func, "name", "Group", 0, "", "New name for the action group"); + RNA_def_parameter_flags(parm, PropertyFlag(0), PARM_REQUIRED); + parm = RNA_def_pointer(func, "action_group", "ActionGroup", "", "Newly created action group"); + RNA_def_function_return(func, parm); + + func = RNA_def_function(srna, "remove", "rna_ChannelBag_group_remove"); + RNA_def_function_ui_description(func, "Remove action group"); + RNA_def_function_flag(func, FUNC_USE_REPORTS); + parm = RNA_def_pointer(func, "action_group", "ActionGroup", "", "Action group to remove"); + RNA_def_parameter_flags(parm, PROP_NEVER_NULL, PARM_REQUIRED | PARM_RNAPTR); + RNA_def_parameter_clear_flags(parm, PROP_THICK_WRAP, ParameterFlag(0)); +} + static void rna_def_action_channelbag(BlenderRNA *brna) { StructRNA *srna; @@ -1968,6 +2130,7 @@ static void rna_def_action_channelbag(BlenderRNA *brna) prop = RNA_def_property(srna, "slot_handle", PROP_INT, PROP_NONE); RNA_def_property_clear_flag(prop, PROP_EDITABLE); + /* ChannelBag.fcurves */ prop = RNA_def_property(srna, "fcurves", PROP_COLLECTION, PROP_NONE); RNA_def_property_collection_funcs(prop, "rna_iterator_ChannelBag_fcurves_begin", @@ -1981,11 +2144,27 @@ static void rna_def_action_channelbag(BlenderRNA *brna) RNA_def_property_struct_type(prop, "FCurve"); RNA_def_property_ui_text(prop, "F-Curves", "The individual F-Curves that animate the slot"); rna_def_channelbag_fcurves(brna, prop); + + /* ChannelBag.groups */ + prop = RNA_def_property(srna, "groups", PROP_COLLECTION, PROP_NONE); + RNA_def_property_collection_funcs(prop, + "rna_iterator_ChannelBag_groups_begin", + "rna_iterator_array_next", + "rna_iterator_array_end", + "rna_iterator_array_dereference_get", + "rna_iterator_ChannelBag_groups_length", + nullptr, + nullptr, + nullptr); + RNA_def_property_struct_type(prop, "ActionGroup"); + RNA_def_property_ui_text( + prop, + "F-Curve Groups", + "Groupings of F-Curves for display purposes, in e.g. the dopesheet and graph editor"); + rna_def_channelbag_groups(brna, prop); } # endif // WITH_ANIM_BAKLAVA -/* =========================== Legacy Action interface =========================== */ - static void rna_def_action_group(BlenderRNA *brna) { StructRNA *srna; @@ -2014,10 +2193,10 @@ static void rna_def_action_group(BlenderRNA *brna) RNA_def_property_collection_sdna(prop, nullptr, "channels", nullptr); RNA_def_property_struct_type(prop, "FCurve"); RNA_def_property_collection_funcs(prop, - nullptr, + "rna_ActionGroup_channels_begin", "rna_ActionGroup_channels_next", - nullptr, - nullptr, + "rna_ActionGroup_channels_end", + "rna_ActionGroup_channels_get", nullptr, nullptr, nullptr, @@ -2062,6 +2241,8 @@ static void rna_def_action_group(BlenderRNA *brna) rna_def_actionbone_group_common(srna, NC_ANIMATION | ND_ANIMCHAN | NA_EDITED, nullptr); } +/* =========================== Legacy Action interface =========================== */ + /* fcurve.keyframe_points */ static void rna_def_action_groups(BlenderRNA *brna, PropertyRNA *cprop) { @@ -2080,7 +2261,6 @@ static void rna_def_action_groups(BlenderRNA *brna, PropertyRNA *cprop) RNA_def_function_ui_description(func, "Create a new action group and add it to the action"); parm = RNA_def_string(func, "name", "Group", 0, "", "New name for the action group"); RNA_def_parameter_flags(parm, PropertyFlag(0), PARM_REQUIRED); - parm = RNA_def_pointer(func, "action_group", "ActionGroup", "", "Newly created action group"); RNA_def_function_return(func, parm); diff --git a/source/blender/makesrna/intern/rna_fcurve.cc b/source/blender/makesrna/intern/rna_fcurve.cc index 8fc35d9aadb..e87173c3ad6 100644 --- a/source/blender/makesrna/intern/rna_fcurve.cc +++ b/source/blender/makesrna/intern/rna_fcurve.cc @@ -31,6 +31,7 @@ #include "ED_keyframing.hh" #ifdef RNA_RUNTIME +# include "ANIM_action.hh" # include "ANIM_fcurve.hh" #endif @@ -642,25 +643,49 @@ static void rna_FCurve_group_set(PointerRNA *ptr, PointerRNA value, ReportList * printf("ERROR: cannot assign F-Curve to group, since F-Curve is not attached to any ID\n"); return; } - /* make sure F-Curve exists in this action first, otherwise we could still have been tricked */ - else if (BLI_findindex(&act->curves, fcu) == -1) { - printf("ERROR: F-Curve (%p) doesn't exist in action '%s'\n", fcu, act->id.name); - return; - } - /* try to remove F-Curve from action (including from any existing groups) */ - action_groups_remove_channel(act, fcu); + blender::animrig::Action &action = act->wrap(); + if (!action.is_action_layered()) { + /* Legacy action. */ - /* add the F-Curve back to the action now in the right place */ - /* TODO: make the api function handle the case where there isn't any group to assign to. */ - if (value.data) { - /* add to its group using API function, which makes sure everything goes ok */ - action_groups_add_channel(act, static_cast(value.data), fcu); + /* make sure F-Curve exists in this action first, otherwise we could still have been tricked */ + if (BLI_findindex(&act->curves, fcu) == -1) { + printf("ERROR: F-Curve (%p) doesn't exist in action '%s'\n", fcu, act->id.name); + return; + } + + /* try to remove F-Curve from action (including from any existing groups) */ + action_groups_remove_channel(act, fcu); + + /* add the F-Curve back to the action now in the right place */ + /* TODO: make the api function handle the case where there isn't any group to assign to. */ + if (value.data) { + /* add to its group using API function, which makes sure everything goes ok */ + action_groups_add_channel(act, static_cast(value.data), fcu); + } + else { + /* Need to add this back, but it can only go at the end of the list + * (or else will corrupt groups). */ + BLI_addtail(&act->curves, fcu); + } } else { - /* Need to add this back, but it can only go at the end of the list - * (or else will corrupt groups). */ - BLI_addtail(&act->curves, fcu); + /* Layered action. */ + + bActionGroup *group = static_cast(value.data); + blender::animrig::ChannelBag *channel_bag; + { + ActionChannelBag *tmp = group->channel_bag; + BLI_assert(tmp != nullptr); + channel_bag = &tmp->wrap(); + } + + if (!channel_bag->fcurve_assign_to_channel_group(*fcu, *group)) { + printf("ERROR: F-Curve (%p) doesn't belong to the same channel bag as channel group '%s'\n", + fcu, + group->name); + return; + } } } diff --git a/tests/python/bl_animation_action.py b/tests/python/bl_animation_action.py index c6d537d4598..d65e018fc48 100644 --- a/tests/python/bl_animation_action.py +++ b/tests/python/bl_animation_action.py @@ -335,6 +335,68 @@ class ChannelBagsTest(unittest.TestCase): channelbag.fcurves.clear() self.assertEquals([], channelbag.fcurves[:]) + def test_channel_groups(self): + channelbag = self.strip.channelbags.new(self.slot) + + # Create some fcurves to play with. + fcurve0 = channelbag.fcurves.new('location', index=0) + fcurve1 = channelbag.fcurves.new('location', index=1) + fcurve2 = channelbag.fcurves.new('location', index=2) + fcurve3 = channelbag.fcurves.new('scale', index=0) + fcurve4 = channelbag.fcurves.new('scale', index=1) + fcurve5 = channelbag.fcurves.new('scale', index=2) + + self.assertEquals([], channelbag.groups[:]) + + # Create some channel groups. + group0 = channelbag.groups.new('group0') + group1 = channelbag.groups.new('group1') + self.assertEquals([group0, group1], channelbag.groups[:]) + self.assertEquals([], group0.channels[:]) + self.assertEquals([], group1.channels[:]) + + # Assign some fcurves to the channel groups. Intentionally not in order + # so we can test that the fcurves get moved around properly. + fcurve5.group = group1 + fcurve3.group = group1 + fcurve2.group = group0 + fcurve4.group = group0 + self.assertEquals([fcurve2, fcurve4], group0.channels[:]) + self.assertEquals([fcurve5, fcurve3], group1.channels[:]) + self.assertEquals([fcurve2, fcurve4, fcurve5, fcurve3, fcurve0, fcurve1], channelbag.fcurves[:]) + + # Weird case to be consistent with the legacy API: assigning None to an + # fcurve's group does *not* unassign it from its group. This is stupid, + # and we should change it at some point. But it's how the legacy API + # already works (presumably an oversight), so sticking to that for now. + fcurve3.group = None + self.assertEquals(group1, fcurve3.group) + self.assertEquals([fcurve2, fcurve4], group0.channels[:]) + self.assertEquals([fcurve5, fcurve3], group1.channels[:]) + self.assertEquals([fcurve2, fcurve4, fcurve5, fcurve3, fcurve0, fcurve1], channelbag.fcurves[:]) + + # Removing a group. + channelbag.groups.remove(group0) + self.assertEquals([group1], channelbag.groups[:]) + self.assertEquals([fcurve5, fcurve3], group1.channels[:]) + self.assertEquals([fcurve5, fcurve3, fcurve2, fcurve4, fcurve0, fcurve1], channelbag.fcurves[:]) + + # Attempting to remove a channel group that belongs to a different + # channel bag should fail. + other_slot = self.action.slots.new() + other_cbag = self.strip.channelbags.new(other_slot) + other_group = other_cbag.groups.new('group1') + with self.assertRaises(RuntimeError): + channelbag.groups.remove(other_group) + + # Another weird case that we reproduce from the legacy API: attempting + # to assign a group to an fcurve that doesn't belong to the same channel + # bag should silently fail (just does a printf to stdout). + fcurve0.group = other_group + self.assertEquals([group1], channelbag.groups[:]) + self.assertEquals([fcurve5, fcurve3], group1.channels[:]) + self.assertEquals([fcurve5, fcurve3, fcurve2, fcurve4, fcurve0, fcurve1], channelbag.fcurves[:]) + class DataPathTest(unittest.TestCase): def setUp(self):