diff --git a/source/blender/animrig/ANIM_bone_collections.h b/source/blender/animrig/ANIM_bone_collections.h index f770cd7906e..acd1cfdcc4d 100644 --- a/source/blender/animrig/ANIM_bone_collections.h +++ b/source/blender/animrig/ANIM_bone_collections.h @@ -74,6 +74,9 @@ struct BoneCollection *ANIM_armature_bonecoll_new(struct bArmature *armature, co /** * Add a bone collection to the Armature. * + * If `anchor` is null or isn't found, this inserts the copy at the start + * of the collection array. + * * NOTE: this should not typically be used. It is only used by the library overrides system to * apply override operations. */ @@ -82,6 +85,11 @@ struct BoneCollection *ANIM_armature_bonecoll_insert_copy_after( struct BoneCollection *anchor, const struct BoneCollection *bcoll_to_copy); +/** + * Remove the bone collection at `index` from the armature. + */ +void ANIM_armature_bonecoll_remove_from_index(bArmature *armature, const int index); + /** * Remove a bone collection from the armature. */ @@ -122,6 +130,15 @@ void ANIM_armature_bonecoll_active_name_set(struct bArmature *armature, const ch bool ANIM_armature_bonecoll_is_editable(const struct bArmature *armature, const struct BoneCollection *bcoll); +/** + * Moves the bone collection at from_index to to_index. + * + * \return true if the collection was successfully moved, false otherwise. + * The latter happens if either index is out of bounds, or if the indices + * are equal. + */ +bool ANIM_armature_bonecoll_move_to_index(bArmature *armature, int from_index, int to_index); + /** * Move the bone collection by \a step places up/down. * diff --git a/source/blender/animrig/ANIM_bone_collections.hh b/source/blender/animrig/ANIM_bone_collections.hh index 144670240fc..0f44dfa961a 100644 --- a/source/blender/animrig/ANIM_bone_collections.hh +++ b/source/blender/animrig/ANIM_bone_collections.hh @@ -35,6 +35,15 @@ namespace blender::animrig { * assumption is that the membership information in the collections will * be rebuilt from the EditBones when leaving edit mode. * + * The source and destination each have two parts: a heap-allocated array of + * `BoneCollection *`, and an integer that keeps track of that array's length. + * The destination parameters are pointers to those components, so they can + * be modified. The destination array should be empty and unallocated. + * + * \param bcoll_array_dst,bcoll_array_dst_num: the destination BoneCollection + * array and array size. + * \param bcoll_array_src,bcoll_array_src_num: the source BoneCollection array + * and array size. * \param do_id_user: when true, increments the user count of IDs that * the BoneCollections' custom properties point to, if any. * @@ -42,8 +51,12 @@ namespace blender::animrig { * pointers-to-the-duplicate-collections. This can be used to remap * collection pointers in other data, such as EditBones. */ -blender::Map ANIM_bonecoll_listbase_copy_no_membership( - ListBase *bone_colls_dst, ListBase *bone_colls_src, bool do_id_user); +blender::Map ANIM_bonecoll_array_copy_no_membership( + BoneCollection ***bcoll_array_dst, + int *bcoll_array_dst_num, + BoneCollection **bcoll_array_src, + int bcoll_array_src_num, + bool do_id_user); /** * Frees a list of BoneCollections. * @@ -54,9 +67,15 @@ blender::Map ANIM_bonecoll_listbase_copy_no_ * handle. Prefer using higher-level functions to remove BoneCollections * from Armatures. * + * \param bcoll_array: pointer to the heap-allocated array of `BoneCollection *` + * to be freed. + * \param bcoll_array_num: pointer to the integer that tracks the length of + * bcoll_array. * \param do_id_user: when true, decrements the user count of IDs that * the BoneCollections' custom properties point to, if any. */ -void ANIM_bonecoll_listbase_free(ListBase *bcolls, bool do_id_user); +void ANIM_bonecoll_array_free(BoneCollection ***bcoll_array, + int *bcoll_array_num, + bool do_id_user); } // namespace blender::animrig diff --git a/source/blender/animrig/intern/bone_collections.cc b/source/blender/animrig/intern/bone_collections.cc index 7f8c04fa753..39cc4aac54d 100644 --- a/source/blender/animrig/intern/bone_collections.cc +++ b/source/blender/animrig/intern/bone_collections.cc @@ -98,7 +98,7 @@ void ANIM_armature_runtime_refresh(bArmature *armature) ANIM_armature_bonecoll_active_set(armature, active); /* Construct the bone-to-collections mapping. */ - LISTBASE_FOREACH (BoneCollection *, bcoll, &armature->collections) { + for (BoneCollection *bcoll : armature->collections_span()) { add_reverse_pointers(bcoll); } } @@ -112,12 +112,85 @@ void ANIM_armature_runtime_free(bArmature *armature) static void bonecoll_ensure_name_unique(bArmature *armature, BoneCollection *bcoll) { - BLI_uniquename(&armature->collections, - bcoll, - DATA_(bonecoll_default_name), - '.', - offsetof(BoneCollection, name), - sizeof(bcoll->name)); + struct DupNameCheckData { + bArmature *arm; + BoneCollection *bcoll; + }; + + auto bonecoll_name_is_duplicate = [](void *arg, const char *name) -> bool { + DupNameCheckData *data = static_cast(arg); + for (BoneCollection *bcoll : data->arm->collections_span()) { + if (bcoll != data->bcoll && STREQ(bcoll->name, name)) { + return true; + } + } + return false; + }; + + DupNameCheckData check_data = {armature, bcoll}; + BLI_uniquename_cb(bonecoll_name_is_duplicate, + &check_data, + DATA_(bonecoll_default_name), + '.', + bcoll->name, + sizeof(bcoll->name)); +} + +/** + * Inserts bcoll into armature's array of bone collecions at index. + * + * Note: the specified index is where the given bone collection will end up. + * This means, for example, that for a collection array of length N, you can + * pass N as the index to append to the end. + */ +static void bonecoll_insert_at_index(bArmature *armature, BoneCollection *bcoll, const int index) +{ + BLI_assert(index <= armature->collection_array_num); + + armature->collection_array = (BoneCollection **)MEM_reallocN_id( + armature->collection_array, + sizeof(BoneCollection *) * (armature->collection_array_num + 1), + __func__); + + /* Shift over the collections to make room at the given index. */ + if (index < armature->collection_array_num) { + BoneCollection **start = armature->collection_array + index; + const size_t count = armature->collection_array_num - index; + memmove((void *)(start + 1), (void *)start, count * sizeof(BoneCollection *)); + } + + armature->collection_array[index] = bcoll; + armature->collection_array_num++; + + if (armature->runtime.active_collection_index >= index) { + ANIM_armature_bonecoll_active_index_set(armature, + armature->runtime.active_collection_index + 1); + } +} + +/** + * Appends bcoll to the end of armature's array of bone collections. + */ +static void bonecoll_append(bArmature *armature, BoneCollection *bcoll) +{ + bonecoll_insert_at_index(armature, bcoll, armature->collection_array_num); +} + +/** + * Returns the index of the given collection in the armature's collection array, + * or -1 if not found. + */ +static int bonecoll_find_index(const bArmature *armature, const BoneCollection *bcoll) +{ + int index = 0; + for (const BoneCollection *arm_bcoll : armature->collections_span()) { + if (arm_bcoll == bcoll) { + return index; + } + index++; + } + + return -1; } BoneCollection *ANIM_armature_bonecoll_new(bArmature *armature, const char *name) @@ -130,7 +203,8 @@ BoneCollection *ANIM_armature_bonecoll_new(bArmature *armature, const char *name } bonecoll_ensure_name_unique(armature, bcoll); - BLI_addtail(&armature->collections, bcoll); + bonecoll_append(armature, bcoll); + return bcoll; } @@ -153,7 +227,8 @@ BoneCollection *ANIM_armature_bonecoll_insert_copy_after(bArmature *armature, 0 /*do_id_user ? 0 : LIB_ID_CREATE_NO_USER_REFCOUNT*/); } - BLI_insertlinkafter(&armature->collections, anchor, bcoll); + const int anchor_index = bonecoll_find_index(armature, anchor); + bonecoll_insert_at_index(armature, bcoll, anchor_index + 1); bonecoll_ensure_name_unique(armature, bcoll); add_reverse_pointers(bcoll); @@ -174,7 +249,7 @@ void ANIM_armature_bonecoll_active_set(bArmature *armature, BoneCollection *bcol return; } - const int index = BLI_findindex(&armature->collections, bcoll); + const int index = bonecoll_find_index(armature, bcoll); if (index == -1) { /* TODO: print warning? Or just ignore this case? */ armature_bonecoll_active_clear(armature); @@ -188,18 +263,12 @@ void ANIM_armature_bonecoll_active_set(bArmature *armature, BoneCollection *bcol void ANIM_armature_bonecoll_active_index_set(bArmature *armature, const int bone_collection_index) { - if (bone_collection_index < 0) { + if (bone_collection_index < 0 || bone_collection_index >= armature->collection_array_num) { armature_bonecoll_active_clear(armature); return; } - void *found_link = BLI_findlink(&armature->collections, bone_collection_index); - BoneCollection *bcoll = static_cast(found_link); - if (bcoll == nullptr) { - /* TODO: print warning? Or just ignore this case? */ - armature_bonecoll_active_clear(armature); - return; - } + BoneCollection *bcoll = armature->collection_array[bone_collection_index]; STRNCPY(armature->active_collection_name, bcoll->name); armature->runtime.active_collection_index = bone_collection_index; @@ -224,19 +293,63 @@ bool ANIM_armature_bonecoll_is_editable(const bArmature *armature, const BoneCol return true; } +bool ANIM_armature_bonecoll_move_to_index(bArmature *armature, + const int from_index, + const int to_index) +{ + if (from_index >= armature->collection_array_num || to_index >= armature->collection_array_num || + from_index == to_index) + { + return false; + } + + BoneCollection *bcoll = armature->collection_array[from_index]; + + /* Shift collections over to fill the gap at from_index and make room at to_index. */ + if (from_index < to_index) { + BoneCollection **start = armature->collection_array + from_index + 1; + const size_t count = to_index - from_index; + memmove((void *)(start - 1), (void *)start, count * sizeof(BoneCollection *)); + } + else { + BoneCollection **start = armature->collection_array + to_index; + const size_t count = from_index - to_index; + memmove((void *)(start + 1), (void *)start, count * sizeof(BoneCollection *)); + } + + armature->collection_array[to_index] = bcoll; + + /* Adjust the active collection index. */ + if (from_index == armature->runtime.active_collection_index) { + /* If the active collection is the collection we moved. */ + ANIM_armature_bonecoll_active_index_set(armature, to_index); + } + else if (from_index <= armature->runtime.active_collection_index && + armature->runtime.active_collection_index <= to_index) + { + /* If the active collection is within the span of shifted collections. */ + const int offset = from_index < to_index ? -1 : 1; + ANIM_armature_bonecoll_active_index_set(armature, + armature->runtime.active_collection_index + offset); + } + + return true; +} + bool ANIM_armature_bonecoll_move(bArmature *armature, BoneCollection *bcoll, const int step) { if (bcoll == nullptr) { return false; } - if (!BLI_listbase_link_move(&armature->collections, bcoll, step)) { + const int bcoll_index = bonecoll_find_index(armature, bcoll); + const int to_index = bcoll_index + step; + if (bcoll_index < 0 || to_index < 0 || to_index >= armature->collection_array_num) { return false; } - if (bcoll == armature->runtime.active_collection) { - armature->runtime.active_collection_index = BLI_findindex(&armature->collections, bcoll); - } + ANIM_armature_bonecoll_move_to_index(armature, bcoll_index, to_index); + return true; } @@ -260,8 +373,13 @@ void ANIM_armature_bonecoll_name_set(bArmature *armature, BoneCollection *bcoll, BKE_animdata_fix_paths_rename_all(&armature->id, "collections", old_name, bcoll->name); } -void ANIM_armature_bonecoll_remove(bArmature *armature, BoneCollection *bcoll) +void ANIM_armature_bonecoll_remove_from_index(bArmature *armature, const int index) { + BLI_assert(0 <= index && index < armature->collection_array_num); + + BoneCollection *bcoll = armature->collection_array[index]; + + /* Remove bone membership. */ LISTBASE_FOREACH_MUTABLE (BoneCollectionMember *, member, &bcoll->bones) { ANIM_armature_bonecoll_unassign(bcoll, member->bone); } @@ -271,18 +389,44 @@ void ANIM_armature_bonecoll_remove(bArmature *armature, BoneCollection *bcoll) } } - BLI_remlink_safe(&armature->collections, bcoll); ANIM_bonecoll_free(bcoll); - /* Make sure the active collection is correct. */ - const int num_collections = BLI_listbase_count(&armature->collections); - const int active_index = min_ii(armature->runtime.active_collection_index, num_collections - 1); - ANIM_armature_bonecoll_active_index_set(armature, active_index); + /* Shift over the collections to fill the gap. */ + if (index < (armature->collection_array_num - 1)) { + BoneCollection **start = armature->collection_array + index; + const size_t count = armature->collection_array_num - index - 1; + memmove((void *)start, (void *)(start + 1), count * sizeof(BoneCollection *)); + } + + /* Note: we don't bother to shrink the allocation. It's okay if the + * capacity has extra space, because the number of valid items is tracked. */ + armature->collection_array_num--; + armature->collection_array[armature->collection_array_num] = nullptr; + + /* Update the active BoneCollection. */ + if (index <= armature->runtime.active_collection_index) { + int active_index = armature->runtime.active_collection_index; + + if (index == armature->collection_array_num) { + /* Removing the last element: activate the now-last element. */ + active_index--; + } + else if (index < active_index) { + /* The active collection shifted, because a collection before it was removed. */ + active_index--; + } + ANIM_armature_bonecoll_active_index_set(armature, active_index); + } +} + +void ANIM_armature_bonecoll_remove(bArmature *armature, BoneCollection *bcoll) +{ + ANIM_armature_bonecoll_remove_from_index(armature, bonecoll_find_index(armature, bcoll)); } BoneCollection *ANIM_armature_bonecoll_get_by_name(bArmature *armature, const char *name) { - LISTBASE_FOREACH (BoneCollection *, bcoll, &armature->collections) { + for (BoneCollection *bcoll : armature->collections_span()) { if (STREQ(bcoll->name, name)) { return bcoll; } @@ -421,7 +565,7 @@ bool ANIM_armature_bonecoll_unassign_editbone(BoneCollection *bcoll, EditBone *e void ANIM_armature_bonecoll_reconstruct(bArmature *armature) { /* Remove all the old collection memberships. */ - LISTBASE_FOREACH (BoneCollection *, bcoll, &armature->collections) { + for (BoneCollection *bcoll : armature->collections_span()) { BLI_freelistN(&bcoll->bones); } @@ -463,14 +607,14 @@ bool ANIM_bonecoll_is_visible_editbone(const bArmature * /*armature*/, const Edi void ANIM_armature_bonecoll_show_all(bArmature *armature) { - LISTBASE_FOREACH (BoneCollection *, bcoll, &armature->collections) { + for (BoneCollection *bcoll : armature->collections_span()) { ANIM_bonecoll_show(bcoll); } } void ANIM_armature_bonecoll_hide_all(bArmature *armature) { - LISTBASE_FOREACH (BoneCollection *, bcoll, &armature->collections) { + for (BoneCollection *bcoll : armature->collections_span()) { ANIM_bonecoll_hide(bcoll); } } @@ -532,13 +676,23 @@ namespace blender::animrig { /* Utility functions for Armature edit-mode undo. */ -blender::Map ANIM_bonecoll_listbase_copy_no_membership( - ListBase *bone_colls_dst, ListBase *bone_colls_src, const bool do_id_user) +blender::Map ANIM_bonecoll_array_copy_no_membership( + BoneCollection ***bcoll_array_dst, + int *bcoll_array_dst_num, + BoneCollection **bcoll_array_src, + const int bcoll_array_src_num, + const bool do_id_user) { - BLI_assert(BLI_listbase_is_empty(bone_colls_dst)); + BLI_assert(*bcoll_array_dst == nullptr); + BLI_assert(*bcoll_array_dst_num == 0); + + *bcoll_array_dst = static_cast( + MEM_malloc_arrayN(bcoll_array_src_num, sizeof(BoneCollection *), __func__)); + *bcoll_array_dst_num = bcoll_array_src_num; blender::Map bcoll_map{}; - LISTBASE_FOREACH (BoneCollection *, bcoll_src, bone_colls_src) { + for (int i = 0; i < bcoll_array_src_num; i++) { + BoneCollection *bcoll_src = bcoll_array_src[i]; BoneCollection *bcoll_dst = static_cast(MEM_dupallocN(bcoll_src)); /* This will be rebuilt from the edit bones, so we don't need to copy it. */ @@ -548,16 +702,22 @@ blender::Map ANIM_bonecoll_listbase_copy_no_ bcoll_dst->prop = IDP_CopyProperty_ex(bcoll_src->prop, do_id_user ? 0 : LIB_ID_CREATE_NO_USER_REFCOUNT); } - BLI_addtail(bone_colls_dst, bcoll_dst); + + (*bcoll_array_dst)[i] = bcoll_dst; + bcoll_map.add(bcoll_src, bcoll_dst); } return bcoll_map; } -void ANIM_bonecoll_listbase_free(ListBase *bcolls, const bool do_id_user) +void ANIM_bonecoll_array_free(BoneCollection ***bcoll_array, + int *bcoll_array_num, + const bool do_id_user) { - LISTBASE_FOREACH_MUTABLE (BoneCollection *, bcoll, bcolls) { + for (int i = 0; i < *bcoll_array_num; i++) { + BoneCollection *bcoll = (*bcoll_array)[i]; + if (bcoll->prop) { IDP_FreeProperty_ex(bcoll->prop, do_id_user); } @@ -568,8 +728,13 @@ void ANIM_bonecoll_listbase_free(ListBase *bcolls, const bool do_id_user) * list on the Armature itself before copying over the undo BoneCollection * list, in which case this of Bone pointers may not be empty. */ BLI_freelistN(&bcoll->bones); + + MEM_freeN(bcoll); } - BLI_freelistN(bcolls); + MEM_freeN(*bcoll_array); + + *bcoll_array = nullptr; + *bcoll_array_num = 0; } } // namespace blender::animrig diff --git a/source/blender/animrig/intern/bone_collections_test.cc b/source/blender/animrig/intern/bone_collections_test.cc index f55dc356d4a..e57117d9a98 100644 --- a/source/blender/animrig/intern/bone_collections_test.cc +++ b/source/blender/animrig/intern/bone_collections_test.cc @@ -56,13 +56,19 @@ class ANIM_armature_bone_collections : public testing::Test { BLI_addtail(&arm.bonebase, &bone1); /* bone1 is root bone. */ BLI_addtail(&arm.bonebase, &bone2); /* bone2 is root bone. */ BLI_addtail(&bone2.childbase, &bone3); /* bone3 has bone2 as parent. */ + + BKE_armature_bone_hash_make(&arm); } void TearDown() override { - LISTBASE_FOREACH_BACKWARD_MUTABLE (BoneCollection *, bcoll, &arm.collections) { - ANIM_armature_bonecoll_remove(&arm, bcoll); + while (arm.collection_array_num > 0) { + ANIM_armature_bonecoll_remove_from_index(&arm, arm.collection_array_num - 1); } + if (arm.collection_array) { + MEM_freeN(arm.collection_array); + } + BKE_armature_bone_hash_free(&arm); } }; @@ -210,4 +216,49 @@ TEST_F(ANIM_armature_bone_collections, bcoll_is_editable) << "Expecting local bone collection on local armature override to be editable"; } +TEST_F(ANIM_armature_bone_collections, bcoll_insert_copy_after) +{ + BoneCollection *bcoll1 = ANIM_armature_bonecoll_new(&arm, "collection"); + BoneCollection *bcoll2 = ANIM_armature_bonecoll_new(&arm, "collection"); + BoneCollection *bcoll3 = ANIM_armature_bonecoll_new(&arm, "collection"); + + EXPECT_EQ(arm.collection_array[0], bcoll1); + EXPECT_EQ(arm.collection_array[1], bcoll2); + EXPECT_EQ(arm.collection_array[2], bcoll3); + + BoneCollection *bcoll4 = ANIM_armature_bonecoll_insert_copy_after(&arm, bcoll2, bcoll2); + + EXPECT_EQ(arm.collection_array[0], bcoll1); + EXPECT_EQ(arm.collection_array[1], bcoll2); + EXPECT_EQ(arm.collection_array[2], bcoll4); + EXPECT_EQ(arm.collection_array[3], bcoll3); +} + +TEST_F(ANIM_armature_bone_collections, bcoll_move_to_index) +{ + BoneCollection *bcoll1 = ANIM_armature_bonecoll_new(&arm, "collection"); + BoneCollection *bcoll2 = ANIM_armature_bonecoll_new(&arm, "collection"); + BoneCollection *bcoll3 = ANIM_armature_bonecoll_new(&arm, "collection"); + BoneCollection *bcoll4 = ANIM_armature_bonecoll_new(&arm, "collection"); + + EXPECT_EQ(arm.collection_array[0], bcoll1); + EXPECT_EQ(arm.collection_array[1], bcoll2); + EXPECT_EQ(arm.collection_array[2], bcoll3); + EXPECT_EQ(arm.collection_array[3], bcoll4); + + ANIM_armature_bonecoll_move_to_index(&arm, 2, 1); + + EXPECT_EQ(arm.collection_array[0], bcoll1); + EXPECT_EQ(arm.collection_array[1], bcoll3); + EXPECT_EQ(arm.collection_array[2], bcoll2); + EXPECT_EQ(arm.collection_array[3], bcoll4); + + ANIM_armature_bonecoll_move_to_index(&arm, 0, 3); + + EXPECT_EQ(arm.collection_array[0], bcoll3); + EXPECT_EQ(arm.collection_array[1], bcoll2); + EXPECT_EQ(arm.collection_array[2], bcoll4); + EXPECT_EQ(arm.collection_array[3], bcoll1); +} + } // namespace blender::animrig::tests diff --git a/source/blender/blenkernel/intern/armature.cc b/source/blender/blenkernel/intern/armature.cc index 67a76a4cd7d..4463af45b32 100644 --- a/source/blender/blenkernel/intern/armature.cc +++ b/source/blender/blenkernel/intern/armature.cc @@ -23,6 +23,7 @@ #include "BLI_math_matrix.h" #include "BLI_math_rotation.h" #include "BLI_math_vector.h" +#include "BLI_span.hh" #include "BLI_string.h" #include "BLI_utildefines.h" #include "BLT_translation.h" @@ -88,6 +89,31 @@ static void armature_init_data(ID *id) MEMCPY_STRUCT_AFTER(armature, DNA_struct_default_get(bArmature), id); } +/** + * Copies the bone collection in `bcoll_src` to `bcoll_dst`, re-hooking up all + * of the bone relationships to the bones in `armature_dst`. + * + * Note: this function's use case is narrow in scope, intended only for use in + * `armature_copy_data()` below. You probably don't want to use this otherwise. + */ +static void copy_bone_collection(bArmature *armature_dst, + BoneCollection *&bcoll_dst, + const BoneCollection *bcoll_src) +{ + bcoll_dst = static_cast(MEM_dupallocN(bcoll_src)); + + /* ID properties. */ + if (bcoll_dst->prop) { + bcoll_dst->prop = IDP_CopyProperty(bcoll_dst->prop); + } + + /* Bone references. */ + BLI_duplicatelist(&bcoll_dst->bones, &bcoll_dst->bones); + LISTBASE_FOREACH (BoneCollectionMember *, member, &bcoll_dst->bones) { + member->bone = BKE_armature_find_bone_name(armature_dst, member->bone->name); + } +} + /** * Only copy internal data of Armature ID from source * to already allocated/initialized destination. @@ -138,19 +164,19 @@ static void armature_copy_data(Main * /*bmain*/, ID *id_dst, const ID *id_src, c armature_dst->act_edbone = nullptr; /* Duplicate bone collections & assignments. */ - BLI_duplicatelist(&armature_dst->collections, &armature_src->collections); - LISTBASE_FOREACH (BoneCollection *, bcoll, &armature_dst->collections) { - /* ID properties. */ - if (bcoll->prop) { - bcoll->prop = IDP_CopyProperty(bcoll->prop); - } - - /* Bone references. */ - BLI_duplicatelist(&bcoll->bones, &bcoll->bones); - LISTBASE_FOREACH (BoneCollectionMember *, member, &bcoll->bones) { - member->bone = BKE_armature_find_bone_name(armature_dst, member->bone->name); + if (armature_src->collection_array) { + armature_dst->collection_array = static_cast( + MEM_dupallocN(armature_src->collection_array)); + armature_dst->collection_array_num = armature_src->collection_array_num; + for (int i = 0; i < armature_src->collection_array_num; i++) { + copy_bone_collection( + armature_dst, armature_dst->collection_array[i], armature_src->collection_array[i]); } } + else { + armature_dst->collection_array = nullptr; + armature_dst->collection_array_num = 0; + } ANIM_armature_bonecoll_active_index_set(armature_dst, armature_src->runtime.active_collection_index); @@ -164,17 +190,15 @@ static void armature_free_data(ID *id) ANIM_armature_runtime_free(armature); /* Free all BoneCollectionMembership objects. */ - LISTBASE_FOREACH_MUTABLE (BoneCollection *, bcoll, &armature->collections) { - /* ID properties. */ - if (bcoll->prop) { - IDP_FreeProperty(bcoll->prop); - bcoll->prop = nullptr; + if (armature->collection_array) { + for (BoneCollection *bcoll : armature->collections_span()) { + BLI_freelistN(&bcoll->bones); + ANIM_bonecoll_free(bcoll); } - - /* Bone references. */ - BLI_freelistN(&bcoll->bones); + MEM_freeN(armature->collection_array); } - BLI_freelistN(&armature->collections); + armature->collection_array = nullptr; + armature->collection_array_num = 0; BKE_armature_bone_hash_free(armature); BKE_armature_bonelist_free(&armature->bonebase, false); @@ -232,7 +256,7 @@ static void armature_foreach_id(ID *id, LibraryForeachIDData *data) } } - LISTBASE_FOREACH (BoneCollection *, bcoll, &arm->collections) { + for (BoneCollection *bcoll : arm->collections_span()) { BKE_LIB_FOREACHID_PROCESS_FUNCTION_CALL(data, armature_foreach_id_bone_collection(bcoll, data)); } @@ -289,18 +313,39 @@ static void armature_blend_write(BlendWriter *writer, ID *id, const void *id_add const bArmature_Runtime runtime_backup = arm->runtime; memset(&arm->runtime, 0, sizeof(arm->runtime)); + /* Convert BoneCollections over to a listbase for writing. */ + BoneCollection **collection_array_backup = arm->collection_array; + if (arm->collection_array_num > 0) { + for (int i = 0; i < arm->collection_array_num - 1; i++) { + arm->collection_array[i]->next = arm->collection_array[i + 1]; + arm->collection_array[i + 1]->prev = arm->collection_array[i]; + } + arm->collections.first = arm->collection_array[0]; + arm->collections.last = arm->collection_array[arm->collection_array_num - 1]; + arm->collection_array = nullptr; + } + BLO_write_id_struct(writer, bArmature, id_address, &arm->id); BKE_id_blend_write(writer, &arm->id); - arm->runtime = runtime_backup; - /* Direct data */ LISTBASE_FOREACH (Bone *, bone, &arm->bonebase) { write_bone(writer, bone); } + LISTBASE_FOREACH (BoneCollection *, bcoll, &arm->collections) { write_bone_collection(writer, bcoll); } + + /* Restore the BoneCollection array and clear the listbase. */ + arm->collection_array = collection_array_backup; + for (int i = 0; i < arm->collection_array_num - 1; i++) { + arm->collection_array[i]->next = nullptr; + arm->collection_array[i + 1]->prev = nullptr; + } + BLI_listbase_clear(&arm->collections); + + arm->runtime = runtime_backup; } static void direct_link_bones(BlendDataReader *reader, Bone *bone) @@ -334,6 +379,42 @@ static void direct_link_bone_collection(BlendDataReader *reader, BoneCollection } } +static void read_bone_collections(BlendDataReader *reader, bArmature *arm) +{ + /* Read as listbase, but convert to an array on the armature. */ + BLO_read_list(reader, &arm->collections); + arm->collection_array_num = BLI_listbase_count(&arm->collections); + arm->collection_array = (BoneCollection **)MEM_malloc_arrayN( + arm->collection_array_num, sizeof(BoneCollection *), __func__); + { + int i; + LISTBASE_FOREACH_INDEX (BoneCollection *, bcoll, &arm->collections, i) { + arm->collection_array[i] = bcoll; + } + } + + /* We don't need the listbase or prev/next pointers because the + * collections are stored in an array. */ + for (int i = 0; i < arm->collection_array_num - 1; i++) { + arm->collection_array[i]->next = nullptr; + arm->collection_array[i + 1]->prev = nullptr; + } + BLI_listbase_clear(&arm->collections); + + /* Bone collections added via an override can be edited, but ones that already exist in + another + * blend file (so on the linked Armature) should not be touched. */ + const bool reset_bcoll_override_flag = ID_IS_LINKED(&arm->id); + for (BoneCollection *bcoll : arm->collections_span()) { + direct_link_bone_collection(reader, bcoll); + if (reset_bcoll_override_flag) { + /* The linked Armature may have overrides in the library file already, and + * those should *not* be editable here. */ + bcoll->flags &= ~BONE_COLLECTION_OVERRIDE_LIBRARY_LOCAL; + } + } +} + static void armature_blend_read_data(BlendDataReader *reader, ID *id) { bArmature *arm = (bArmature *)id; @@ -347,19 +428,7 @@ static void armature_blend_read_data(BlendDataReader *reader, ID *id) direct_link_bones(reader, bone); } - BLO_read_list(reader, &arm->collections); - - /* Bone collections added via an override can be edited, but ones that already exist in another - * blend file (so on the linked Armature) should not be touched. */ - const bool reset_bcoll_override_flag = ID_IS_LINKED(&arm->id); - LISTBASE_FOREACH (BoneCollection *, bcoll, &arm->collections) { - direct_link_bone_collection(reader, bcoll); - if (reset_bcoll_override_flag) { - /* The linked Armature may have overrides in the library file already, and - * those should *not* be editable here. */ - bcoll->flags &= ~BONE_COLLECTION_OVERRIDE_LIBRARY_LOCAL; - } - } + read_bone_collections(reader, arm); BLO_read_data_address(reader, &arm->act_bone); arm->act_edbone = nullptr; @@ -3047,4 +3116,16 @@ bPoseChannel *BKE_armature_splineik_solver_find_root(bPoseChannel *pchan, return rootchan; } +/* -------------------------------------------------------------------- */ + +blender::Span bArmature::collections_span() const +{ + return blender::Span(collection_array, collection_array_num); +} + +blender::Span bArmature::collections_span() +{ + return blender::Span(collection_array, collection_array_num); +} + /** \} */ diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc index da6aa681cec..193c0d01584 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc @@ -16,6 +16,7 @@ #include "MEM_guardedalloc.h" #include "BLI_blenlib.h" +#include "BLI_span.hh" #include "BLI_string.h" #include "BLI_utildefines.h" @@ -1814,7 +1815,7 @@ void DepsgraphNodeBuilder::build_armature(bArmature *armature) add_operation_node( &armature->id, NodeType::ARMATURE, OperationCode::ARMATURE_EVAL, [](::Depsgraph *) {}); build_armature_bones(&armature->bonebase); - build_armature_bone_collections(&armature->collections); + build_armature_bone_collections(armature->collections_span()); } void DepsgraphNodeBuilder::build_armature_bones(ListBase *bones) @@ -1825,9 +1826,10 @@ void DepsgraphNodeBuilder::build_armature_bones(ListBase *bones) } } -void DepsgraphNodeBuilder::build_armature_bone_collections(ListBase *collections) +void DepsgraphNodeBuilder::build_armature_bone_collections( + blender::Span collections) { - LISTBASE_FOREACH (BoneCollection *, bcoll, collections) { + for (BoneCollection *bcoll : collections) { build_idproperties(bcoll->prop); } } diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.h b/source/blender/depsgraph/intern/builder/deg_builder_nodes.h index 1ea6125a970..9124480ed25 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.h +++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.h @@ -8,6 +8,8 @@ #pragma once +#include "BLI_span.hh" + #include "intern/builder/deg_builder.h" #include "intern/builder/deg_builder_key.h" #include "intern/builder/deg_builder_map.h" @@ -247,7 +249,7 @@ class DepsgraphNodeBuilder : public DepsgraphBuilder { virtual void build_rig(Object *object); virtual void build_armature(bArmature *armature); virtual void build_armature_bones(ListBase *bones); - virtual void build_armature_bone_collections(ListBase *collections); + virtual void build_armature_bone_collections(blender::Span collections); virtual void build_shapekeys(Key *key); virtual void build_camera(Camera *camera); virtual void build_light(Light *lamp); diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc index 05fdead2f18..480a9d60805 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc @@ -17,6 +17,7 @@ #include "MEM_guardedalloc.h" #include "BLI_blenlib.h" +#include "BLI_span.hh" #include "BLI_utildefines.h" #include "DNA_action_types.h" @@ -2722,7 +2723,7 @@ void DepsgraphRelationBuilder::build_armature(bArmature *armature) build_animdata(&armature->id); build_parameters(&armature->id); build_armature_bones(&armature->bonebase); - build_armature_bone_collections(&armature->collections); + build_armature_bone_collections(armature->collections_span()); } void DepsgraphRelationBuilder::build_armature_bones(ListBase *bones) @@ -2733,9 +2734,10 @@ void DepsgraphRelationBuilder::build_armature_bones(ListBase *bones) } } -void DepsgraphRelationBuilder::build_armature_bone_collections(ListBase *collections) +void DepsgraphRelationBuilder::build_armature_bone_collections( + blender::Span collections) { - LISTBASE_FOREACH (BoneCollection *, bcoll, collections) { + for (BoneCollection *bcoll : collections) { build_idproperties(bcoll->prop); } } diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.h b/source/blender/depsgraph/intern/builder/deg_builder_relations.h index b01ebe95c12..573eb0a7528 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_relations.h +++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.h @@ -17,6 +17,7 @@ #include "RNA_path.hh" +#include "BLI_span.hh" #include "BLI_string.h" #include "BLI_utildefines.h" @@ -226,7 +227,7 @@ class DepsgraphRelationBuilder : public DepsgraphBuilder { virtual void build_shapekeys(Key *key); virtual void build_armature(bArmature *armature); virtual void build_armature_bones(ListBase *bones); - virtual void build_armature_bone_collections(ListBase *collections); + virtual void build_armature_bone_collections(blender::Span collections); virtual void build_camera(Camera *camera); virtual void build_light(Light *lamp); virtual void build_nodetree(bNodeTree *ntree); diff --git a/source/blender/editors/armature/armature_relations.cc b/source/blender/editors/armature/armature_relations.cc index 1a49e38e890..9d34ecb9a30 100644 --- a/source/blender/editors/armature/armature_relations.cc +++ b/source/blender/editors/armature/armature_relations.cc @@ -297,7 +297,7 @@ int ED_armature_join_objects_exec(bContext *C, wmOperator *op) /* Index bone collections by name. This is also used later to keep track * of collections added from other armatures. */ blender::Map bone_collection_by_name; - LISTBASE_FOREACH (BoneCollection *, bcoll, &arm->collections) { + for (BoneCollection *bcoll : arm->collections_span()) { bone_collection_by_name.add(bcoll->name, bcoll); } @@ -335,15 +335,12 @@ int ED_armature_join_objects_exec(bContext *C, wmOperator *op) * already broken in that situation. When that gets fixed, this should * also get fixed. Note that copying the collections should include * copying their custom properties. (Nathan Vegdahl) */ - LISTBASE_FOREACH_MUTABLE (BoneCollection *, bcoll, &curarm->collections) { + for (BoneCollection *bcoll : curarm->collections_span()) { BoneCollection *mapped = bone_collection_by_name.lookup_default(bcoll->name, nullptr); - if (!mapped) { - BLI_remlink(&curarm->collections, bcoll); - BLI_addtail(&arm->collections, bcoll); - - bone_collection_by_name.add(bcoll->name, bcoll); - mapped = bcoll; + BoneCollection *new_bcoll = ANIM_armature_bonecoll_new(arm, bcoll->name); + bone_collection_by_name.add(bcoll->name, new_bcoll); + mapped = new_bcoll; } bone_collection_remap.add(bcoll, mapped); diff --git a/source/blender/editors/armature/bone_collections.cc b/source/blender/editors/armature/bone_collections.cc index 4589a9fd7b5..1a09453c359 100644 --- a/source/blender/editors/armature/bone_collections.cc +++ b/source/blender/editors/armature/bone_collections.cc @@ -782,9 +782,7 @@ static BoneCollection *add_or_move_to_collection_bcoll(wmOperator *op, bArmature ANIM_armature_bonecoll_active_set(arm, target_bcoll); } else { - target_bcoll = static_cast( - BLI_findlink(&arm->collections, collection_index)); - if (target_bcoll == nullptr) { + if (collection_index >= arm->collection_array_num) { BKE_reportf(op->reports, RPT_ERROR, "Bone collection with index %d not found on Armature %s", @@ -792,6 +790,7 @@ static BoneCollection *add_or_move_to_collection_bcoll(wmOperator *op, bArmature arm->id.name + 2); return nullptr; } + target_bcoll = arm->collection_array[collection_index]; } if (!ANIM_armature_bonecoll_is_editable(arm, target_bcoll)) { @@ -896,8 +895,8 @@ static bool bone_collection_enum_itemf_for_object(Object *ob, EnumPropertyItem item_tmp = {0}; bArmature *arm = static_cast(ob->data); - int bcoll_index = 0; - LISTBASE_FOREACH_INDEX (BoneCollection *, bcoll, &arm->collections, bcoll_index) { + for (int bcoll_index = 0; bcoll_index < arm->collection_array_num; bcoll_index++) { + BoneCollection *bcoll = arm->collection_array[bcoll_index]; if (!ANIM_armature_bonecoll_is_editable(arm, bcoll)) { /* Skip bone collections that cannot be assigned to because they're * linked and thus uneditable. If there is a way to still show these, but in a disabled diff --git a/source/blender/editors/armature/editarmature_undo.cc b/source/blender/editors/armature/editarmature_undo.cc index 0ee710f799d..47be9094d9a 100644 --- a/source/blender/editors/armature/editarmature_undo.cc +++ b/source/blender/editors/armature/editarmature_undo.cc @@ -73,7 +73,8 @@ struct UndoArmature { EditBone *act_edbone; char active_collection_name[MAX_NAME]; ListBase /* EditBone */ ebones; - ListBase /* BoneCollection */ bone_collections; + BoneCollection **collection_array; + int collection_array_num; size_t undo_size; }; @@ -96,9 +97,12 @@ static void undoarm_to_editarm(UndoArmature *uarm, bArmature *arm) ED_armature_ebone_listbase_temp_clear(arm->edbo); /* Copy bone collections. */ - ANIM_bonecoll_listbase_free(&arm->collections, true); - auto bcoll_map = ANIM_bonecoll_listbase_copy_no_membership( - &arm->collections, &uarm->bone_collections, true); + ANIM_bonecoll_array_free(&arm->collection_array, &arm->collection_array_num, true); + auto bcoll_map = ANIM_bonecoll_array_copy_no_membership(&arm->collection_array, + &arm->collection_array_num, + uarm->collection_array, + uarm->collection_array_num, + true); /* Always do a lookup-by-name and assignment. Even when the name of the active collection is * still the same, the order may have changed and thus the index needs to be updated. */ @@ -127,8 +131,11 @@ static void *undoarm_from_editarm(UndoArmature *uarm, bArmature *arm) ED_armature_ebone_listbase_temp_clear(&uarm->ebones); /* Copy bone collections. */ - auto bcoll_map = ANIM_bonecoll_listbase_copy_no_membership( - &uarm->bone_collections, &arm->collections, false); + auto bcoll_map = ANIM_bonecoll_array_copy_no_membership(&uarm->collection_array, + &uarm->collection_array_num, + arm->collection_array, + arm->collection_array_num, + false); STRNCPY(uarm->active_collection_name, arm->active_collection_name); /* Point the new edit bones at the new collections. */ @@ -142,7 +149,10 @@ static void *undoarm_from_editarm(UndoArmature *uarm, bArmature *arm) uarm->undo_size += sizeof(BoneCollectionReference) * BLI_listbase_count(&ebone->bone_collections); } - uarm->undo_size += sizeof(BoneCollection) * BLI_listbase_count(&uarm->bone_collections); + /* Size of the bone collections + the size of the pointers to those + * bone collections in the bone collection array. */ + uarm->undo_size += (sizeof(BoneCollection) + sizeof(BoneCollection *)) * + uarm->collection_array_num; return uarm; } @@ -150,7 +160,7 @@ static void *undoarm_from_editarm(UndoArmature *uarm, bArmature *arm) static void undoarm_free_data(UndoArmature *uarm) { ED_armature_ebone_listbase_free(&uarm->ebones, false); - ANIM_bonecoll_listbase_free(&uarm->bone_collections, false); + ANIM_bonecoll_array_free(&uarm->collection_array, &uarm->collection_array_num, false); } static Object *editarm_object_from_context(bContext *C) diff --git a/source/blender/editors/space_outliner/tree/tree_element_bone_collection.cc b/source/blender/editors/space_outliner/tree/tree_element_bone_collection.cc index 9b4cd29cc2a..d7feb7a8316 100644 --- a/source/blender/editors/space_outliner/tree/tree_element_bone_collection.cc +++ b/source/blender/editors/space_outliner/tree/tree_element_bone_collection.cc @@ -30,8 +30,8 @@ TreeElementBoneCollectionBase::TreeElementBoneCollectionBase(TreeElement &legacy void TreeElementBoneCollectionBase::expand(SpaceOutliner & /*space_outliner*/) const { - int index; - LISTBASE_FOREACH_INDEX (BoneCollection *, bcoll, &armature_.collections, index) { + for (int index = 0; index < armature_.collection_array_num; index++) { + BoneCollection *bcoll = armature_.collection_array[index]; add_element( &legacy_te_.subtree, &armature_.id, bcoll, &legacy_te_, TSE_BONE_COLLECTION, index); } diff --git a/source/blender/editors/space_outliner/tree/tree_element_id_armature.cc b/source/blender/editors/space_outliner/tree/tree_element_id_armature.cc index 72d3bf80882..3382017d84d 100644 --- a/source/blender/editors/space_outliner/tree/tree_element_id_armature.cc +++ b/source/blender/editors/space_outliner/tree/tree_element_id_armature.cc @@ -48,7 +48,7 @@ void TreeElementIDArmature::expand(SpaceOutliner &space_outliner) const } } - if (!BLI_listbase_is_empty(&arm_.collections)) { + if (arm_.collection_array_num > 0) { add_element(&legacy_te_.subtree, &arm_.id, nullptr, &legacy_te_, TSE_BONE_COLLECTION_BASE, 0); } } diff --git a/source/blender/makesdna/DNA_armature_types.h b/source/blender/makesdna/DNA_armature_types.h index b88b13129d1..4a11fc47bc5 100644 --- a/source/blender/makesdna/DNA_armature_types.h +++ b/source/blender/makesdna/DNA_armature_types.h @@ -18,6 +18,7 @@ #include "BLI_utildefines.h" #ifdef __cplusplus +# include "BLI_span.hh" namespace blender::animrig { class BoneColor; } @@ -187,8 +188,15 @@ typedef struct bArmature { short deformflag; short pathflag; - /* BoneCollection. */ - ListBase collections; + /** This is used only for reading/writing BoneCollections in blend + * files, for forwards/backwards compatibility with Blender 4.0. It + * should always be empty at runtime. + * Use collection_array for everything other than file reading/writing. */ + ListBase collections; /* BoneCollection. */ + + struct BoneCollection **collection_array; /* Array of `collection_array_num` BoneCollections. */ + int collection_array_num; + char _pad2[4]; /** Do not directly assign, use `ANIM_armature_bonecoll_active_set` instead. * This is stored as a string to make it possible for the library overrides system to understand @@ -206,6 +214,12 @@ typedef struct bArmature { /** Keep last, for consistency with the position of other DNA runtime structures. */ struct bArmature_Runtime runtime; + +#ifdef __cplusplus + /* Collection array access for convenient for-loop iteration. */ + blender::Span collections_span() const; + blender::Span collections_span(); +#endif } bArmature; /** diff --git a/source/blender/makesrna/intern/rna_armature.cc b/source/blender/makesrna/intern/rna_armature.cc index f4c8a6a1eb9..b37c4f3a44b 100644 --- a/source/blender/makesrna/intern/rna_armature.cc +++ b/source/blender/makesrna/intern/rna_armature.cc @@ -185,6 +185,23 @@ static void rna_Armature_edit_bone_remove(bArmature *arm, RNA_POINTER_INVALIDATE(ebone_ptr); } +static void rna_iterator_bone_collections_begin(CollectionPropertyIterator *iter, PointerRNA *ptr) +{ + bArmature *arm = (bArmature *)ptr->data; + rna_iterator_array_begin(iter, + arm->collection_array, + sizeof(BoneCollection *), + arm->collection_array_num, + false, + nullptr); +} + +static int rna_iterator_bone_collections_length(PointerRNA *ptr) +{ + bArmature *arm = (bArmature *)ptr->data; + return arm->collection_array_num; +} + static void rna_BoneCollections_active_set(PointerRNA *ptr, PointerRNA value, struct ReportList * /*reports*/) @@ -216,7 +233,7 @@ static void rna_BoneCollections_active_index_range( // TODO: Figure out what this function actually is used for, as we may want to protect the first // collection (i.e. the default collection that should remain first). *min = 0; - *max = max_ii(0, BLI_listbase_count(&arm->collections) - 1); + *max = max_ii(0, arm->collection_array_num - 1); } static void rna_BoneCollections_active_name_set(PointerRNA *ptr, const char *name) @@ -228,9 +245,9 @@ static void rna_BoneCollections_active_name_set(PointerRNA *ptr, const char *nam static void rna_BoneCollections_move(bArmature *arm, ReportList *reports, int from, int to) { - const int count = BLI_listbase_count(&arm->collections); + const int count = arm->collection_array_num; if (from < 0 || from >= count || to < 0 || to >= count || - (from != to && !BLI_listbase_move_index(&arm->collections, from, to))) + (from != to && !ANIM_armature_bonecoll_move_to_index(arm, from, to))) { BKE_reportf(reports, RPT_ERROR, "Cannot move collection from index '%d' to '%d'", from, to); } @@ -1952,8 +1969,16 @@ static void rna_def_armature(BlenderRNA *brna) rna_def_armature_edit_bones(brna, prop); prop = RNA_def_property(srna, "collections", PROP_COLLECTION, PROP_NONE); - RNA_def_property_collection_sdna(prop, nullptr, "collections", nullptr); RNA_def_property_struct_type(prop, "BoneCollection"); + RNA_def_property_collection_funcs(prop, + "rna_iterator_bone_collections_begin", + "rna_iterator_array_next", + "rna_iterator_array_end", + "rna_iterator_array_dereference_get", + "rna_iterator_bone_collections_length", + nullptr, /* TODO */ + nullptr, /* TODO */ + nullptr); RNA_def_property_ui_text(prop, "Bone Collections", ""); RNA_def_property_override_funcs( prop, nullptr, nullptr, "rna_Armature_collections_override_apply");