Pose Library: remove assumption about Action group names

Remove the assumption of the pose library that Action groups are named
after the bones in the armature. Even though this assumption is correct
when the keys are created by Blender, action groups can be renamed. Keys
created by Python scripts can also use arbitrary group names.

Since there is more code in Blender making this assumption, and looping
over selected bones is also a common occurrence, this commit contains
some generic functionality to aid in this:

- `BKE_armature_find_selected_bones`: function that iterates over all
  bones in an armature and calls a callback for each selected one. It
  returns a struct with info about the selection states (all or no bones
  selected).
- `BKE_armature_find_selected_bone_names(armature)` uses the above
  function to return a set of selected bone names.
- `BKE_pose_find_fcurves_with_bones()` calls a callback for each FCurve
  in an Action that targets a bone, also passing it the bone name.

source/blender/blenkernel/BKE_action.hh

@@ -0,0 +1,35 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+#pragma once
+
+/** \file
+ * \ingroup bke
+ */
+#ifndef __cplusplus
+#  error This is a C++ only header.
+#endif
+
+#include "BLI_function_ref.hh"
+
+struct bAction;
+struct FCurve;
+
+namespace blender::bke {
+
+using FoundFCurveCallback = blender::FunctionRef<void(FCurve *fcurve, const char *bone_name)>;
+void BKE_action_find_fcurves_with_bones(const bAction *action, FoundFCurveCallback callback);
+
+};  // namespace blender::bke

source/blender/blenkernel/BKE_armature.hh

@@ -0,0 +1,48 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+#pragma once
+
+/** \file
+ * \ingroup bke
+ */
+#ifndef __cplusplus
+#  error This is a C++ only header.
+#endif
+
+#include "BKE_armature.h"
+
+#include "BLI_function_ref.hh"
+#include "BLI_set.hh"
+
+namespace blender::bke {
+
+struct SelectedBonesResult {
+  bool all_bones_selected = true;
+  bool no_bones_selected = true;
+};
+
+using SelectedBoneCallback = blender::FunctionRef<void(Bone *bone)>;
+SelectedBonesResult BKE_armature_find_selected_bones(const bArmature *armature,
+                                                     SelectedBoneCallback callback);
+
+using BoneNameSet = blender::Set<std::string>;
+/**
+ * Return a set of names of the selected bones. An empty set means "ignore bone
+ * selection", which either means all bones are selected, or none are.
+ */
+BoneNameSet BKE_armature_find_selected_bone_names(const bArmature *armature);
+
+};  // namespace blender::bke

source/blender/blenkernel/CMakeLists.txt

@@ -69,6 +69,7 @@ set(SRC
   intern/CCGSubSurf_util.c
   intern/DerivedMesh.cc
   intern/action.c
+  intern/action_bones.cc
   intern/action_mirror.c
   intern/addon.c
   intern/anim_data.c
@@ -77,6 +78,7 @@ set(SRC
   intern/anim_visualization.c
   intern/appdir.c
   intern/armature.c
+  intern/armature_selection.cc
   intern/armature_deform.c
   intern/armature_pose.cc
   intern/armature_update.c
@@ -287,6 +289,7 @@ set(SRC
 
   BKE_DerivedMesh.h
   BKE_action.h
+  BKE_action.hh
   BKE_addon.h
   BKE_anim_data.h
   BKE_anim_path.h
@@ -294,6 +297,7 @@ set(SRC
   BKE_animsys.h
   BKE_appdir.h
   BKE_armature.h
+  BKE_armature.hh
   BKE_asset.h
   BKE_attribute.h
   BKE_attribute_access.hh

source/blender/blenkernel/intern/action_bones.cc

@@ -0,0 +1,48 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * The Original Code is Copyright (C) 2001-2002 by NaN Holding BV.
+ * All rights reserved.
+ */
+
+/** \file
+ * \ingroup bke
+ */
+
+#include "BKE_action.hh"
+
+#include "BLI_listbase.h"
+#include "BLI_string.h"
+
+#include "DNA_action_types.h"
+#include "DNA_anim_types.h"
+
+#include "MEM_guardedalloc.h"
+
+namespace blender::bke {
+
+void BKE_action_find_fcurves_with_bones(const bAction *action, FoundFCurveCallback callback)
+{
+  LISTBASE_FOREACH (FCurve *, fcu, &action->curves) {
+    char *bone_name = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
+    if (!bone_name) {
+      continue;
+    }
+    callback(fcu, bone_name);
+    MEM_freeN(bone_name);
+  }
+}
+
+}  // namespace blender::bke

source/blender/blenkernel/intern/armature_pose.cc

@@ -23,8 +23,9 @@
  * \ingroup bke
  */
 
+#include "BKE_action.hh"
 #include "BKE_animsys.h"
-#include "BKE_armature.h"
+#include "BKE_armature.hh"
 
 #include "BLI_function_ref.hh"
 #include "BLI_set.hh"
@@ -36,14 +37,14 @@
 
 #include "RNA_access.h"
 
+using namespace blender::bke;
+
 namespace {
-using BoneNameSet = blender::Set<std::string>;
 
 using ActionApplier =
     blender::FunctionRef<void(PointerRNA *, bAction *, const AnimationEvalContext *)>;
 
 /* Forward declarations. */
-BoneNameSet pose_apply_find_selected_bones(const bArmature *armature, const bPose *pose);
 void pose_apply_disable_fcurves_for_unselected_bones(bAction *action,
                                                      const BoneNameSet &selected_bone_names);
 void pose_apply_restore_fcurves(bAction *action);
@@ -102,7 +103,7 @@ void pose_apply(struct Object *ob,
   }
 
   const bArmature *armature = (bArmature *)ob->data;
-  const BoneNameSet selected_bone_names = pose_apply_find_selected_bones(armature, pose);
+  const BoneNameSet selected_bone_names = BKE_armature_find_selected_bone_names(armature);
   const bool limit_to_selected_bones = !selected_bone_names.is_empty();
 
   if (limit_to_selected_bones) {
@@ -122,30 +123,6 @@ void pose_apply(struct Object *ob,
   }
 }
 
-BoneNameSet pose_apply_find_selected_bones(const bArmature *armature, const bPose *pose)
-{
-  BoneNameSet selected_bone_names;
-  bool all_bones_selected = true;
-  bool no_bones_selected = true;
-
-  LISTBASE_FOREACH (bPoseChannel *, pchan, &pose->chanbase) {
-    const bool is_selected = PBONE_SELECTED(armature, pchan->bone);
-    all_bones_selected &= is_selected;
-    no_bones_selected &= !is_selected;
-
-    if (is_selected) {
-      /* Bone names are unique, so no need to check for duplicates. */
-      selected_bone_names.add_new(pchan->name);
-    }
-  }
-
-  /* If no bones are selected, act as if all are. */
-  if (all_bones_selected || no_bones_selected) {
-    return BoneNameSet(); /* An empty set means "ignore bone selection". */
-  }
-  return selected_bone_names;
-}
-
 void pose_apply_restore_fcurves(bAction *action)
 {
   /* TODO(Sybren): Restore the FCurve flags, instead of just erasing the 'disabled' flag. */
@@ -157,24 +134,13 @@ void pose_apply_restore_fcurves(bAction *action)
 void pose_apply_disable_fcurves_for_unselected_bones(bAction *action,
                                                      const BoneNameSet &selected_bone_names)
 {
-  LISTBASE_FOREACH (FCurve *, fcu, &action->curves) {
-    if (!fcu->rna_path || !strstr(fcu->rna_path, "pose.bones[")) {
-      continue;
+  auto disable_unselected_fcurve = [&](FCurve *fcu, const char *bone_name) {
+    const bool is_bone_selected = selected_bone_names.contains(bone_name);
+    if (!is_bone_selected) {
+      fcu->flag |= FCURVE_DISABLED;
     }
-
-    /* Get bone name, and check if this bone is selected. */
-    char *bone_name = BLI_str_quoted_substrN(fcu->rna_path, "pose.bones[");
-    if (!bone_name) {
-      continue;
-    }
-    const bool is_selected = selected_bone_names.contains(bone_name);
-    MEM_freeN(bone_name);
-    if (is_selected) {
-      continue;
-    }
-
-    fcu->flag |= FCURVE_DISABLED;
-  }
+  };
+  BKE_action_find_fcurves_with_bones(action, disable_unselected_fcurve);
 }
 
 }  // namespace

source/blender/blenkernel/intern/armature_selection.cc

@@ -0,0 +1,78 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+/** \file
+ * \ingroup bke
+ */
+
+#include "BKE_armature.hh"
+
+#include "BLI_listbase.h"
+
+#include "DNA_action_types.h"
+#include "DNA_armature_types.h"
+
+namespace blender::bke {
+
+namespace {
+
+void find_selected_bones__visit_bone(const bArmature *armature,
+                                     SelectedBoneCallback callback,
+                                     SelectedBonesResult &result,
+                                     Bone *bone)
+{
+  const bool is_selected = PBONE_SELECTED(armature, bone);
+  result.all_bones_selected &= is_selected;
+  result.no_bones_selected &= !is_selected;
+
+  if (is_selected) {
+    callback(bone);
+  }
+
+  LISTBASE_FOREACH (Bone *, child_bone, &bone->childbase) {
+    find_selected_bones__visit_bone(armature, callback, result, child_bone);
+  }
+}
+}  // namespace
+
+SelectedBonesResult BKE_armature_find_selected_bones(const bArmature *armature,
+                                                     SelectedBoneCallback callback)
+{
+  SelectedBonesResult result;
+  LISTBASE_FOREACH (Bone *, root_bone, &armature->bonebase) {
+    find_selected_bones__visit_bone(armature, callback, result, root_bone);
+  }
+
+  return result;
+}
+
+BoneNameSet BKE_armature_find_selected_bone_names(const bArmature *armature)
+{
+  BoneNameSet selected_bone_names;
+
+  /* Iterate over the selected bones to fill the set of bone names. */
+  auto callback = [&](Bone *bone) { selected_bone_names.add(bone->name); };
+  SelectedBonesResult result = BKE_armature_find_selected_bones(armature, callback);
+
+  /* If no bones are selected, act as if all are. */
+  if (result.all_bones_selected || result.no_bones_selected) {
+    return BoneNameSet();
+  }
+
+  return selected_bone_names;
+}
+
+}  // namespace blender::bke

source/blender/blenkernel/intern/armature_test.cc

@@ -17,10 +17,13 @@
  * All rights reserved.
  */
 
-#include "BKE_armature.h"
+#include "BKE_armature.hh"
 
+#include "BLI_listbase.h"
 #include "BLI_math.h"
 
+#include "DNA_armature_types.h"
+
 #include "testing/testing.h"
 
 namespace blender::bke::tests {
@@ -157,4 +160,76 @@ TEST(vec_roll_to_mat3_normalized, Rotationmatrix)
   }
 }
 
+class BKE_armature_find_selected_bones_test : public testing::Test {
+ protected:
+  bArmature arm;
+  Bone bone1, bone2, bone3;
+
+  void SetUp() override
+  {
+    strcpy(bone1.name, "bone1");
+    strcpy(bone2.name, "bone2");
+    strcpy(bone3.name, "bone3");
+
+    arm.bonebase = {NULL, NULL};
+    BLI_addtail(&arm.bonebase, &bone1);
+    BLI_addtail(&arm.bonebase, &bone2);
+    BLI_addtail(&arm.bonebase, &bone3);
+
+    // Make sure the armature & its bones are visible, to make them selectable.
+    arm.layer = bone1.layer = bone2.layer = bone3.layer = 1;
+  }
+};
+
+TEST_F(BKE_armature_find_selected_bones_test, some_bones_selected)
+{
+  bone1.flag = BONE_SELECTED;
+  bone2.flag = 0;
+  bone3.flag = BONE_SELECTED;
+
+  std::vector<Bone *> seen_bones;
+  auto callback = [&](Bone *bone) { seen_bones.push_back(bone); };
+
+  SelectedBonesResult result = BKE_armature_find_selected_bones(&arm, callback);
+
+  ASSERT_EQ(seen_bones.size(), 2) << "Expected 2 selected bones, got " << seen_bones.size();
+  EXPECT_EQ(seen_bones[0], &bone1);
+  EXPECT_EQ(seen_bones[1], &bone3);
+
+  EXPECT_FALSE(result.all_bones_selected);  // Bone 2 was not selected.
+  EXPECT_FALSE(result.no_bones_selected);   // Bones 1 and 3 were selected.
+}
+
+TEST_F(BKE_armature_find_selected_bones_test, no_bones_selected)
+{
+  bone1.flag = bone2.flag = bone3.flag = 0;
+
+  std::vector<Bone *> seen_bones;
+  auto callback = [&](Bone *bone) { seen_bones.push_back(bone); };
+
+  SelectedBonesResult result = BKE_armature_find_selected_bones(&arm, callback);
+
+  EXPECT_TRUE(seen_bones.empty()) << "Expected no selected bones, got " << seen_bones.size();
+  EXPECT_FALSE(result.all_bones_selected);
+  EXPECT_TRUE(result.no_bones_selected);
+}
+
+TEST_F(BKE_armature_find_selected_bones_test, all_bones_selected)
+{
+  bone1.flag = bone2.flag = bone3.flag = BONE_SELECTED;
+
+  std::vector<Bone *> seen_bones;
+  auto callback = [&](Bone *bone) { seen_bones.push_back(bone); };
+
+  SelectedBonesResult result = BKE_armature_find_selected_bones(&arm, callback);
+
+  ASSERT_EQ(seen_bones.size(), 3) << "Expected 3 selected bones, got " << seen_bones.size();
+  EXPECT_EQ(seen_bones[0], &bone1);
+  EXPECT_EQ(seen_bones[1], &bone2);
+  EXPECT_EQ(seen_bones[2], &bone3);
+
+  EXPECT_TRUE(result.all_bones_selected);
+  EXPECT_FALSE(result.no_bones_selected);
+}
+
 }  // namespace blender::bke::tests

source/blender/editors/armature/pose_backup.cc

@@ -31,9 +31,12 @@
 #include "DNA_object_types.h"
 
 #include "BKE_action.h"
-#include "BKE_armature.h"
+#include "BKE_action.hh"
+#include "BKE_armature.hh"
 #include "BKE_idprop.h"
 
+using namespace blender::bke;
+
 /* simple struct for storing backup info for one pose channel */
 typedef struct PoseChannelBackup {
   struct PoseChannelBackup *next, *prev;
@@ -51,21 +54,27 @@ struct PoseBackup {
 
 static PoseBackup *pose_backup_create(const Object *ob,
                                       const bAction *action,
-                                      const bool is_bone_selection_relevant)
+                                      const BoneNameSet &selected_bone_names)
 {
   ListBase backups = {nullptr, nullptr};
-  const bArmature *armature = static_cast<const bArmature *>(ob->data);
+  const bool is_bone_selection_relevant = !selected_bone_names.is_empty();
 
-  /* TODO(Sybren): reuse same approach as in `armature_pose.cc` in this function, as that doesn't
-   * have the assumption that action group names are bone names. */
-  LISTBASE_FOREACH (bActionGroup *, agrp, &action->groups) {
-    bPoseChannel *pchan = BKE_pose_channel_find_name(ob->pose, agrp->name);
-    if (pchan == nullptr) {
-      continue;
+  BoneNameSet backed_up_bone_names;
+  /* Make a backup of the given pose channel. */
+  auto store_animated_pchans = [&](FCurve *, const char *bone_name) {
+    if (backed_up_bone_names.contains(bone_name)) {
+      /* Only backup each bone once. */
+      return;
     }
 
-    if (is_bone_selection_relevant && !PBONE_SELECTED(armature, pchan->bone)) {
-      continue;
+    bPoseChannel *pchan = BKE_pose_channel_find_name(ob->pose, bone_name);
+    if (pchan == nullptr) {
+      /* FCurve targets non-existent bone. */
+      return;
+    }
+
+    if (is_bone_selection_relevant && !selected_bone_names.contains(bone_name)) {
+      return;
     }
 
     PoseChannelBackup *chan_bak = static_cast<PoseChannelBackup *>(
@@ -78,7 +87,11 @@ static PoseBackup *pose_backup_create(const Object *ob,
     }
 
     BLI_addtail(&backups, chan_bak);
-  }
+    backed_up_bone_names.add_new(bone_name);
+  };
+
+  /* Call `store_animated_pchans()` for each FCurve that targets a bone. */
+  BKE_action_find_fcurves_with_bones(action, store_animated_pchans);
 
   /* PoseBackup is constructed late, so that the above loop can use stack variables. */
   PoseBackup *pose_backup = static_cast<PoseBackup *>(MEM_callocN(sizeof(*pose_backup), __func__));
@@ -89,24 +102,14 @@ static PoseBackup *pose_backup_create(const Object *ob,
 
 PoseBackup *ED_pose_backup_create_all_bones(const Object *ob, const bAction *action)
 {
-  return pose_backup_create(ob, action, false);
+  return pose_backup_create(ob, action, BoneNameSet());
 }
 
 PoseBackup *ED_pose_backup_create_selected_bones(const Object *ob, const bAction *action)
 {
-  /* See if bone selection is relevant. */
-  bool all_bones_selected = true;
-  bool no_bones_selected = true;
   const bArmature *armature = static_cast<const bArmature *>(ob->data);
-  LISTBASE_FOREACH (bPoseChannel *, pchan, &ob->pose->chanbase) {
-    const bool is_selected = PBONE_SELECTED(armature, pchan->bone);
-    all_bones_selected &= is_selected;
-    no_bones_selected &= !is_selected;
-  }
-
-  /* If no bones are selected, act as if all are. */
-  const bool is_bone_selection_relevant = !all_bones_selected && !no_bones_selected;
-  return pose_backup_create(ob, action, is_bone_selection_relevant);
+  const BoneNameSet selected_bone_names = BKE_armature_find_selected_bone_names(armature);
+  return pose_backup_create(ob, action, selected_bone_names);
 }
 
 bool ED_pose_backup_is_selection_relevant(const struct PoseBackup *pose_backup)