griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Cleanup: Anim: rename *_move() methods to *_move_to_index()

Browse Source 
There were three methods with succinct but slightly unclear names:

- Action::slot_move()
- Channelbag::channel_group_move()
- Channelbag::fcurve_move()

The ambiguity is due to other functions with similar names that e.g.
move fcurves *between* Channelbags, etc. Whereas these methods only move
items to other positions within the array they're already in.

To make this clearer, this PR adds `_to_index` to the end of these
array-oriented methods.

Pull Request: https://projects.blender.org/blender/blender/pulls/133730
This commit is contained in:
Nathan Vegdahl
2025-01-30 10:24:47 +01:00
committed by Nathan Vegdahl
parent 4c7129be72
commit 77f4fd2637
4 changed files with 39 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
source/blender/animrig/ANIM_action.hh
View File

@@ -325,7 +325,7 @@ class Action : public ::bAction {
* `slot` must belong to this action, and `to_slot_index` must be a
* valid index in the slot array.
*/
void slot_move(Slot &slot, int to_slot_index);
void slot_move_to_index(Slot &slot, int to_slot_index);
/**
* Set the active Slot, ensuring only one Slot is flagged as the Active one.
@@ -1131,7 +1131,7 @@ class Channelbag : public ::ActionChannelbag {
* `fcurve` must belong to this channel bag, and `to_fcurve_index` must be a
* valid index in the fcurve array.
*/
void fcurve_move(FCurve &fcurve, int to_fcurve_index);
void fcurve_move_to_index(FCurve &fcurve, int to_fcurve_index);
/**
* Remove all F-Curves from this Channelbag.
@@ -1211,7 +1211,7 @@ class Channelbag : public ::ActionChannelbag {
* `group` must belong to this channel bag, and `to_group_index` must be a
* valid index in the channel group array.
*/
void channel_group_move(bActionGroup &group, int to_group_index);
void channel_group_move_to_index(bActionGroup &group, int to_group_index);
/**
* Assigns the given FCurve to the given channel group.

6
source/blender/animrig/intern/action.cc
View File

@@ -586,7 +586,7 @@ bool Action::slot_remove(Slot &slot_to_remove)
return true;
}
void Action::slot_move(Slot &slot, const int to_slot_index)
void Action::slot_move_to_index(Slot &slot, const int to_slot_index)
{
BLI_assert(this->slots().index_range().contains(to_slot_index));
@@ -1936,7 +1936,7 @@ void Channelbag::fcurve_detach_by_index(const int64_t fcurve_index)
* depsgraph evaluation results though. */
}
void Channelbag::fcurve_move(FCurve &fcurve, int to_fcurve_index)
void Channelbag::fcurve_move_to_index(FCurve &fcurve, int to_fcurve_index)
{
BLI_assert(to_fcurve_index >= 0 && to_fcurve_index < this->fcurves().size());
@@ -2288,7 +2288,7 @@ bool Channelbag::channel_group_remove(bActionGroup &group)
return true;
}
void Channelbag::channel_group_move(bActionGroup &group, const int to_group_index)
void Channelbag::channel_group_move_to_index(bActionGroup &group, const int to_group_index)
{
BLI_assert(to_group_index >= 0 && to_group_index < this->channel_groups().size());

30
source/blender/animrig/intern/action_test.cc
View File

@@ -448,7 +448,7 @@ TEST_F(ActionLayersTest, slot_remove)
}
}
TEST_F(ActionLayersTest, slot_move)
TEST_F(ActionLayersTest, slot_move_to_index)
{
Slot &slot_a = action->slot_add_for_id_type(ID_ME);
Slot &slot_b = action->slot_add_for_id_type(ID_CA);
@@ -475,7 +475,7 @@ TEST_F(ActionLayersTest, slot_move)
ASSERT_EQ(action->slot(3)->users(*bmain)[0], &suzanne->id);
/* First "move" a slot to its own location, which should do nothing. */
action->slot_move(slot_b, 1);
action->slot_move_to_index(slot_b, 1);
EXPECT_EQ(action->slot(0)->handle, handle_a);
EXPECT_EQ(action->slot(0)->identifier_prefix_for_idtype(), "ME");
EXPECT_EQ(action->slot(1)->handle, handle_b);
@@ -489,7 +489,7 @@ TEST_F(ActionLayersTest, slot_move)
/* Then move slots around in various ways. */
action->slot_move(slot_a, 2);
action->slot_move_to_index(slot_a, 2);
EXPECT_EQ(action->slot(0)->handle, handle_b);
EXPECT_EQ(action->slot(0)->identifier_prefix_for_idtype(), "CA");
EXPECT_EQ(action->slot(1)->handle, handle_cube);
@@ -501,7 +501,7 @@ TEST_F(ActionLayersTest, slot_move)
EXPECT_EQ(action->slot(3)->identifier_prefix_for_idtype(), "OB");
EXPECT_EQ(action->slot(3)->users(*bmain)[0], &suzanne->id);
action->slot_move(slot_suzanne, 1);
action->slot_move_to_index(slot_suzanne, 1);
EXPECT_EQ(action->slot(0)->handle, handle_b);
EXPECT_EQ(action->slot(0)->identifier_prefix_for_idtype(), "CA");
EXPECT_EQ(action->slot(1)->handle, handle_suzanne);
@@ -513,7 +513,7 @@ TEST_F(ActionLayersTest, slot_move)
EXPECT_EQ(action->slot(3)->handle, handle_a);
EXPECT_EQ(action->slot(3)->identifier_prefix_for_idtype(), "ME");
action->slot_move(slot_cube, 3);
action->slot_move_to_index(slot_cube, 3);
EXPECT_EQ(action->slot(0)->handle, handle_b);
EXPECT_EQ(action->slot(0)->identifier_prefix_for_idtype(), "CA");
EXPECT_EQ(action->slot(1)->handle, handle_suzanne);
@@ -525,7 +525,7 @@ TEST_F(ActionLayersTest, slot_move)
EXPECT_EQ(action->slot(3)->identifier_prefix_for_idtype(), "OB");
EXPECT_EQ(action->slot(3)->users(*bmain)[0], &cube->id);
action->slot_move(slot_suzanne, 0);
action->slot_move_to_index(slot_suzanne, 0);
EXPECT_EQ(action->slot(0)->handle, handle_suzanne);
EXPECT_EQ(action->slot(0)->identifier_prefix_for_idtype(), "OB");
EXPECT_EQ(action->slot(0)->users(*bmain)[0], &suzanne->id);
@@ -1391,7 +1391,7 @@ class ChannelbagTest : public testing::Test {
}
};
TEST_F(ChannelbagTest, fcurve_move)
TEST_F(ChannelbagTest, fcurve_move_to_index)
{
FCurve &fcu0 = channelbag->fcurve_ensure(nullptr, {"fcu0", 0, std::nullopt, "group0"});
FCurve &fcu1 = channelbag->fcurve_ensure(nullptr, {"fcu1", 0, std::nullopt, "group0"});
@@ -1406,7 +1406,7 @@ TEST_F(ChannelbagTest, fcurve_move)
bActionGroup &group1 = *channelbag->channel_group(1);
/* Moving an fcurve to where it already is should be fine. */
channelbag->fcurve_move(fcu0, 0);
channelbag->fcurve_move_to_index(fcu0, 0);
EXPECT_EQ(&fcu0, channelbag->fcurve(0));
EXPECT_EQ(&fcu1, channelbag->fcurve(1));
EXPECT_EQ(&fcu2, channelbag->fcurve(2));
@@ -1419,7 +1419,7 @@ TEST_F(ChannelbagTest, fcurve_move)
EXPECT_EQ(nullptr, fcu4.grp);
/* Move to first. */
channelbag->fcurve_move(fcu4, 0);
channelbag->fcurve_move_to_index(fcu4, 0);
EXPECT_EQ(0, group0.fcurve_range_start);
EXPECT_EQ(2, group0.fcurve_range_length);
EXPECT_EQ(2, group1.fcurve_range_start);
@@ -1436,7 +1436,7 @@ TEST_F(ChannelbagTest, fcurve_move)
EXPECT_EQ(nullptr, fcu3.grp);
/* Move to last. */
channelbag->fcurve_move(fcu1, 4);
channelbag->fcurve_move_to_index(fcu1, 4);
EXPECT_EQ(0, group0.fcurve_range_start);
EXPECT_EQ(2, group0.fcurve_range_length);
EXPECT_EQ(2, group1.fcurve_range_start);
@@ -1453,7 +1453,7 @@ TEST_F(ChannelbagTest, fcurve_move)
EXPECT_EQ(nullptr, fcu1.grp);
/* Move to middle. */
channelbag->fcurve_move(fcu4, 2);
channelbag->fcurve_move_to_index(fcu4, 2);
EXPECT_EQ(0, group0.fcurve_range_start);
EXPECT_EQ(2, group0.fcurve_range_length);
EXPECT_EQ(2, group1.fcurve_range_start);
@@ -1789,7 +1789,7 @@ TEST_F(ChannelbagTest, channel_group_fcurve_removal)
ASSERT_EQ(0, channelbag->channel_groups().size());
}
TEST_F(ChannelbagTest, channel_group_move)
TEST_F(ChannelbagTest, channel_group_move_to_index)
{
FCurve &fcu0 = channelbag->fcurve_ensure(nullptr, {"fcu0", 0, std::nullopt, "group0"});
FCurve &fcu1 = channelbag->fcurve_ensure(nullptr, {"fcu1", 0, std::nullopt, "group1"});
@@ -1804,7 +1804,7 @@ TEST_F(ChannelbagTest, channel_group_move)
bActionGroup &group1 = *channelbag->channel_group(1);
bActionGroup &group2 = *channelbag->channel_group(2);
channelbag->channel_group_move(group0, 2);
channelbag->channel_group_move_to_index(group0, 2);
EXPECT_EQ(&group1, channelbag->channel_group(0));
EXPECT_EQ(&group2, channelbag->channel_group(1));
EXPECT_EQ(&group0, channelbag->channel_group(2));
@@ -1825,7 +1825,7 @@ TEST_F(ChannelbagTest, channel_group_move)
EXPECT_EQ(&group0, fcu0.grp);
EXPECT_EQ(nullptr, fcu4.grp);
channelbag->channel_group_move(group1, 1);
channelbag->channel_group_move_to_index(group1, 1);
EXPECT_EQ(&group2, channelbag->channel_group(0));
EXPECT_EQ(&group1, channelbag->channel_group(1));
EXPECT_EQ(&group0, channelbag->channel_group(2));
@@ -1846,7 +1846,7 @@ TEST_F(ChannelbagTest, channel_group_move)
EXPECT_EQ(&group0, fcu0.grp);
EXPECT_EQ(nullptr, fcu4.grp);
channelbag->channel_group_move(group0, 0);
channelbag->channel_group_move_to_index(group0, 0);
EXPECT_EQ(&group0, channelbag->channel_group(0));
EXPECT_EQ(&group2, channelbag->channel_group(1));
EXPECT_EQ(&group1, channelbag->channel_group(2));

35
source/blender/editors/animation/anim_channels_edit.cc
View File

@@ -1724,7 +1724,7 @@ static bool rearrange_layered_action_slots(bAnimContext *ac, const eRearrangeAni
continue;
}
action.slot_move(slot, to_index);
action.slot_move_to_index(slot, to_index);
total_moved++;
}
break;
@@ -1740,7 +1740,7 @@ static bool rearrange_layered_action_slots(bAnimContext *ac, const eRearrangeAni
const int current_index = action.slots().first_index_try(&slot);
const int to_index = 0;
if (current_index != to_index) {
action.slot_move(slot, to_index);
action.slot_move_to_index(slot, to_index);
total_moved++;
}
}
@@ -1766,7 +1766,7 @@ static bool rearrange_layered_action_slots(bAnimContext *ac, const eRearrangeAni
continue;
}
action.slot_move(slot, to_index);
action.slot_move_to_index(slot, to_index);
total_moved++;
}
break;
@@ -1782,7 +1782,7 @@ static bool rearrange_layered_action_slots(bAnimContext *ac, const eRearrangeAni
const int current_index = action.slots().first_index_try(&slot);
const int to_index = action.slots().size() - 1;
if (current_index != to_index) {
action.slot_move(slot, to_index);
action.slot_move_to_index(slot, to_index);
total_moved++;
}
}
@@ -1801,8 +1801,8 @@ static bool rearrange_layered_action_slots(bAnimContext *ac, const eRearrangeAni
*
* NOTE: the current implementation has quadratic performance with respect to
* the number of groups in a `Channelbag`, due to both `Span::first_index_try()`
* and `Channelbag::channel_group_move()` having linear performance. If this
* becomes a performance bottleneck in practice, we can create a dedicated
* and `Channelbag::channel_group_move_to_index()` having linear performance. If
* this becomes a performance bottleneck in practice, we can create a dedicated
* method on `Channelbag` for collectively moving a non-contiguous set of
* channel groups that works in linear time.
*
@@ -1846,7 +1846,7 @@ static void rearrange_layered_action_channel_groups(bAnimContext *ac,
continue;
}
bag.channel_group_move(*group, to_index);
bag.channel_group_move_to_index(*group, to_index);
}
break;
}
@@ -1859,7 +1859,7 @@ static void rearrange_layered_action_channel_groups(bAnimContext *ac,
continue;
}
blender::animrig::Channelbag &bag = group->channelbag->wrap();
bag.channel_group_move(*group, 0);
bag.channel_group_move_to_index(*group, 0);
}
break;
}
@@ -1884,7 +1884,7 @@ static void rearrange_layered_action_channel_groups(bAnimContext *ac,
continue;
}
bag.channel_group_move(*group, to_index);
bag.channel_group_move_to_index(*group, to_index);
}
break;
}
@@ -1897,7 +1897,7 @@ static void rearrange_layered_action_channel_groups(bAnimContext *ac,
continue;
}
blender::animrig::Channelbag &bag = group->channelbag->wrap();
bag.channel_group_move(*group, bag.channel_groups().size() - 1);
bag.channel_group_move_to_index(*group, bag.channel_groups().size() - 1);
}
break;
}
@@ -1911,9 +1911,9 @@ static void rearrange_layered_action_channel_groups(bAnimContext *ac,
*
* NOTE: the current implementation has quadratic performance with respect to
* the number of fcurves in a `Channelbag`, due to both
* `Span::first_index_try()` and `Channelbag::fcurve_move()` having linear
* performance. If this becomes a performance bottleneck in practice, we can
* create a dedicated method on `Channelbag` for collectively moving a
* `Span::first_index_try()` and `Channelbag::fcurve_move_to_index()` having
* linear performance. If this becomes a performance bottleneck in practice, we
* can create a dedicated method on `Channelbag` for collectively moving a
* non-contiguous set of fcurves that works in linear time.
*
* TODO: there's a fair amount of apparent repetition in this code and the code
@@ -1997,7 +1997,7 @@ static void rearrange_layered_action_fcurves(bAnimContext *ac,
continue;
}
bag.fcurve_move(*fcurve, to_index);
bag.fcurve_move_to_index(*fcurve, to_index);
}
return;
}
@@ -2013,7 +2013,7 @@ static void rearrange_layered_action_fcurves(bAnimContext *ac,
}
blender::animrig::Channelbag &bag = group.channelbag->wrap();
bag.fcurve_move(*fcurve, group.fcurve_range_start);
bag.fcurve_move_to_index(*fcurve, group.fcurve_range_start);
}
return;
}
@@ -2042,7 +2042,7 @@ static void rearrange_layered_action_fcurves(bAnimContext *ac,
continue;
}
bag.fcurve_move(*fcurve, to_index);
bag.fcurve_move_to_index(*fcurve, to_index);
}
return;
}
@@ -2058,7 +2058,8 @@ static void rearrange_layered_action_fcurves(bAnimContext *ac,
}
blender::animrig::Channelbag &bag = group.channelbag->wrap();
bag.fcurve_move(*fcurve, group.fcurve_range_start + group.fcurve_range_length - 1);
bag.fcurve_move_to_index(*fcurve,
group.fcurve_range_start + group.fcurve_range_length - 1);
}
return;
}