From a96f1208cc729f2b4cfaf8152f80d49f627c82b5 Mon Sep 17 00:00:00 2001 From: Christoph Lendenfeld Date: Thu, 29 Feb 2024 10:33:02 +0100 Subject: [PATCH] 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 --- .../editors/space_action/action_draw.cc | 21 ++------- .../editors/space_action/action_intern.hh | 5 ++- .../editors/space_action/space_action.cc | 36 ++++++++++----- .../blender/editors/space_graph/graph_draw.cc | 23 ++-------- .../editors/space_graph/graph_intern.h | 5 ++- .../editors/space_graph/space_graph.cc | 22 ++++++++-- source/blender/editors/space_nla/nla_draw.cc | 24 ++-------- .../blender/editors/space_nla/nla_intern.hh | 5 ++- source/blender/editors/space_nla/space_nla.cc | 44 ++++++++++--------- 9 files changed, 91 insertions(+), 94 deletions(-) diff --git a/source/blender/editors/space_action/action_draw.cc b/source/blender/editors/space_action/action_draw.cc index 6645396e780..a65592a7692 100644 --- a/source/blender/editors/space_action/action_draw.cc +++ b/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 = ®ion->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); } /** \} */ diff --git a/source/blender/editors/space_action/action_intern.hh b/source/blender/editors/space_action/action_intern.hh index 9793d39d7b0..915b7d068b4 100644 --- a/source/blender/editors/space_action/action_intern.hh +++ b/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. */ diff --git a/source/blender/editors/space_action/space_action.cc b/source/blender/editors/space_action/space_action.cc index a42574bdfc2..8df31fcd1cd 100644 --- a/source/blender/editors/space_action/space_action.cc +++ b/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(®ion->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 = ®ion->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 = ®ion->v2d; - UI_view2d_curRect_clamp_y(v2d); -} - void ED_spacetype_action() { std::unique_ptr st = std::make_unique(); @@ -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); diff --git a/source/blender/editors/space_graph/graph_draw.cc b/source/blender/editors/space_graph/graph_draw.cc index 87c44513267..8d117432a7a 100644 --- a/source/blender/editors/space_graph/graph_draw.cc +++ b/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 = ®ion->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); } /** \} */ diff --git a/source/blender/editors/space_graph/graph_intern.h b/source/blender/editors/space_graph/graph_intern.h index fe868e6aaef..28b642d5ee2 100644 --- a/source/blender/editors/space_graph/graph_intern.h +++ b/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() diff --git a/source/blender/editors/space_graph/space_graph.cc b/source/blender/editors/space_graph/space_graph.cc index e7196c14588..e616d84dc9f 100644 --- a/source/blender/editors/space_graph/space_graph.cc +++ b/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(®ion->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 = ®ion->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 */ diff --git a/source/blender/editors/space_nla/nla_draw.cc b/source/blender/editors/space_nla/nla_draw.cc index 794d4007b88..f9eb8f1edf5 100644 --- a/source/blender/editors/space_nla/nla_draw.cc +++ b/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(ac->sl); View2D *v2d = ®ion->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); } /* *********************************************** */ diff --git a/source/blender/editors/space_nla/nla_intern.hh b/source/blender/editors/space_nla/nla_intern.hh index f629c0f96b4..f1dae05c913 100644 --- a/source/blender/editors/space_nla/nla_intern.hh +++ b/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` */ diff --git a/source/blender/editors/space_nla/space_nla.cc b/source/blender/editors/space_nla/space_nla.cc index b41d5b7271f..0b16e9a608b 100644 --- a/source/blender/editors/space_nla/space_nla.cc +++ b/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 = ®ion->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(ac.sl); + View2D *v2d = ®ion->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 = ®ion->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);