Cleanup: Animation, simplify channel selection code

Split up `ANIM_deselect_anim_channels()` into
`ANIM_anim_channels_select_set()` and
`ANIM_anim_channels_select_toggle()`.

`ANIM_anim_channels_select_set()` is equivalent to the old
`ANIM_deselect_anim_channels(..., false, ACHANNEL_SETFLAG_xxx)`.

`ANIM_anim_channels_select_toggle()` is equivalent to the old
`ANIM_deselect_anim_channels(..., true, ACHANNEL_SETFLAG_ADD)`.

`ANIM_deselect_anim_channels(..., true, ACHANNEL_SETFLAG_CLEAR)` was
also called once. The `true` parameter suggested the "toggle" behaviour
was intended, but the `ACHANNEL_SETFLAG_CLEAR` argument prevented any
toggling. This is now replaced with `ANIM_anim_channels_select_set(ac,
ACHANNEL_SETFLAG_CLEAR)` to make this explicit.

No functional changes, just a cleanup in order to better understand how
the selection works.

7 files changed, 130 insertions, 111 deletions

diff --git a/source/blender/editors/animation/anim_channels_edit.c b/source/blender/editors/animation/anim_channels_edit.c
index 8a13872ca61..5bed6175ebc 100644
--- a/source/blender/editors/animation/anim_channels_edit.c
+++ b/source/blender/editors/animation/anim_channels_edit.c
@@ -253,109 +253,107 @@ static void select_pchan_for_action_group(bAnimContext *ac, bActionGroup *agrp,
   }
 }
 
-/* Deselect all animation channels
- * - data: pointer to datatype, as contained in bAnimContext
- * - datatype: the type of data that 'data' represents (eAnimCont_Types)
- * - test: check if deselecting instead of selecting
- * - sel: eAnimChannels_SetFlag;
- */
-void ANIM_deselect_anim_channels(
-    bAnimContext *ac, void *data, eAnimCont_Types datatype, bool test, eAnimChannels_SetFlag sel)
+static ListBase /* bAnimListElem */ anim_channels_for_selection(bAnimContext *ac)
 {
   ListBase anim_data = {NULL, NULL};
-  bAnimListElem *ale;
-  int filter;
 
   /* filter data */
   /* NOTE: no list visible, otherwise, we get dangling */
-  filter = ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_CHANNELS;
-  ANIM_animdata_filter(ac, &anim_data, filter, data, datatype);
+  const int filter = ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_CHANNELS;
+  ANIM_animdata_filter(ac, &anim_data, filter, ac->data, ac->datatype);
 
-  /* See if we should be selecting or deselecting */
-  if (test) {
-    for (ale = anim_data.first; ale; ale = ale->next) {
-      if (sel == 0) {
-        break;
-      }
+  return anim_data;
+}
 
-      switch (ale->type) {
-        case ANIMTYPE_SCENE:
-          if (ale->flag & SCE_DS_SELECTED) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
-          }
-          break;
-        case ANIMTYPE_OBJECT:
+static eAnimChannels_SetFlag anim_channels_selection_flag_for_toggle(const ListBase anim_data)
+{
+  /* See if we should be selecting or deselecting. */
+  for (bAnimListElem *ale = anim_data.first; ale; ale = ale->next) {
+    switch (ale->type) {
+      case ANIMTYPE_SCENE:
+        if (ale->flag & SCE_DS_SELECTED) {
+          return ACHANNEL_SETFLAG_CLEAR;
+        }
+        break;
+      case ANIMTYPE_OBJECT:
 #if 0 /* for now, do not take object selection into account, since it gets too annoying */
           if (ale->flag & SELECT) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
+            return ACHANNEL_SETFLAG_CLEAR;
           }
 #endif
-          break;
-        case ANIMTYPE_GROUP:
-          if (ale->flag & AGRP_SELECTED) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
-          }
-          break;
-        case ANIMTYPE_FCURVE:
-        case ANIMTYPE_NLACURVE:
-          if (ale->flag & FCURVE_SELECTED) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
-          }
-          break;
-        case ANIMTYPE_SHAPEKEY:
-          if (ale->flag & KEYBLOCK_SEL) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
-          }
-          break;
-        case ANIMTYPE_NLATRACK:
-          if (ale->flag & NLATRACK_SELECTED) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
-          }
-          break;
+        break;
+      case ANIMTYPE_GROUP:
+        if (ale->flag & AGRP_SELECTED) {
+          return ACHANNEL_SETFLAG_CLEAR;
+        }
+        break;
+      case ANIMTYPE_FCURVE:
+      case ANIMTYPE_NLACURVE:
+        if (ale->flag & FCURVE_SELECTED) {
+          return ACHANNEL_SETFLAG_CLEAR;
+        }
+        break;
+      case ANIMTYPE_SHAPEKEY:
+        if (ale->flag & KEYBLOCK_SEL) {
+          return ACHANNEL_SETFLAG_CLEAR;
+        }
+        break;
+      case ANIMTYPE_NLATRACK:
+        if (ale->flag & NLATRACK_SELECTED) {
+          return ACHANNEL_SETFLAG_CLEAR;
+        }
+        break;
 
-        case ANIMTYPE_FILLACTD: /* Action Expander */
-        case ANIMTYPE_DSMAT:    /* Datablock AnimData Expanders */
-        case ANIMTYPE_DSLAM:
-        case ANIMTYPE_DSCAM:
-        case ANIMTYPE_DSCACHEFILE:
-        case ANIMTYPE_DSCUR:
-        case ANIMTYPE_DSSKEY:
-        case ANIMTYPE_DSWOR:
-        case ANIMTYPE_DSPART:
-        case ANIMTYPE_DSMBALL:
-        case ANIMTYPE_DSARM:
-        case ANIMTYPE_DSMESH:
-        case ANIMTYPE_DSNTREE:
-        case ANIMTYPE_DSTEX:
-        case ANIMTYPE_DSLAT:
-        case ANIMTYPE_DSLINESTYLE:
-        case ANIMTYPE_DSSPK:
-        case ANIMTYPE_DSGPENCIL:
-        case ANIMTYPE_DSMCLIP:
-        case ANIMTYPE_DSHAIR:
-        case ANIMTYPE_DSPOINTCLOUD:
-        case ANIMTYPE_DSVOLUME:
-        case ANIMTYPE_DSSIMULATION: {
-          if ((ale->adt) && (ale->adt->flag & ADT_UI_SELECTED)) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
-          }
-          break;
+      case ANIMTYPE_FILLACTD: /* Action Expander */
+      case ANIMTYPE_DSMAT:    /* Datablock AnimData Expanders */
+      case ANIMTYPE_DSLAM:
+      case ANIMTYPE_DSCAM:
+      case ANIMTYPE_DSCACHEFILE:
+      case ANIMTYPE_DSCUR:
+      case ANIMTYPE_DSSKEY:
+      case ANIMTYPE_DSWOR:
+      case ANIMTYPE_DSPART:
+      case ANIMTYPE_DSMBALL:
+      case ANIMTYPE_DSARM:
+      case ANIMTYPE_DSMESH:
+      case ANIMTYPE_DSNTREE:
+      case ANIMTYPE_DSTEX:
+      case ANIMTYPE_DSLAT:
+      case ANIMTYPE_DSLINESTYLE:
+      case ANIMTYPE_DSSPK:
+      case ANIMTYPE_DSGPENCIL:
+      case ANIMTYPE_DSMCLIP:
+      case ANIMTYPE_DSHAIR:
+      case ANIMTYPE_DSPOINTCLOUD:
+      case ANIMTYPE_DSVOLUME:
+      case ANIMTYPE_DSSIMULATION: {
+        if ((ale->adt) && (ale->adt->flag & ADT_UI_SELECTED)) {
+          return ACHANNEL_SETFLAG_CLEAR;
         }
-        case ANIMTYPE_GPLAYER:
-          if (ale->flag & GP_LAYER_SELECT) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
-          }
-          break;
-        case ANIMTYPE_MASKLAYER:
-          if (ale->flag & MASK_LAYERFLAG_SELECT) {
-            sel = ACHANNEL_SETFLAG_CLEAR;
-          }
-          break;
+        break;
       }
+      case ANIMTYPE_GPLAYER:
+        if (ale->flag & GP_LAYER_SELECT) {
+          return ACHANNEL_SETFLAG_CLEAR;
+        }
+        break;
+      case ANIMTYPE_MASKLAYER:
+        if (ale->flag & MASK_LAYERFLAG_SELECT) {
+          return ACHANNEL_SETFLAG_CLEAR;
+        }
+        break;
     }
   }
 
-  /* Now set the flags */
+  return ACHANNEL_SETFLAG_ADD;
+}
+
+static void anim_channels_select_set(bAnimContext *ac,
+                                     const ListBase anim_data,
+                                     eAnimChannels_SetFlag sel)
+{
+  bAnimListElem *ale;
+
   for (ale = anim_data.first; ale; ale = ale->next) {
     switch (ale->type) {
       case ANIMTYPE_SCENE: {
@@ -395,6 +393,7 @@ void ANIM_deselect_anim_channels(
 
         ACHANNEL_SET_FLAG(fcu, sel, FCURVE_SELECTED);
         fcu->flag &= ~FCURVE_ACTIVE;
+        printf("    selecting and deactivating FCurve[%s; %d]\n", fcu->rna_path, fcu->array_index);
         break;
       }
       case ANIMTYPE_SHAPEKEY: {
@@ -454,8 +453,22 @@ void ANIM_deselect_anim_channels(
       }
     }
   }
+}
+
+/* Set selection state of all animation channels in the context. */
+void ANIM_anim_channels_select_set(bAnimContext *ac, eAnimChannels_SetFlag sel)
+{
+  ListBase anim_data = anim_channels_for_selection(ac);
+  anim_channels_select_set(ac, anim_data, sel);
+  ANIM_animdata_freelist(&anim_data);
+}
 
-  /* Cleanup */
+/* Toggle selection state of all animation channels in the context. */
+void ANIM_anim_channels_select_toggle(bAnimContext *ac)
+{
+  ListBase anim_data = anim_channels_for_selection(ac);
+  const eAnimChannels_SetFlag sel = anim_channels_selection_flag_for_toggle(anim_data);
+  anim_channels_select_set(ac, anim_data, sel);
   ANIM_animdata_freelist(&anim_data);
 }
 
@@ -2539,16 +2552,16 @@ static int animchannels_selectall_exec(bContext *C, wmOperator *op)
   const int action = RNA_enum_get(op->ptr, "action");
   switch (action) {
     case SEL_TOGGLE:
-      ANIM_deselect_anim_channels(&ac, ac.data, ac.datatype, true, ACHANNEL_SETFLAG_ADD);
+      ANIM_anim_channels_select_toggle(&ac);
       break;
     case SEL_SELECT:
-      ANIM_deselect_anim_channels(&ac, ac.data, ac.datatype, false, ACHANNEL_SETFLAG_ADD);
+      ANIM_anim_channels_select_set(&ac, ACHANNEL_SETFLAG_ADD);
       break;
     case SEL_DESELECT:
-      ANIM_deselect_anim_channels(&ac, ac.data, ac.datatype, false, ACHANNEL_SETFLAG_CLEAR);
+      ANIM_anim_channels_select_set(&ac, ACHANNEL_SETFLAG_CLEAR);
       break;
     case SEL_INVERT:
-      ANIM_deselect_anim_channels(&ac, ac.data, ac.datatype, false, ACHANNEL_SETFLAG_INVERT);
+      ANIM_anim_channels_select_set(&ac, ACHANNEL_SETFLAG_INVERT);
       break;
     default:
       BLI_assert(0);
@@ -2670,7 +2683,11 @@ static int animchannels_box_select_exec(bContext *C, wmOperator *op)
   WM_operator_properties_border_to_rcti(op, &rect);
 
   if (!extend) {
-    ANIM_deselect_anim_channels(&ac, ac.data, ac.datatype, true, ACHANNEL_SETFLAG_CLEAR);
+    printf("\n\n\n\033[92mBox-selecting channels without extend!\033[0m\n");
+    ANIM_anim_channels_select_set(&ac, ACHANNEL_SETFLAG_CLEAR);
+  }
+  else {
+    printf("\n\n\n\033[91mBox-selecting channels WITH extend!\033[0m\n");
   }
 
   if (select) {
@@ -2942,7 +2959,7 @@ static int click_select_channel_dummy(bAnimContext *ac,
   }
   else {
     /* select AnimData block by itself */
-    ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, false, ACHANNEL_SETFLAG_CLEAR);
+    ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     ale->adt->flag |= ADT_UI_SELECTED;
   }
 
@@ -2998,7 +3015,7 @@ static int click_select_channel_group(bAnimContext *ac,
     FCurve *fcu;
 
     /* deselect all other channels */
-    ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, false, ACHANNEL_SETFLAG_CLEAR);
+    ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     if (pchan) {
       ED_pose_deselect_all(ob, SEL_DESELECT, false);
     }
@@ -3011,7 +3028,7 @@ static int click_select_channel_group(bAnimContext *ac,
   }
   else {
     /* select group by itself */
-    ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, false, ACHANNEL_SETFLAG_CLEAR);
+    ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     if (pchan) {
       ED_pose_deselect_all(ob, SEL_DESELECT, false);
     }
@@ -3050,7 +3067,7 @@ static int click_select_channel_fcurve(bAnimContext *ac,
   }
   else {
     /* select F-Curve by itself */
-    ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, false, ACHANNEL_SETFLAG_CLEAR);
+    ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     fcu->flag |= FCURVE_SELECTED;
   }
 
@@ -3075,7 +3092,7 @@ static int click_select_channel_shapekey(bAnimContext *ac,
   }
   else {
     /* select ShapeKey by itself */
-    ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, false, ACHANNEL_SETFLAG_CLEAR);
+    ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     kb->flag |= KEYBLOCK_SEL;
   }
 
@@ -3125,7 +3142,7 @@ static int click_select_channel_gplayer(bContext *C,
   }
   else {
     /* select layer by itself */
-    ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, false, ACHANNEL_SETFLAG_CLEAR);
+    ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     gpl->flag |= GP_LAYER_SELECT;
   }
 
@@ -3169,7 +3186,7 @@ static int click_select_channel_masklayer(bAnimContext *ac,
   }
   else {
     /* select layer by itself */
-    ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, false, ACHANNEL_SETFLAG_CLEAR);
+    ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     masklay->flag |= MASK_LAYERFLAG_SELECT;
   }
 
diff --git a/source/blender/editors/include/ED_anim_api.h b/source/blender/editors/include/ED_anim_api.h
index 04f6932d671..9cf67816df2 100644
--- a/source/blender/editors/include/ED_anim_api.h
+++ b/source/blender/editors/include/ED_anim_api.h
@@ -620,9 +620,11 @@ void ANIM_flush_setting_anim_channels(bAnimContext *ac,
                                       eAnimChannel_Settings setting,
                                       eAnimChannels_SetFlag mode);
 
-/* Deselect all animation channels */
-void ANIM_deselect_anim_channels(
-    bAnimContext *ac, void *data, eAnimCont_Types datatype, bool test, eAnimChannels_SetFlag sel);
+/* Select or deselect animation channels */
+void ANIM_anim_channels_select_set(bAnimContext *ac, eAnimChannels_SetFlag sel);
+
+/* Toggle selection of animation channels */
+void ANIM_anim_channels_select_toggle(bAnimContext *ac);
 
 /* Set the 'active' channel of type channel_type, in the given action */
 void ANIM_set_active_channel(bAnimContext *ac,
diff --git a/source/blender/editors/screen/screen_ops.c b/source/blender/editors/screen/screen_ops.c
index 16d7c641c50..6d5580f58ea 100644
--- a/source/blender/editors/screen/screen_ops.c
+++ b/source/blender/editors/screen/screen_ops.c
@@ -4989,7 +4989,7 @@ static int drivers_editor_show_exec(bContext *C, wmOperator *op)
         bAnimContext ac;
         if (ANIM_animdata_get_context(C, &ac)) {
           int filter = ANIMFILTER_DATA_VISIBLE | ANIMFILTER_NODUPLIS;
-          ANIM_deselect_anim_channels(&ac, ac.data, ac.datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+          ANIM_anim_channels_select_set(&ac, ACHANNEL_SETFLAG_CLEAR);
           ANIM_set_active_channel(&ac, ac.data, ac.datatype, filter, fcu, ANIMTYPE_FCURVE);
         }
         else {
diff --git a/source/blender/editors/space_action/action_select.c b/source/blender/editors/space_action/action_select.c
index c49bac58474..ab5f1e0ab22 100644
--- a/source/blender/editors/space_action/action_select.c
+++ b/source/blender/editors/space_action/action_select.c
@@ -1730,7 +1730,7 @@ static int mouse_action_keys(bAnimContext *ac,
       /* highlight channel clicked on */
       if (ELEM(ac->datatype, ANIMCONT_ACTION, ANIMCONT_DOPESHEET, ANIMCONT_TIMELINE)) {
         /* deselect all other channels first */
-        ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+        ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
 
         /* Highlight Action-Group or F-Curve? */
         if (ale != NULL && ale->data) {
@@ -1750,7 +1750,7 @@ static int mouse_action_keys(bAnimContext *ac,
       }
       else if (ac->datatype == ANIMCONT_GPENCIL) {
         /* deselect all other channels first */
-        ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+        ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
 
         /* Highlight GPencil Layer */
         if (ale != NULL && ale->data != NULL && ale->type == ANIMTYPE_GPLAYER) {
@@ -1768,7 +1768,7 @@ static int mouse_action_keys(bAnimContext *ac,
       }
       else if (ac->datatype == ANIMCONT_MASK) {
         /* deselect all other channels first */
-        ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+        ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
 
         if (ale != NULL && ale->data != NULL && ale->type == ANIMTYPE_MASKLAYER) {
           MaskLayer *masklay = ale->data;
diff --git a/source/blender/editors/space_graph/graph_select.c b/source/blender/editors/space_graph/graph_select.c
index 36c836b0a91..df89a4ff725 100644
--- a/source/blender/editors/space_graph/graph_select.c
+++ b/source/blender/editors/space_graph/graph_select.c
@@ -1471,7 +1471,7 @@ static int mouse_graph_keys(bAnimContext *ac,
      * doesn't depend on this
      */
     if ((sipo->flag & SIPO_SELCUVERTSONLY) == 0) {
-      ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+      ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     }
   }
 
diff --git a/source/blender/editors/space_nla/nla_channels.c b/source/blender/editors/space_nla/nla_channels.c
index fbda0ff7bde..561de5e82a6 100644
--- a/source/blender/editors/space_nla/nla_channels.c
+++ b/source/blender/editors/space_nla/nla_channels.c
@@ -204,7 +204,7 @@ static int mouse_nla_channels(
         }
         else {
           /* select AnimData block by itself */
-          ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+          ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
           ale->adt->flag |= ADT_UI_SELECTED;
         }
 
@@ -267,7 +267,7 @@ static int mouse_nla_channels(
         }
         else {
           /* select F-Curve by itself */
-          ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+          ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
           nlt->flag |= NLATRACK_SELECTED;
         }
 
@@ -329,7 +329,7 @@ static int mouse_nla_channels(
           }
           else {
             /* select AnimData block by itself */
-            ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+            ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
             adt->flag |= ADT_UI_SELECTED;
           }
 
diff --git a/source/blender/editors/space_nla/nla_select.c b/source/blender/editors/space_nla/nla_select.c
index 74e5f1cfdd0..751a05b605e 100644
--- a/source/blender/editors/space_nla/nla_select.c
+++ b/source/blender/editors/space_nla/nla_select.c
@@ -638,7 +638,7 @@ static int mouse_nla_strips(bContext *C,
       deselect_nla_strips(ac, 0, SELECT_SUBTRACT);
 
       /* deselect all other channels first */
-      ANIM_deselect_anim_channels(ac, ac->data, ac->datatype, 0, ACHANNEL_SETFLAG_CLEAR);
+      ANIM_anim_channels_select_set(ac, ACHANNEL_SETFLAG_CLEAR);
     }
   }