Fix #134746: Action Slot keyframe summary incorrect in Action Editor and Dopesheet

In the Action Editor and Dopesheet, Action/Slot summaries erroneously showed
keys that were hidden in their sub-channels.

The root cause was that the functions for building Action/Slot summary key
lists, `action_to_keylist()` and `action_slot_to_keylist()`, weren't doing
animation filtering *at all*, instead just looping over all f-curves.

This PR fixes the bug by replacing `action_slot_to_keylist()` with a new
`action_slot_summary_to_keylist()` function, which calls into the animation
filtering code to determine which f-curves to include in their key list, and
also using it in place of `action_to_keylist()` at the relevant call sites.
Additionally, properly filtering the channels for the summaries requires both a
`bAnimContext` and the ID that's being animated, so this PR threads those
through to `action_slot_summary_to_keylist()`.

Note that `action_to_keylist()` is still used in some places that actually
*want* an unfiltered list of keys. Those call sites have been left as-is, and
`action_to_keylist()` remains for that purpose.

Pull Request: https://projects.blender.org/blender/blender/pulls/134922

source/blender/editors/animation/anim_filter.cc

@@ -1550,16 +1550,37 @@ static size_t animfilter_act_group(bAnimContext *ac,
   return items;
 }
 
-/**
- * Add a channel for each Slot, with their FCurves when the Slot is expanded.
- */
-static size_t animfilter_action_slot(bAnimContext *ac,
-                                     ListBase *anim_data,
-                                     animrig::Action &action,
-                                     animrig::Slot &slot,
-                                     const eAnimFilter_Flags filter_mode,
-                                     ID *animated_id)
+size_t ANIM_animfilter_action_slot(bAnimContext *ac,
+                                   ListBase * /* bAnimListElem */ anim_data,
+                                   animrig::Action &action,
+                                   animrig::Slot &slot,
+                                   const eAnimFilter_Flags filter_mode,
+                                   ID *animated_id)
 {
+  BLI_assert(ac);
+
+  /* In some cases (see `ob_to_keylist()` and friends) fake bDopeSheet and fake bAnimContext are
+   * created. These are mostly null-initialized, and so do not have a bmain. This means that
+   * lookup of the animated ID is not possible, which can result in failure to look up the proper
+   * F-Curve display name. For the `..._to_keylist` functions that doesn't matter, as those are
+   * only interested in the key data anyway. So rather than trying to get a reliable `bmain`
+   * through the maze, this code just treats it as optional (even though ideally it should always
+   * be known). */
+  ID *slot_user_id = nullptr;
+  if (ac->bmain) {
+    slot_user_id = animrig::action_slot_get_id_best_guess(*ac->bmain, slot, animated_id);
+  }
+  if (!slot_user_id) {
+    BLI_assert(animated_id);
+    /* At the time of writing this (PR #134922), downstream code (see e.g.
+     * `animfilter_fcurves_span()`) assumes this is non-null, so we need to set
+     * it to *something*. If it's not an actual user of the slot then channels
+     * might not resolve to an actual property and thus be displayed oddly in
+     * the channel list, but that's not technically a problem, it's just a
+     * little strange for the end user. */
+    slot_user_id = animated_id;
+  }
+
   /* Don't include anything from this animation if it is linked in from another
    * file, and we're getting stuff for editing... */
   if ((filter_mode & ANIMFILTER_FOREDIT) &&
@@ -1584,7 +1605,7 @@ static size_t animfilter_action_slot(bAnimContext *ac,
   const bool show_slot_channel = (is_action_mode && selection_ok_for_slot &&
                                   include_summary_channels);
   if (show_slot_channel) {
-    ANIMCHANNEL_NEW_CHANNEL(ac->bmain, &slot, ANIMTYPE_ACTION_SLOT, animated_id, &action.id);
+    ANIMCHANNEL_NEW_CHANNEL(ac->bmain, &slot, ANIMTYPE_ACTION_SLOT, slot_user_id, &action.id);
     items++;
   }
 
@@ -1605,7 +1626,7 @@ static size_t animfilter_action_slot(bAnimContext *ac,
   /* Add channel groups and their member channels. */
   for (bActionGroup *group : channelbag->channel_groups()) {
     items += animfilter_act_group(
-        ac, anim_data, &action, slot.handle, group, filter_mode, animated_id);
+        ac, anim_data, &action, slot.handle, group, filter_mode, slot_user_id);
   }
 
   /* Add ungrouped channels. */
@@ -1619,7 +1640,7 @@ static size_t animfilter_action_slot(bAnimContext *ac,
 
     Span<FCurve *> fcurves = channelbag->fcurves().drop_front(first_ungrouped_fcurve_index);
     items += animfilter_fcurves_span(
-        ac, anim_data, fcurves, slot.handle, filter_mode, animated_id, &action.id);
+        ac, anim_data, fcurves, slot.handle, filter_mode, slot_user_id, &action.id);
   }
 
   return items;
@@ -1643,22 +1664,7 @@ static size_t animfilter_action_slots(bAnimContext *ac,
   for (animrig::Slot *slot : action.slots()) {
     BLI_assert(slot);
 
-    /* In some cases (see `ob_to_keylist()` and friends) fake bDopeSheet and fake bAnimContext are
-     * created. These are mostly null-initialized, and so do not have a bmain. This means that
-     * lookup of the animated ID is not possible, which can result in failure to look up the proper
-     * F-Curve display name. For the `..._to_keylist` functions that doesn't matter, as those are
-     * only interested in the key data anyway. So rather than trying to get a reliable `bmain`
-     * through the maze, this code just treats it as optional (even though ideally it should always
-     * be known). */
-    ID *animated_id = nullptr;
-    if (ac->bmain) {
-      animated_id = animrig::action_slot_get_id_best_guess(*ac->bmain, *slot, owner_id);
-    }
-    if (!animated_id) {
-      /* This is not necessarily correct, but at least it prevents nullptr dereference. */
-      animated_id = owner_id;
-    }
-    num_items += animfilter_action_slot(ac, anim_data, action, *slot, filter_mode, animated_id);
+    num_items += ANIM_animfilter_action_slot(ac, anim_data, action, *slot, filter_mode, owner_id);
   }
 
   return num_items;
@@ -1726,7 +1732,7 @@ static size_t animfilter_action(bAnimContext *ac,
     /* Can happen when an Action is assigned, but not a Slot. */
     return 0;
   }
-  return animfilter_action_slot(ac, anim_data, action, *slot, filter_mode, owner_id);
+  return ANIM_animfilter_action_slot(ac, anim_data, action, *slot, filter_mode, owner_id);
 }
 
 /* Include NLA-Data for NLA-Editor:

source/blender/editors/animation/keyframes_draw.cc

@@ -420,6 +420,7 @@ struct ChannelListElement {
   bDopeSheet *ads;
   Scene *sce;
   Object *ob;
+  ID *animated_id; /* The ID that adt (below) belongs to. */
   AnimData *adt;
   FCurve *fcu;
   bAction *act;
@@ -453,18 +454,36 @@ static void build_channel_keylist(ChannelListElement *elem, blender::float2 rang
       break;
     }
     case ChannelType::ACTION_LAYERED: {
-      action_to_keylist(elem->adt, elem->act, elem->keylist, elem->saction_flag, range);
+      /* This is only called for action summaries in the Dopesheet, *not* the
+       * Action Editor. Therefore despite the name `ACTION_LAYERED`, this is
+       * only used to show a *single slot* of the action: the slot used by the
+       * ID the action is listed under.
+       *
+       * Thus we use the same function as the `ChannelType::ACTION_SLOT` case
+       * below because in practice the only distinction between these cases is
+       * where they get the slot from. In this case, we get it from `elem`'s
+       * ADT. */
+      BLI_assert(elem->act);
+      BLI_assert(elem->adt);
+      action_slot_summary_to_keylist(elem->ac,
+                                     elem->animated_id,
+                                     elem->act->wrap(),
+                                     elem->adt->slot_handle,
+                                     elem->keylist,
+                                     elem->saction_flag,
+                                     range);
       break;
     }
     case ChannelType::ACTION_SLOT: {
       BLI_assert(elem->act);
       BLI_assert(elem->action_slot);
-      action_slot_to_keylist(elem->adt,
-                             elem->act->wrap(),
-                             elem->action_slot->handle,
-                             elem->keylist,
-                             elem->saction_flag,
-                             range);
+      action_slot_summary_to_keylist(elem->ac,
+                                     elem->animated_id,
+                                     elem->act->wrap(),
+                                     elem->action_slot->handle,
+                                     elem->keylist,
+                                     elem->saction_flag,
+                                     range);
       break;
     }
     case ChannelType::ACTION_LEGACY: {
@@ -716,6 +735,7 @@ void ED_add_fcurve_channel(ChannelDrawList *channel_list,
 
   ChannelListElement *draw_elem = channel_list_add_element(
       channel_list, ChannelType::FCURVE, ypos, yscale_fac, eSAction_Flag(saction_flag));
+  draw_elem->animated_id = ale->id;
   draw_elem->adt = ale->adt;
   draw_elem->fcu = fcu;
   draw_elem->channel_locked = locked;
@@ -735,12 +755,14 @@ void ED_add_action_group_channel(ChannelDrawList *channel_list,
 
   ChannelListElement *draw_elem = channel_list_add_element(
       channel_list, ChannelType::ACTION_GROUP, ypos, yscale_fac, eSAction_Flag(saction_flag));
+  draw_elem->animated_id = ale->id;
   draw_elem->adt = ale->adt;
   draw_elem->agrp = agrp;
   draw_elem->channel_locked = locked;
 }
 
 void ED_add_action_layered_channel(ChannelDrawList *channel_list,
+                                   bAnimContext *ac,
                                    bAnimListElem *ale,
                                    bAction *action,
                                    const float ypos,
@@ -755,12 +777,15 @@ void ED_add_action_layered_channel(ChannelDrawList *channel_list,
 
   ChannelListElement *draw_elem = channel_list_add_element(
       channel_list, ChannelType::ACTION_LAYERED, ypos, yscale_fac, eSAction_Flag(saction_flag));
+  draw_elem->ac = ac;
+  draw_elem->animated_id = ale->id;
   draw_elem->adt = ale->adt;
   draw_elem->act = action;
   draw_elem->channel_locked = locked;
 }
 
 void ED_add_action_slot_channel(ChannelDrawList *channel_list,
+                                bAnimContext *ac,
                                 bAnimListElem *ale,
                                 animrig::Action &action,
                                 animrig::Slot &slot,
@@ -773,6 +798,8 @@ void ED_add_action_slot_channel(ChannelDrawList *channel_list,
 
   ChannelListElement *draw_elem = channel_list_add_element(
       channel_list, ChannelType::ACTION_SLOT, ypos, yscale_fac, eSAction_Flag(saction_flag));
+  draw_elem->ac = ac;
+  draw_elem->animated_id = ale->id;
   draw_elem->adt = ale->adt;
   draw_elem->act = &action;
   draw_elem->action_slot = &slot;
@@ -793,6 +820,7 @@ void ED_add_action_channel(ChannelDrawList *channel_list,
 
   ChannelListElement *draw_elem = channel_list_add_element(
       channel_list, ChannelType::ACTION_LEGACY, ypos, yscale_fac, eSAction_Flag(saction_flag));
+  draw_elem->animated_id = ale->id;
   draw_elem->adt = ale->adt;
   draw_elem->act = act;
   draw_elem->channel_locked = locked;
@@ -813,6 +841,7 @@ void ED_add_grease_pencil_datablock_channel(ChannelDrawList *channel_list,
                                                            eSAction_Flag(saction_flag));
   /* GreasePencil properties can be animated via an Action, so the GP-related
    * animation data is not limited to GP drawings. */
+  draw_elem->animated_id = ale->id;
   draw_elem->adt = ale->adt;
   draw_elem->act = ale->adt ? ale->adt->action : nullptr;
   draw_elem->grease_pencil = grease_pencil;

source/blender/editors/animation/keyframes_keylist.cc

@@ -978,6 +978,50 @@ void summary_to_keylist(bAnimContext *ac,
   ANIM_animdata_freelist(&anim_data);
 }
 
+void action_slot_summary_to_keylist(bAnimContext *ac,
+                                    ID *animated_id,
+                                    animrig::Action &action,
+                                    const animrig::slot_handle_t slot_handle,
+                                    AnimKeylist *keylist,
+                                    const int /* eSAction_Flag */ saction_flag,
+                                    blender::float2 range)
+{
+  /* TODO: downstream code depends on this being non-null (see e.g.
+   * `ANIM_animfilter_action_slot()` and `animfilter_fcurves_span()`). Either
+   * change this parameter to be a reference, or modify the downstream code to
+   * not assume that it's non-null and do something reasonable when it is null. */
+  BLI_assert(animated_id);
+
+  if (!ac) {
+    return;
+  }
+
+  animrig::Slot *slot = action.slot_for_handle(slot_handle);
+  BLI_assert(slot);
+
+  ListBase anim_data = {nullptr, nullptr};
+
+  /* Get F-Curves to take keyframes from. */
+  const eAnimFilter_Flags filter = ANIMFILTER_DATA_VISIBLE;
+  ANIM_animfilter_action_slot(ac, &anim_data, action, *slot, filter, animated_id);
+
+  LISTBASE_FOREACH (const bAnimListElem *, ale, &anim_data) {
+    /* As of the writing of this code, Actions ultimately only contain FCurves.
+     * If/when that changes in the future, this may need to be updated. */
+    if (ale->datatype != ALE_FCURVE) {
+      continue;
+    }
+    fcurve_to_keylist(ale->adt,
+                      static_cast<FCurve *>(ale->data),
+                      keylist,
+                      saction_flag,
+                      range,
+                      ANIM_nla_mapping_allowed(ale));
+  }
+
+  ANIM_animdata_freelist(&anim_data);
+}
+
 void scene_to_keylist(bDopeSheet *ads,
                       Scene *sce,
                       AnimKeylist *keylist,
@@ -1238,19 +1282,6 @@ void action_group_to_keylist(AnimData *adt,
   }
 }
 
-void action_slot_to_keylist(AnimData *adt,
-                            animrig::Action &action,
-                            const animrig::slot_handle_t slot_handle,
-                            AnimKeylist *keylist,
-                            const int saction_flag,
-                            blender::float2 range)
-{
-  BLI_assert(GS(action.id.name) == ID_AC);
-  for (FCurve *fcurve : fcurves_for_action_slot(action, slot_handle)) {
-    fcurve_to_keylist(adt, fcurve, keylist, saction_flag, range, true);
-  }
-}
-
 void action_to_keylist(AnimData *adt,
                        bAction *dna_action,
                        AnimKeylist *keylist,
@@ -1276,7 +1307,9 @@ void action_to_keylist(AnimData *adt,
    * have things like reference strips, where the strip can reference another slot handle.
    */
   BLI_assert(adt);
-  action_slot_to_keylist(adt, action, adt->slot_handle, keylist, saction_flag, range);
+  for (FCurve *fcurve : fcurves_for_action_slot(action, adt->slot_handle)) {
+    fcurve_to_keylist(adt, fcurve, keylist, saction_flag, range, true);
+  }
 }
 
 void gpencil_to_keylist(bDopeSheet *ads, bGPdata *gpd, AnimKeylist *keylist, const bool active)

source/blender/editors/animation/keyframes_keylist_test.cc

@@ -4,6 +4,10 @@
 
 #include "testing/testing.h"
 
+#include "ANIM_action.hh"
+#include "ANIM_fcurve.hh"
+
+#include "ED_anim_api.hh"
 #include "ED_keyframes_keylist.hh"
 
 #include "DNA_anim_types.h"
@@ -11,7 +15,19 @@
 
 #include "MEM_guardedalloc.h"
 
+#include "BKE_action.hh"
+#include "BKE_armature.hh"
 #include "BKE_fcurve.hh"
+#include "BKE_global.hh"
+#include "BKE_idtype.hh"
+#include "BKE_lib_id.hh"
+#include "BKE_main.hh"
+#include "BKE_object.hh"
+
+#include "BLI_string.h"
+
+#include "CLG_log.h"
+#include "testing/testing.h"
 
 #include <functional>
 #include <optional>
@@ -138,4 +154,186 @@ TEST(keylist, find_exact)
   ED_keylist_free(keylist);
 }
 
+class KeylistSummaryTest : public testing::Test {
+ public:
+  Main *bmain;
+  blender::animrig::Action *action;
+  Object *cube;
+  Object *armature;
+  bArmature *armature_data;
+  Bone *bone1;
+  Bone *bone2;
+
+  SpaceAction saction = {nullptr};
+  bAnimContext ac = {nullptr};
+
+  static void SetUpTestSuite()
+  {
+    /* BKE_id_free() hits a code path that uses CLOG, which crashes if not initialized properly. */
+    CLG_init();
+
+    /* To make id_can_have_animdata() and friends work, the `id_types` array needs to be set up. */
+    BKE_idtype_init();
+  }
+
+  static void TearDownTestSuite()
+  {
+    CLG_exit();
+  }
+
+  void SetUp() override
+  {
+    bmain = BKE_main_new();
+    G_MAIN = bmain; /* For BKE_animdata_free(). */
+
+    action = &static_cast<bAction *>(BKE_id_new(bmain, ID_AC, "ACÄnimåtië"))->wrap();
+    cube = BKE_object_add_only_object(bmain, OB_EMPTY, "Küüübus");
+
+    armature_data = BKE_armature_add(bmain, "ARArmature");
+    bone1 = reinterpret_cast<Bone *>(MEM_callocN(sizeof(Bone), "KeylistSummaryTest"));
+    bone2 = reinterpret_cast<Bone *>(MEM_callocN(sizeof(Bone), "KeylistSummaryTest"));
+    STRNCPY(bone1->name, "Bone.001");
+    STRNCPY(bone2->name, "Bone.002");
+    BLI_addtail(&armature_data->bonebase, bone1);
+    BLI_addtail(&armature_data->bonebase, bone2);
+    BKE_armature_bone_hash_make(armature_data);
+
+    armature = BKE_object_add_only_object(bmain, OB_ARMATURE, "OBArmature");
+    armature->data = armature_data;
+    BKE_pose_ensure(bmain, armature, armature_data, false);
+
+    /*
+     * Fill in the common bits for the mock bAnimContext, for an Action editor.
+     *
+     * Tests should fill in:
+     * - saction.action_slot_handle
+     * - ac.obact
+     */
+    saction.action = action;
+    saction.ads.filterflag = eDopeSheet_FilterFlag(0);
+    ac.bmain = bmain;
+    ac.datatype = ANIMCONT_ACTION;
+    ac.data = action;
+    ac.spacetype = SPACE_ACTION;
+    ac.sl = reinterpret_cast<SpaceLink *>(&saction);
+    ac.ads = &saction.ads;
+  }
+
+  void TearDown() override
+  {
+    saction.action_slot_handle = blender::animrig::Slot::unassigned;
+    ac.obact = nullptr;
+
+    BKE_main_free(bmain);
+    G_MAIN = nullptr;
+  }
+};
+
+TEST_F(KeylistSummaryTest, slot_summary_simple)
+{
+  /* Test that a key summary is generated correctly for a slot that's animating
+   * an object's transforms. */
+
+  using namespace blender::animrig;
+
+  Slot &slot_cube = action->slot_add_for_id(cube->id);
+  ASSERT_EQ(ActionSlotAssignmentResult::OK, assign_action_and_slot(action, &slot_cube, cube->id));
+  Channelbag &channelbag = action_channelbag_ensure(*action, cube->id);
+
+  FCurve &loc_x = channelbag.fcurve_ensure(bmain, {"location", 0});
+  FCurve &loc_y = channelbag.fcurve_ensure(bmain, {"location", 1});
+  FCurve &loc_z = channelbag.fcurve_ensure(bmain, {"location", 2});
+
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&loc_x, {1.0, 0.0}, {}, {}));
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&loc_x, {2.0, 1.0}, {}, {}));
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&loc_y, {2.0, 2.0}, {}, {}));
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&loc_y, {3.0, 3.0}, {}, {}));
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&loc_z, {2.0, 4.0}, {}, {}));
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&loc_z, {5.0, 5.0}, {}, {}));
+
+  /* Generate slot summary keylist. */
+  AnimKeylist *keylist = ED_keylist_create();
+  saction.action_slot_handle = slot_cube.handle;
+  ac.obact = cube;
+  action_slot_summary_to_keylist(
+      &ac, &cube->id, *action, slot_cube.handle, keylist, 0, {0.0, 6.0});
+  ED_keylist_prepare_for_direct_access(keylist);
+
+  const ActKeyColumn *col_0 = ED_keylist_find_exact(keylist, 0.0);
+  const ActKeyColumn *col_1 = ED_keylist_find_exact(keylist, 1.0);
+  const ActKeyColumn *col_2 = ED_keylist_find_exact(keylist, 2.0);
+  const ActKeyColumn *col_3 = ED_keylist_find_exact(keylist, 3.0);
+  const ActKeyColumn *col_4 = ED_keylist_find_exact(keylist, 4.0);
+  const ActKeyColumn *col_5 = ED_keylist_find_exact(keylist, 5.0);
+  const ActKeyColumn *col_6 = ED_keylist_find_exact(keylist, 6.0);
+
+  /* Check that we only have columns at the frames with keys. */
+  EXPECT_EQ(nullptr, col_0);
+  EXPECT_NE(nullptr, col_1);
+  EXPECT_NE(nullptr, col_2);
+  EXPECT_NE(nullptr, col_3);
+  EXPECT_EQ(nullptr, col_4);
+  EXPECT_NE(nullptr, col_5);
+  EXPECT_EQ(nullptr, col_6);
+
+  /* Check that the right number of keys are indicated in each column. */
+  EXPECT_EQ(1, col_1->totkey);
+  EXPECT_EQ(3, col_2->totkey);
+  EXPECT_EQ(1, col_3->totkey);
+  EXPECT_EQ(1, col_5->totkey);
+
+  ED_keylist_free(keylist);
+}
+
+TEST_F(KeylistSummaryTest, slot_summary_bone_selection)
+{
+  /* Test that a key summary is generated correctly, excluding keys for
+   * unselected bones when filter-by-selection is on. */
+
+  using namespace blender::animrig;
+
+  Slot &slot_armature = action->slot_add_for_id(armature->id);
+  ASSERT_EQ(ActionSlotAssignmentResult::OK,
+            assign_action_and_slot(action, &slot_armature, armature->id));
+  Channelbag &channelbag = action_channelbag_ensure(*action, armature->id);
+
+  FCurve &bone1_loc_x = channelbag.fcurve_ensure(
+      bmain, {"pose.bones[\"Bone.001\"].location", 0, std::nullopt, "Bone.001"});
+  FCurve &bone2_loc_x = channelbag.fcurve_ensure(
+      bmain, {"pose.bones[\"Bone.002\"].location", 0, std::nullopt, "Bone.002"});
+
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&bone1_loc_x, {1.0, 0.0}, {}, {}));
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&bone1_loc_x, {2.0, 1.0}, {}, {}));
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&bone2_loc_x, {2.0, 2.0}, {}, {}));
+  ASSERT_EQ(SingleKeyingResult::SUCCESS, insert_vert_fcurve(&bone2_loc_x, {3.0, 3.0}, {}, {}));
+
+  /* Select only Bone.001. */
+  bone1->flag |= BONE_SELECTED;
+  bone2->flag &= ~BONE_SELECTED;
+
+  /* Generate slot summary keylist. */
+  AnimKeylist *keylist = ED_keylist_create();
+  saction.ads.filterflag = ADS_FILTER_ONLYSEL; /* Filter by selection. */
+  saction.action_slot_handle = slot_armature.handle;
+  ac.obact = armature;
+  action_slot_summary_to_keylist(
+      &ac, &armature->id, *action, slot_armature.handle, keylist, 0, {0.0, 6.0});
+  ED_keylist_prepare_for_direct_access(keylist);
+
+  const ActKeyColumn *col_1 = ED_keylist_find_exact(keylist, 1.0);
+  const ActKeyColumn *col_2 = ED_keylist_find_exact(keylist, 2.0);
+  const ActKeyColumn *col_3 = ED_keylist_find_exact(keylist, 3.0);
+
+  /* Check that we only have columns at the frames with keys for Bone.001. */
+  EXPECT_NE(nullptr, col_1);
+  EXPECT_NE(nullptr, col_2);
+  EXPECT_EQ(nullptr, col_3);
+
+  /* Check that the right number of keys are indicated in each column. */
+  EXPECT_EQ(1, col_1->totkey);
+  EXPECT_EQ(1, col_2->totkey);
+
+  ED_keylist_free(keylist);
+}
+
 }  // namespace blender::editor::animation::tests

source/blender/editors/include/ED_anim_api.hh

@@ -55,6 +55,11 @@ struct PropertyRNA;
 
 struct MPathTarget;
 
+namespace blender::animrig {
+class Action;
+class Slot;
+}  // namespace blender::animrig
+
 /* ************************************************ */
 /* ANIMATION CHANNEL FILTERING */
 /* `anim_filter.cc` */
@@ -517,6 +522,33 @@ ENUM_OPERATORS(eAnimFilter_Flags, ANIMFILTER_TMP_IGNORE_ONLYSEL);
 /** \name Public API
  * \{ */
 
+/**
+ * Add the channel and sub-channels for an Action Slot to `anim_data`, filtered
+ * according to `filter_mode`.
+ *
+ * \param action: the action containing the slot to generate the channels for.
+ *
+ * \param slot: the slot to generate the channels for.
+ *
+ * \param filter_mode: the filters to use for deciding what channels get
+ * included.
+ *
+ * \param animated_id: the particular animated ID that the slot channels are
+ * being generated for. This is needed for filtering channels based on bone
+ * selection, and also for resolving the names of animated properties. This
+ * should never be null, but it's okay(ish) if it's an ID not actually animated
+ * by the slot, in which case it will act as a fallback in case an ID actually
+ * animated by the slot can't be found.
+ *
+ * \return The number of items added to `anim_data`.
+ */
+size_t ANIM_animfilter_action_slot(bAnimContext *ac,
+                                   ListBase * /* bAnimListElem */ anim_data,
+                                   blender::animrig::Action &action,
+                                   blender::animrig::Slot &slot,
+                                   eAnimFilter_Flags filter_mode,
+                                   ID *animated_id);
+
 /**
  * This function filters the active data source to leave only animation channels suitable for
  * usage by the caller. It will return the length of the list

source/blender/editors/include/ED_keyframes_draw.hh

@@ -75,6 +75,7 @@ void ED_add_action_group_channel(ChannelDrawList *channel_list,
                                  int saction_flag);
 /* Layered Action Summary. */
 void ED_add_action_layered_channel(ChannelDrawList *channel_list,
+                                   bAnimContext *ac,
                                    bAnimListElem *ale,
                                    bAction *action,
                                    const float ypos,
@@ -82,6 +83,7 @@ void ED_add_action_layered_channel(ChannelDrawList *channel_list,
                                    int saction_flag);
 /* Action Slot summary. */
 void ED_add_action_slot_channel(ChannelDrawList *channel_list,
+                                bAnimContext *ac,
                                 bAnimListElem *ale,
                                 blender::animrig::Action &action,
                                 blender::animrig::Slot &slot,

source/blender/editors/include/ED_keyframes_keylist.hh

@@ -180,17 +180,27 @@ void action_group_to_keylist(AnimData *adt,
                              int saction_flag,
                              blender::float2 range);
 /* Action */
+
+/**
+ * Generate a full list of the keys in `dna_action` that are within the frame
+ * range `range`.
+ *
+ * For layered actions, this is limited to the keys that are for the slot
+ * assigned to `adt`.
+ *
+ * Note: this should only be used in places that need or want the *full* list of
+ * keys, without any filtering by e.g. channel selection/visibility, etc. For
+ * use cases that need such filtering, use `action_slot_summary_to_keylist()`
+ * instead.
+ *
+ * \see action_slot_summary_to_keylist()
+ */
 void action_to_keylist(AnimData *adt,
                        bAction *dna_action,
                        AnimKeylist *keylist,
                        int saction_flag,
                        blender::float2 range);
-void action_slot_to_keylist(AnimData *adt,
-                            blender::animrig::Action &action,
-                            blender::animrig::slot_handle_t slot_handle,
-                            AnimKeylist *keylist,
-                            int saction_flag,
-                            blender::float2 range);
+
 /* Object */
 void ob_to_keylist(
     bDopeSheet *ads, Object *ob, AnimKeylist *keylist, int saction_flag, blender::float2 range);
@@ -208,6 +218,42 @@ void summary_to_keylist(bAnimContext *ac,
                         int saction_flag,
                         blender::float2 range);
 
+/**
+ * Generate a summary channel keylist for the specified slot, merging it into
+ * `keylist`.
+ *
+ * This filters the keys to be consistent with the visible channels in the
+ * editor indicated by `ac`
+ *
+ * \param animated_id: the particular animated ID that the slot summary is being
+ * generated for. This is needed for filtering channels based on bone selection,
+ * etc. NOTE: despite being passed as a pointer, this should never be null. It's
+ * currently passed as a pointer to be defensive because I (Nathan) am not 100%
+ * confident at the time of writing (PR #134922) that the callers of this
+ * actually guarantee a non-null pointer (they should, but bugs). This way we
+ * can assert internally to catch if that ever happens.
+ *
+ * \param action: the action containing the slot to generate the summary for.
+ *
+ * \param slot_handle: the handle of the slot to generate the summary for.
+ *
+ * \param keylist: the keylist that the generated summary will be merged into.
+ *
+ * \param saction_flag: needed for the `SACTION_SHOW_EXTREMES` flag, to
+ * determine whether to compute and store the data needed to determine which
+ * keys are "extremes" (local maxima/minima).
+ *
+ * \param range: only keys within this time range will be included in the
+ * summary.
+ */
+void action_slot_summary_to_keylist(bAnimContext *ac,
+                                    ID *animated_id,
+                                    blender::animrig::Action &action,
+                                    blender::animrig::slot_handle_t slot_handle,
+                                    AnimKeylist *keylist,
+                                    int /* eSAction_Flag */ saction_flag,
+                                    blender::float2 range);
+
 /* Grease Pencil datablock summary (Legacy) */
 void gpencil_to_keylist(bDopeSheet *ads, bGPdata *gpd, AnimKeylist *keylist, bool active);
 

source/blender/editors/space_action/action_draw.cc

@@ -370,6 +370,7 @@ static void draw_keyframes(bAnimContext *ac,
         break;
       case ALE_ACTION_LAYERED:
         ED_add_action_layered_channel(draw_list,
+                                      ac,
                                       ale,
                                       static_cast<bAction *>(ale->key_data),
                                       ycenter,
@@ -378,6 +379,7 @@ static void draw_keyframes(bAnimContext *ac,
         break;
       case ALE_ACTION_SLOT:
         ED_add_action_slot_channel(draw_list,
+                                   ac,
                                    ale,
                                    static_cast<bAction *>(ale->key_data)->wrap(),
                                    *static_cast<animrig::Slot *>(ale->data),

source/blender/editors/space_action/action_select.cc

@@ -110,8 +110,19 @@ static void actkeys_list_element_to_keylist(bAnimContext *ac,
         break;
       }
       case ALE_ACTION_LAYERED: {
-        bAction *action = (bAction *)ale->key_data;
-        action_to_keylist(ale->adt, action, keylist, 0, range);
+        /* This is only called for action summaries in the Dopesheet, *not* the
+         * Action Editor. Therefore despite the name `ALE_ACTION_LAYERED`, this
+         * is only used to show a *single slot* of the action: the slot used by
+         * the ID the action is listed under.
+         *
+         * Thus we use the same function as the `ALE_ACTION_SLOT` case below
+         * because in practice the only distinction between these cases is where
+         * they get the slot from. In this case, we get it from `elem`'s ADT. */
+        animrig::Action *action = static_cast<animrig::Action *>(ale->key_data);
+        BLI_assert(action);
+        BLI_assert(ale->adt);
+        action_slot_summary_to_keylist(
+            ac, ale->id, *action, ale->adt->slot_handle, keylist, 0, range);
         break;
       }
       case ALE_ACTION_SLOT: {
@@ -119,10 +130,11 @@ static void actkeys_list_element_to_keylist(bAnimContext *ac,
         animrig::Slot *slot = static_cast<animrig::Slot *>(ale->data);
         BLI_assert(action);
         BLI_assert(slot);
-        action_slot_to_keylist(ale->adt, *action, slot->handle, keylist, 0, range);
+        action_slot_summary_to_keylist(ac, ale->id, *action, slot->handle, keylist, 0, range);
         break;
       }
       case ALE_ACT: {
+        /* Legacy action. */
         bAction *act = (bAction *)ale->key_data;
         action_to_keylist(ale->adt, act, keylist, 0, range);
         break;