tmaq
/
tornavis
1
0
Fork 0
Code Issues 2 Pull Requests 29 Packages Projects 24 Releases Wiki Activity

Anim: Bone Collections, replace `.move_to_parent()` with `.child_number` 

Instead of moving bone collections by absolute index, they can now be
moved by manipulating `.child_number`. This is the relative index of the
bone collection within the list of its siblings.

This replaces the much more cumbersome `collections.move_to_parent()`
function. Since that function is now no longer necessary (it's been
replaced by assignment to `.parent` and `.child_number`), it's removed
from RNA. Note that this function was never part of even a beta build of
Blender.
This commit is contained in:
Sybren A. Stüvel 2024-01-05 16:51:26 +01:00
parent 6444eeb14c
commit 66cdc112fc
6 changed files with 150 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
source/blender/animrig/ANIM_bone_collections.hh
View File

@ -327,6 +327,28 @@ int armature_bonecoll_find_index(const bArmature *armature, const ::BoneCollecti
*/
int armature_bonecoll_find_parent_index(const bArmature *armature, int bcoll_index);
/**
* Find the child number of this bone collection.
*
* This is the offset of this collection relative to the parent's first child.
* In other words, the first child has number 0, second child has number 1, etc.
*
* This requires a scan of the array, hence the function is called 'find' and not 'get'.
*/
int armature_bonecoll_child_number_find(const bArmature *armature, const ::BoneCollection *bcoll);
/**
* Move this bone collection to a new child number.
*
* \return the new absolute index of the bone collection, or -1 if the new child number was not
* valid.
*
* \see armature_bonecoll_child_number_find
*/
int armature_bonecoll_child_number_set(bArmature *armature,
::BoneCollection *bcoll,
int new_child_number);
bool armature_bonecoll_is_root(const bArmature *armature, int bcoll_index);
bool armature_bonecoll_is_child_of(const bArmature *armature,

49
source/blender/animrig/intern/bone_collections.cc
View File

@ -1075,6 +1075,55 @@ int armature_bonecoll_find_parent_index(const bArmature *armature, const int bco
return -1;
}
int armature_bonecoll_child_number_find(const bArmature *armature, const ::BoneCollection *bcoll)
{
const int bcoll_index = armature_bonecoll_find_index(armature, bcoll);
const int parent_index = armature_bonecoll_find_parent_index(armature, bcoll_index);
return bonecoll_child_number(armature, parent_index, bcoll_index);
}
int armature_bonecoll_child_number_set(bArmature *armature,
::BoneCollection *bcoll,
int new_child_number)
{
const int bcoll_index = armature_bonecoll_find_index(armature, bcoll);
const int parent_index = armature_bonecoll_find_parent_index(armature, bcoll_index);
BoneCollection fake_armature_parent = {};
fake_armature_parent.child_count = armature->collection_root_count;
BoneCollection *parent_bcoll;
if (parent_index < 0) {
parent_bcoll = &fake_armature_parent;
}
else {
parent_bcoll = armature->collection_array[parent_index];
}
/* Bounds checks. */
if (new_child_number >= parent_bcoll->child_count) {
return -1;
}
if (new_child_number < 0) {
new_child_number = parent_bcoll->child_count - 1;
}
/* Store the parent's child_index, as that might move if to_index is the first child
* (bonecolls_move_to_index() will keep it pointing at that first child). */
const int old_parent_child_index = parent_bcoll->child_index;
const int to_index = parent_bcoll->child_index + new_child_number;
internal::bonecolls_move_to_index(armature, bcoll_index, to_index);
parent_bcoll->child_index = old_parent_child_index;
/* Make sure that if this was the active bone collection, its index also changes. */
if (armature->runtime.active_collection_index == bcoll_index) {
ANIM_armature_bonecoll_active_index_set(armature, to_index);
}
return to_index;
}
bool armature_bonecoll_is_root(const bArmature *armature, const int bcoll_index)
{
BLI_assert(bcoll_index >= 0);

53
source/blender/animrig/intern/bone_collections_test.cc
View File

@ -1426,6 +1426,59 @@ TEST_F(ANIM_armature_bone_collections_testlist, move_before_after_index__to_root
EXPECT_EQ(-1, armature_bonecoll_find_parent_index(&arm, 1));
}
TEST_F(ANIM_armature_bone_collections_testlist, child_number_set__roots)
{
/* Test with only one root. */
EXPECT_EQ(0, armature_bonecoll_child_number_set(&arm, root, 0));
EXPECT_TRUE(expect_bcolls({"root", "child0", "child1", "child2", "child1_0"}));
/* Move to "after the last child", which is the one root itself. */
EXPECT_EQ(0, armature_bonecoll_child_number_set(&arm, root, -1));
EXPECT_TRUE(expect_bcolls({"root", "child0", "child1", "child2", "child1_0"}));
EXPECT_EQ(0, armature_bonecoll_child_number_set(&arm, root, 0));
EXPECT_TRUE(expect_bcolls({"root", "child0", "child1", "child2", "child1_0"}));
/* Going beyond the number of children is not allowed. */
EXPECT_EQ(-1, armature_bonecoll_child_number_set(&arm, root, 1));
EXPECT_TRUE(expect_bcolls({"root", "child0", "child1", "child2", "child1_0"}));
/* Add two roots to be able to play. */
ANIM_armature_bonecoll_new(&arm, "root1");
ANIM_armature_bonecoll_new(&arm, "root2");
EXPECT_TRUE(expect_bcolls({"root", "root1", "root2", "child0", "child1", "child2", "child1_0"}));
/* Move the old root in between the two new ones. */
EXPECT_EQ(1, armature_bonecoll_child_number_set(&arm, root, 1));
EXPECT_TRUE(expect_bcolls({"root1", "root", "root2", "child0", "child1", "child2", "child1_0"}));
/* And to the last one. */
EXPECT_EQ(2, armature_bonecoll_child_number_set(&arm, root, 2));
EXPECT_TRUE(expect_bcolls({"root1", "root2", "root", "child0", "child1", "child2", "child1_0"}));
}
TEST_F(ANIM_armature_bone_collections_testlist, child_number_set__siblings)
{
/* Move child0 to itself. */
EXPECT_EQ(1, armature_bonecoll_child_number_set(&arm, child0, 0));
EXPECT_TRUE(expect_bcolls({"root", "child0", "child1", "child2", "child1_0"}));
/* Move child2 to itself. */
EXPECT_EQ(3, armature_bonecoll_child_number_set(&arm, child2, -1));
EXPECT_TRUE(expect_bcolls({"root", "child0", "child1", "child2", "child1_0"}));
/* Going beyond the number of children is not allowed. */
EXPECT_EQ(-1, armature_bonecoll_child_number_set(&arm, child0, 3));
EXPECT_TRUE(expect_bcolls({"root", "child0", "child1", "child2", "child1_0"}));
/* Move child0 to in between child1 and child2. */
EXPECT_EQ(2, armature_bonecoll_child_number_set(&arm, child0, 1));
EXPECT_TRUE(expect_bcolls({"root", "child1", "child0", "child2", "child1_0"}));
/* Move child0 to the last spot. */
EXPECT_EQ(3, armature_bonecoll_child_number_set(&arm, child0, 2));
EXPECT_TRUE(expect_bcolls({"root", "child1", "child2", "child0", "child1_0"}));
}
class ANIM_armature_bone_collections_liboverrides
: public ANIM_armature_bone_collections_testlist {
protected:

23
source/blender/makesrna/intern/rna_armature.cc
View File

@ -425,6 +425,20 @@ static int rna_BoneCollection_index_get(PointerRNA *ptr)
return blender::animrig::armature_bonecoll_find_index(arm, bcoll);
}
static int rna_BoneCollection_child_number_get(PointerRNA *ptr)
{
bArmature *arm = reinterpret_cast<bArmature *>(ptr->owner_id);
BoneCollection *bcoll = static_cast<BoneCollection *>(ptr->data);
return blender::animrig::armature_bonecoll_child_number_find(arm, bcoll);
}
static void rna_BoneCollection_child_number_set(PointerRNA *ptr, const int new_child_number)
{
bArmature *arm = reinterpret_cast<bArmature *>(ptr->owner_id);
BoneCollection *bcoll = static_cast<BoneCollection *>(ptr->data);
blender::animrig::armature_bonecoll_child_number_set(arm, bcoll, new_child_number);
WM_main_add_notifier(NC_OBJECT | ND_BONE_COLLECTION, nullptr);
}
/* BoneCollection.bones iterator functions. */
static void rna_BoneCollection_bones_begin(CollectionPropertyIterator *iter, PointerRNA *ptr)
@ -2348,6 +2362,15 @@ static void rna_def_bonecollection(BlenderRNA *brna)
"Index of this bone collection in the armature.collections.all array. Note that finding "
"this index requires a scan of all the bone collections, so do access this with care");
prop = RNA_def_property(srna, "child_number", PROP_INT, PROP_NONE);
RNA_def_property_int_funcs(
prop, "rna_BoneCollection_child_number_get", "rna_BoneCollection_child_number_set", nullptr);
RNA_def_property_ui_text(
prop,
"Child Number",
"Index of this collection into its parent's list of children. Note that finding "
"this index requires a scan of all the bone collections, so do access this with care");
RNA_api_bonecollection(srna);
}

72
source/blender/makesrna/intern/rna_armature_api.cc
View File

@ -171,35 +171,6 @@ static bool rna_BoneCollection_unassign(BoneCollection *bcoll,
ANIM_armature_bonecoll_unassign_editbone);
}
static int rna_BoneCollection_move_to_parent(ID *owner_id,
BoneCollection *self,
bContext *C,
ReportList *reports,
BoneCollection *to_parent,
const int to_child_num)
{
using namespace blender::animrig;
bArmature *armature = (bArmature *)owner_id;
const int from_bcoll_index = armature_bonecoll_find_index(armature, self);
const int from_parent_index = armature_bonecoll_find_parent_index(armature, from_bcoll_index);
const int to_parent_index = armature_bonecoll_find_index(armature, to_parent);
if (to_parent_index == from_bcoll_index ||
armature_bonecoll_is_decendent_of(armature, from_bcoll_index, to_parent_index))
{
BKE_report(reports, RPT_ERROR, "Cannot make a bone collection a decendent of itself");
return from_bcoll_index;
}
const int new_bcoll_index = armature_bonecoll_move_to_parent(
armature, from_bcoll_index, to_child_num, from_parent_index, to_parent_index);
WM_event_add_notifier(C, NC_OBJECT | ND_BONE_COLLECTION, nullptr);
return new_bcoll_index;
}
#else
void RNA_api_armature_edit_bone(StructRNA *srna)
@ -349,49 +320,6 @@ void RNA_api_bonecollection(StructRNA *srna)
"Whether the bone was actually removed; will be false if the bone was "
"not a member of the collection to begin with");
RNA_def_function_return(func, parm);
/* collection.move_to_parent(parent, child_num) */
func = RNA_def_function(srna, "move_to_parent", "rna_BoneCollection_move_to_parent");
RNA_def_function_ui_description(
func,
"Change the hierarchy, by moving this bone collection to become a child of a different "
"parent. Use `parent=None` to make this collection a root collection");
RNA_def_function_flag(func, FUNC_USE_SELF_ID | FUNC_USE_CONTEXT | FUNC_USE_REPORTS);
parm = RNA_def_pointer(func,
"to_parent",
"BoneCollection",
"Parent Collection",
"The bone collection becomes a child of this collection. If `None`, the "
"bone collection becomes a root");
RNA_def_parameter_flags(parm, PropertyFlag(0), PARM_REQUIRED);
parm = RNA_def_int(func,
"to_child_num",
-1,
-1,
INT_MAX,
"Child Number",
"Place the bone collection before this child; `child_num=0` makes it the "
"first child, `child_num=1` the second, etc. Both `child_num=-1` and "
"`child_num=child_count` will move the bone collection to be the last child "
"of the new parent",
-1,
INT_MAX);
/* Return value. */
parm = RNA_def_int(func,
"new_index",
-1,
-1,
INT_MAX,
"New Index",
"Resulting index of the bone collection, after the move. This can be used "
"for manipulating the active bone collection index",
-1,
INT_MAX);
RNA_def_function_return(func, parm);
}
#endif

5
tests/python/bl_animation_armature.py
View File

@ -87,7 +87,7 @@ class BoneCollectionTest(unittest.TestCase):
self.assertEqual([root1, root2, r1_child1, r1_child1_001, r2_child1, r2_child2], list(bcolls.all))
# Move root2 to become the child of r1_child1.
self.assertEqual(5, root2.move_to_parent(r1_child1))
root2.parent = r1_child1
# Check the hierarchy.
self.assertEqual([root1], list(bcolls), 'armature.collections should reflect only the roots')
@ -100,7 +100,8 @@ class BoneCollectionTest(unittest.TestCase):
self.assertEqual([root1, r1_child1, r1_child1_001, r2_child1, r2_child2, root2], list(bcolls.all))
# Move root2 between r1_child1 and r1_child1_001.
self.assertEqual(2, root2.move_to_parent(root1, to_child_num=1))
root2.parent = root1
root2.child_num = 1
# Check the hierarchy.
self.assertEqual([root1], list(bcolls), 'armature.collections should reflect only the roots')