Fix #97202: Channels of animation editors disappearing when applying filters

The issue is that when applying filters, the list of channels shrink, but the `View2D` isn't updated accordingly.
When you move the viewport, the channels would jump back into view but not before.

The total height of the channel stack is computed every frame,
and the issue is fixed by calling the `UI_view2d_curRect_clamp_y` after that.

Since this has to be done before `UI_view2d_view_ortho` is called, I had to extract
the height calculations into the caller function.
I thought about making a generic function for all 3 editors but they were too different
to meaningfully do that.

I removed the fix that stopped the channels going off screen when using the cursor to scroll,
since the new logic already does that.

Note: while the report only mentions the Dope Sheet, this fix is applied to
the Graph Editor and NLA editor as well since they had the same issues. This PR fixes them all.

Also fixes #46649

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

source/blender/editors/space_action/action_draw.cc

@@ -55,23 +55,13 @@
 /** \name Channel List
  * \{ */
 
-void draw_channel_names(bContext *C, bAnimContext *ac, ARegion *region)
+void draw_channel_names(bContext *C,
+                        bAnimContext *ac,
+                        ARegion *region,
+                        const ListBase /* bAnimListElem */ &anim_data)
 {
-  ListBase anim_data = {nullptr, nullptr};
   bAnimListElem *ale;
-  eAnimFilter_Flags filter;
-
   View2D *v2d = &region->v2d;
-  size_t items;
-
-  /* build list of channels to draw */
-  filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE | ANIMFILTER_LIST_CHANNELS);
-  items = ANIM_animdata_filter(ac, &anim_data, filter, ac->data, eAnimCont_Types(ac->datatype));
-
-  const int height = ANIM_UI_get_channels_total_height(v2d, items);
-  const float pad_bottom = BLI_listbase_is_empty(ac->markers) ? 0 : UI_MARKER_MARGIN_Y;
-  v2d->tot.ymin = -(height + pad_bottom);
-
   /* need to do a view-sync here, so that the keys area doesn't jump around (it must copy this) */
   UI_view2d_sync(nullptr, ac->area, v2d, V2D_LOCK_COPY);
 
@@ -119,9 +109,6 @@ void draw_channel_names(bContext *C, bAnimContext *ac, ARegion *region)
     UI_block_end(C, block);
     UI_block_draw(C, block);
   }
-
-  /* Free temporary channels. */
-  ANIM_animdata_freelist(&anim_data);
 }
 
 /** \} */

source/blender/editors/space_action/action_intern.hh

@@ -30,7 +30,10 @@ void action_buttons_register(ARegionType *art);
 /**
  * Left hand part.
  */
-void draw_channel_names(bContext *C, bAnimContext *ac, ARegion *region);
+void draw_channel_names(bContext *C,
+                        bAnimContext *ac,
+                        ARegion *region,
+                        const ListBase /* bAnimListElem */ &anim_data);
 /**
  * Draw keyframes in each channel.
  */

source/blender/editors/space_action/space_action.cc

@@ -271,21 +271,39 @@ static void action_channel_region_init(wmWindowManager *wm, ARegion *region)
   WM_event_add_keymap_handler(&region->handlers, keymap);
 }
 
+static void set_v2d_height(View2D *v2d, const size_t item_count, const bool add_marker_padding)
+{
+  const int height = ANIM_UI_get_channels_total_height(v2d, item_count);
+  const float pad_bottom = add_marker_padding ? UI_MARKER_MARGIN_Y : 0;
+  v2d->tot.ymin = -(height + pad_bottom);
+  UI_view2d_curRect_clamp_y(v2d);
+}
+
 static void action_channel_region_draw(const bContext *C, ARegion *region)
 {
   /* draw entirely, view changes should be handled here */
   bAnimContext ac;
+  if (!ANIM_animdata_get_context(C, &ac)) {
+    return;
+  }
+
   View2D *v2d = &region->v2d;
 
   /* clear and setup matrix */
   UI_ThemeClearColor(TH_BACK);
 
-  UI_view2d_view_ortho(v2d);
+  ListBase anim_data = {nullptr, nullptr};
+  /* Build list of channels to draw. */
+  const eAnimFilter_Flags filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE |
+                                    ANIMFILTER_LIST_CHANNELS);
+  const size_t item_count = ANIM_animdata_filter(
+      &ac, &anim_data, filter, ac.data, eAnimCont_Types(ac.datatype));
+  /* The View2D's height needs to be set before calling UI_view2d_view_ortho because the latter
+   * uses the View2D's `cur` rect which might be modified when setting the height. */
+  set_v2d_height(v2d, item_count, !BLI_listbase_is_empty(ac.markers));
 
-  /* data */
-  if (ANIM_animdata_get_context(C, &ac)) {
-    draw_channel_names((bContext *)C, &ac, region);
-  }
+  UI_view2d_view_ortho(v2d);
+  draw_channel_names((bContext *)C, &ac, region, anim_data);
 
   /* channel filter next to scrubbing area */
   ED_time_scrub_channel_search_draw(C, region, ac.ads);
@@ -294,6 +312,7 @@ static void action_channel_region_draw(const bContext *C, ARegion *region)
   UI_view2d_view_restore(C);
 
   /* no scrollers here */
+  ANIM_animdata_freelist(&anim_data);
 }
 
 /* add handlers, stuff you only do once or on area/region changes */
@@ -857,12 +876,6 @@ static void action_space_blend_write(BlendWriter *writer, SpaceLink *sl)
   BLO_write_struct(writer, SpaceAction, sl);
 }
 
-static void action_main_region_view2d_changed(const bContext * /*C*/, ARegion *region)
-{
-  View2D *v2d = &region->v2d;
-  UI_view2d_curRect_clamp_y(v2d);
-}
-
 void ED_spacetype_action()
 {
   std::unique_ptr<SpaceType> st = std::make_unique<SpaceType>();
@@ -896,7 +909,6 @@ void ED_spacetype_action()
   art->draw_overlay = action_main_region_draw_overlay;
   art->listener = action_main_region_listener;
   art->message_subscribe = saction_main_region_message_subscribe;
-  art->on_view2d_changed = action_main_region_view2d_changed;
   art->keymapflag = ED_KEYMAP_GIZMO | ED_KEYMAP_VIEW2D | ED_KEYMAP_ANIMATION | ED_KEYMAP_FRAMES;
 
   BLI_addhead(&st->regiontypes, art);

source/blender/editors/space_graph/graph_draw.cc

@@ -1521,27 +1521,15 @@ void graph_draw_curves(bAnimContext *ac, SpaceGraph *sipo, ARegion *region, shor
 /** \name Channel List
  * \{ */
 
-void graph_draw_channel_names(bContext *C, bAnimContext *ac, ARegion *region)
+void graph_draw_channel_names(bContext *C,
+                              bAnimContext *ac,
+                              ARegion *region,
+                              const ListBase /* bAnimListElem */ &anim_data)
 {
-  ListBase anim_data = {nullptr, nullptr};
   bAnimListElem *ale;
-  int filter;
 
   View2D *v2d = &region->v2d;
-  float height;
-  size_t items;
 
-  /* build list of channels to draw */
-  filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE | ANIMFILTER_LIST_CHANNELS |
-            ANIMFILTER_FCURVESONLY);
-  items = ANIM_animdata_filter(
-      ac, &anim_data, eAnimFilter_Flags(filter), ac->data, eAnimCont_Types(ac->datatype));
-
-  /* Update max-extent of channels here (taking into account scrollers):
-   * - this is done to allow the channel list to be scrollable, but must be done here
-   *   to avoid regenerating the list again and/or also because channels list is drawn first */
-  height = ANIM_UI_get_channels_total_height(v2d, items);
-  v2d->tot.ymin = -height;
   const float channel_step = ANIM_UI_get_channel_step();
 
   /* Loop through channels, and set up drawing depending on their type. */
@@ -1592,9 +1580,6 @@ void graph_draw_channel_names(bContext *C, bAnimContext *ac, ARegion *region)
 
     GPU_blend(GPU_BLEND_NONE);
   }
-
-  /* Free temporary channels. */
-  ANIM_animdata_freelist(&anim_data);
 }
 
 /** \} */

source/blender/editors/space_graph/graph_intern.h

@@ -27,7 +27,10 @@ extern "C" {
 /**
  * Left hand part.
  */
-void graph_draw_channel_names(struct bContext *C, struct bAnimContext *ac, struct ARegion *region);
+void graph_draw_channel_names(struct bContext *C,
+                              struct bAnimContext *ac,
+                              struct ARegion *region,
+                              const ListBase /* bAnimListElem */ &anim_data);
 
 /**
  * This is called twice from `space_graph.cc`, #graph_main_region_draw()

source/blender/editors/space_graph/space_graph.cc

@@ -368,20 +368,34 @@ static void graph_channel_region_init(wmWindowManager *wm, ARegion *region)
   WM_event_add_keymap_handler(&region->handlers, keymap);
 }
 
+static void set_v2d_height(View2D *v2d, const size_t item_count)
+{
+  const int height = ANIM_UI_get_channels_total_height(v2d, item_count);
+  v2d->tot.ymin = -height;
+  UI_view2d_curRect_clamp_y(v2d);
+}
+
 static void graph_channel_region_draw(const bContext *C, ARegion *region)
 {
   bAnimContext ac;
+  if (!ANIM_animdata_get_context(C, &ac)) {
+    return;
+  }
   View2D *v2d = &region->v2d;
 
   /* clear and setup matrix */
   UI_ThemeClearColor(TH_BACK);
 
+  ListBase anim_data = {nullptr, nullptr};
+  const eAnimFilter_Flags filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE |
+                                    ANIMFILTER_LIST_CHANNELS | ANIMFILTER_FCURVESONLY);
+  const size_t item_count = ANIM_animdata_filter(
+      &ac, &anim_data, filter, ac.data, eAnimCont_Types(ac.datatype));
+  set_v2d_height(v2d, item_count);
   UI_view2d_view_ortho(v2d);
 
   /* draw channels */
-  if (ANIM_animdata_get_context(C, &ac)) {
-    graph_draw_channel_names((bContext *)C, &ac, region);
-  }
+  graph_draw_channel_names((bContext *)C, &ac, region, anim_data);
 
   /* channel filter next to scrubbing area */
   ED_time_scrub_channel_search_draw(C, region, ac.ads);
@@ -391,6 +405,8 @@ static void graph_channel_region_draw(const bContext *C, ARegion *region)
 
   /* scrollers */
   UI_view2d_scrollers_draw(v2d, nullptr);
+
+  ANIM_animdata_freelist(&anim_data);
 }
 
 /* add handlers, stuff you only do once or on area/region changes */

source/blender/editors/space_nla/nla_draw.cc

@@ -913,27 +913,14 @@ void draw_nla_main_data(bAnimContext *ac, SpaceNla *snla, ARegion *region)
 /* *********************************************** */
 /* Track List */
 
-void draw_nla_track_list(const bContext *C, bAnimContext *ac, ARegion *region)
+void draw_nla_track_list(const bContext *C,
+                         bAnimContext *ac,
+                         ARegion *region,
+                         const ListBase /* bAnimListElem */ &anim_data)
 {
-  ListBase anim_data = {nullptr, nullptr};
 
   SpaceNla *snla = reinterpret_cast<SpaceNla *>(ac->sl);
   View2D *v2d = &region->v2d;
-  size_t items;
-
-  /* build list of tracks to draw */
-  eAnimFilter_Flags filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE |
-                              ANIMFILTER_LIST_CHANNELS | ANIMFILTER_FCURVESONLY);
-  items = ANIM_animdata_filter(ac, &anim_data, filter, ac->data, eAnimCont_Types(ac->datatype));
-
-  /* Update max-extent of tracks here (taking into account scrollers):
-   * - this is done to allow the track list to be scrollable, but must be done here
-   *   to avoid regenerating the list again and/or also because tracks list is drawn first
-   * - offset of NLATRACK_HEIGHT*2 is added to the height of the tracks, as first is for
-   *  start of list offset, and the second is as a correction for the scrollers.
-   */
-  int height = NLATRACK_TOT_HEIGHT(ac, items);
-  v2d->tot.ymin = -height;
 
   /* need to do a view-sync here, so that the keys area doesn't jump around
    * (it must copy this) */
@@ -988,9 +975,6 @@ void draw_nla_track_list(const bContext *C, bAnimContext *ac, ARegion *region)
 
     GPU_blend(GPU_BLEND_NONE);
   }
-
-  /* free temporary tracks */
-  ANIM_animdata_freelist(&anim_data);
 }
 
 /* *********************************************** */

source/blender/editors/space_nla/nla_intern.hh

@@ -27,7 +27,10 @@ void nla_buttons_register(ARegionType *art);
 /* `nla_draw.cc` */
 
 void draw_nla_main_data(bAnimContext *ac, SpaceNla *snla, ARegion *region);
-void draw_nla_track_list(const bContext *C, bAnimContext *ac, ARegion *region);
+void draw_nla_track_list(const bContext *C,
+                         bAnimContext *ac,
+                         ARegion *region,
+                         const ListBase /* bAnimListElem */ &anim_data);
 
 /* **************************************** */
 /* `nla_select.cc` */

source/blender/editors/space_nla/space_nla.cc

@@ -176,17 +176,35 @@ static void nla_track_region_init(wmWindowManager *wm, ARegion *region)
 static void nla_track_region_draw(const bContext *C, ARegion *region)
 {
   bAnimContext ac;
-  View2D *v2d = &region->v2d;
+  if (!ANIM_animdata_get_context(C, &ac)) {
+    return;
+  }
 
   /* clear and setup matrix */
   UI_ThemeClearColor(TH_BACK);
 
+  ListBase anim_data = {nullptr, nullptr};
+
+  SpaceNla *snla = reinterpret_cast<SpaceNla *>(ac.sl);
+  View2D *v2d = &region->v2d;
+
+  const eAnimFilter_Flags filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_LIST_VISIBLE |
+                                    ANIMFILTER_LIST_CHANNELS | ANIMFILTER_FCURVESONLY);
+  const size_t item_count = ANIM_animdata_filter(
+      &ac, &anim_data, filter, ac.data, eAnimCont_Types(ac.datatype));
+
+  /* Recalculate the height of the track list. Needs to be done before the call to
+   * `UI_view2d_view_ortho`.*/
+  int height = NLATRACK_TOT_HEIGHT(&ac, item_count);
+  if (!BLI_listbase_is_empty(ED_context_get_markers(C))) {
+    height -= (UI_MARKER_MARGIN_Y - NLATRACK_STEP(snla));
+  }
+  v2d->tot.ymin = -height;
+  UI_view2d_curRect_clamp_y(v2d);
+
   UI_view2d_view_ortho(v2d);
 
-  /* data */
-  if (ANIM_animdata_get_context(C, &ac)) {
-    draw_nla_track_list(C, &ac, region);
-  }
+  draw_nla_track_list(C, &ac, region, anim_data);
 
   /* track filter next to scrubbing area */
   ED_time_scrub_channel_search_draw(C, region, ac.ads);
@@ -196,6 +214,7 @@ static void nla_track_region_draw(const bContext *C, ARegion *region)
 
   /* scrollers */
   UI_view2d_scrollers_draw(v2d, nullptr);
+  ANIM_animdata_freelist(&anim_data);
 }
 
 /* add handlers, stuff you only do once or on area/region changes */
@@ -430,20 +449,6 @@ static void nla_main_region_message_subscribe(const wmRegionMessageSubscribePara
   }
 }
 
-static void nla_main_region_view2d_changed(const bContext *C, ARegion *region)
-{
-  SpaceNla *snla = CTX_wm_space_nla(C);
-  View2D *v2d = &region->v2d;
-
-  /* If markers are present add region padding
-   * so bottom strip isn't hidden.
-   */
-  if (!BLI_listbase_is_empty(ED_context_get_markers(C))) {
-    v2d->tot.ymin -= (UI_MARKER_MARGIN_Y - NLATRACK_STEP(snla));
-  }
-  UI_view2d_curRect_clamp_y(v2d);
-}
-
 static void nla_track_region_listener(const wmRegionListenerParams *params)
 {
   ARegion *region = params->region;
@@ -625,7 +630,6 @@ void ED_spacetype_nla()
   art->draw_overlay = nla_main_region_draw_overlay;
   art->listener = nla_main_region_listener;
   art->message_subscribe = nla_main_region_message_subscribe;
-  art->on_view2d_changed = nla_main_region_view2d_changed;
   art->keymapflag = ED_KEYMAP_VIEW2D | ED_KEYMAP_ANIMATION | ED_KEYMAP_FRAMES;
 
   BLI_addhead(&st->regiontypes, art);