From d1962be44c313464ad3c4e5143418fbab076d3e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Mon, 8 Sep 2025 18:12:18 +0200 Subject: [PATCH] Anim: remove deprecated `SpaceAction::action` pointer Other code that needs to operate on "the Action that's shown in the Dope Sheet" now accesses the newly-added `bAnimContext::active_action`, which is now also used by `context.active_action`. I've also added `bAnimContext::active_action_user` in case the ID that is animated by this Action is needed. That's either the active `Object` or the active `Key`, again depending on the mode of the Dope Sheet editor. The active Action can also be obtained via `ANIM_active_action_from_area(scene, view_layer, area)`. This is a faster method than the usual `ANIM_animdata_get_context(C, &ac)` and it doesn't need the entire `bContext`. The "Stash Action" and "Push Down Action" to the NLA were also implemented by writing to `SpaceAction::action` via RNA. They now use `ANIM_animdata_get_context()` to get the active action owner ID, and a direct call to `blender::animrig::assign_action()` instead. The remaining use of `SpaceAction::action` was for display & manipulation of Scene/Action markers. This required some work to get addressed, as there was quite a bit of spaghetti and duplicate logic to churn through. More can be improved there, but I had to limit the time I spent on this. Python code that was still using `context.space_data.action` to find the currently-showing Action has been migrated to `context.active_action`. Related: #119626 Pull Request: https://projects.blender.org/blender/blender/pulls/145672 --- scripts/startup/bl_ui/space_dopesheet.py | 7 +- scripts/startup/bl_ui/space_topbar.py | 4 +- .../editors/animation/anim_channels_edit.cc | 13 +- .../blender/editors/animation/anim_filter.cc | 85 +++++++--- .../editors/animation/anim_filter_test.cc | 8 +- .../blender/editors/animation/anim_markers.cc | 50 +++--- .../animation/keyframes_keylist_test.cc | 11 +- source/blender/editors/include/ED_anim_api.hh | 14 ++ source/blender/editors/include/ED_markers.hh | 9 +- .../blender/editors/screen/screen_context.cc | 9 +- .../editors/space_action/action_data.cc | 135 +++------------ .../editors/space_action/action_edit.cc | 19 +-- .../editors/space_action/action_select.cc | 5 +- .../editors/space_action/space_action.cc | 10 +- source/blender/makesdna/DNA_action_types.h | 8 +- source/blender/makesrna/intern/rna_space.cc | 159 ------------------ .../windowmanager/intern/wm_event_system.cc | 8 +- 17 files changed, 180 insertions(+), 374 deletions(-) diff --git a/scripts/startup/bl_ui/space_dopesheet.py b/scripts/startup/bl_ui/space_dopesheet.py index 066db52a7ac..53ba5dc8f41 100644 --- a/scripts/startup/bl_ui/space_dopesheet.py +++ b/scripts/startup/bl_ui/space_dopesheet.py @@ -376,20 +376,21 @@ class DOPESHEET_MT_editor_menus(Menu): def draw(self, context): layout = self.layout st = context.space_data + active_action = context.active_action layout.menu("DOPESHEET_MT_view") layout.menu("DOPESHEET_MT_select") if st.show_markers: layout.menu("DOPESHEET_MT_marker") - if st.mode == 'DOPESHEET' or (st.mode == 'ACTION' and st.action is not None): + if st.mode == 'DOPESHEET' or (st.mode == 'ACTION' and active_action is not None): layout.menu("DOPESHEET_MT_channel") elif st.mode == 'GPENCIL': layout.menu("DOPESHEET_MT_gpencil_channel") layout.menu("DOPESHEET_MT_key") - if st.mode in {'ACTION', 'SHAPEKEY'} and st.action is not None: + if st.mode in {'ACTION', 'SHAPEKEY'} and active_action is not None: layout.menu("DOPESHEET_MT_action") @@ -544,7 +545,7 @@ class DOPESHEET_MT_marker(Menu): st = context.space_data - if st.mode in {'ACTION', 'SHAPEKEY'} and st.action: + if st.mode in {'ACTION', 'SHAPEKEY'} and context.active_action: layout.separator() layout.prop(st, "show_pose_markers") diff --git a/scripts/startup/bl_ui/space_topbar.py b/scripts/startup/bl_ui/space_topbar.py index 05c545c9de9..208d13a0354 100644 --- a/scripts/startup/bl_ui/space_topbar.py +++ b/scripts/startup/bl_ui/space_topbar.py @@ -750,12 +750,12 @@ class TOPBAR_PT_name_marker(Panel): def is_using_pose_markers(context): sd = context.space_data return (sd.type == 'DOPESHEET_EDITOR' and sd.mode in {'ACTION', 'SHAPEKEY'} and - sd.show_pose_markers and sd.action) + sd.show_pose_markers and context.active_action) @staticmethod def get_selected_marker(context): if TOPBAR_PT_name_marker.is_using_pose_markers(context): - markers = context.space_data.action.pose_markers + markers = context.active_action.pose_markers else: markers = context.scene.timeline_markers diff --git a/source/blender/editors/animation/anim_channels_edit.cc b/source/blender/editors/animation/anim_channels_edit.cc index 1e1e53629b2..e4ab90b827e 100644 --- a/source/blender/editors/animation/anim_channels_edit.cc +++ b/source/blender/editors/animation/anim_channels_edit.cc @@ -5466,15 +5466,16 @@ static wmOperatorStatus slot_channels_move_to_new_action_exec(bContext *C, wmOpe static bool slot_channels_move_to_new_action_poll(bContext *C) { - SpaceAction *space_action = CTX_wm_space_action(C); - if (!space_action) { - return false; - } - if (!space_action->action) { + Scene *scene = CTX_data_scene(C); + ViewLayer *view_layer = CTX_data_view_layer(C); + ScrArea *area = CTX_wm_area(C); + bAction *action = ANIM_active_action_from_area(scene, view_layer, area); + + if (!action) { CTX_wm_operator_poll_msg_set(C, "No active action to operate on"); return false; } - if (!space_action->action->wrap().is_action_layered()) { + if (!action->wrap().is_action_layered()) { CTX_wm_operator_poll_msg_set(C, "Active action is not layered"); return false; } diff --git a/source/blender/editors/animation/anim_filter.cc b/source/blender/editors/animation/anim_filter.cc index d7bc4456cfd..d871b5b80ca 100644 --- a/source/blender/editors/animation/anim_filter.cc +++ b/source/blender/editors/animation/anim_filter.cc @@ -102,6 +102,55 @@ using namespace blender; /* ************************************************************ */ /* Blender Context <-> Animation Context mapping */ +bAction *ANIM_active_action_from_area(Scene *scene, + ViewLayer *view_layer, + const ScrArea *area, + ID **r_action_user) +{ + if (area->spacetype != SPACE_ACTION) { + return nullptr; + } + + BKE_view_layer_synced_ensure(scene, view_layer); + Object *ob = BKE_view_layer_active_object_get(view_layer); + if (!ob) { + return nullptr; + } + + const SpaceAction *saction = static_cast(area->spacedata.first); + switch (eAnimEdit_Context(saction->mode)) { + case SACTCONT_ACTION: { + bAction *active_action = ob->adt ? ob->adt->action : nullptr; + if (r_action_user) { + *r_action_user = &ob->id; + } + return active_action; + } + + case SACTCONT_SHAPEKEY: { + Key *active_key = BKE_key_from_object(ob); + bAction *active_action = (active_key && active_key->adt) ? active_key->adt->action : nullptr; + if (r_action_user) { + *r_action_user = &active_key->id; + } + return active_action; + } + + case SACTCONT_GPENCIL: + case SACTCONT_DOPESHEET: + case SACTCONT_MASK: + case SACTCONT_CACHEFILE: + case SACTCONT_TIMELINE: + if (r_action_user) { + *r_action_user = nullptr; + } + return nullptr; + } + + BLI_assert_unreachable(); + return nullptr; +} + /* ----------- Private Stuff - Action Editor ------------- */ /* Get shapekey data being edited (for Action Editor -> ShapeKey mode) */ @@ -132,21 +181,18 @@ static bool actedit_get_context(bAnimContext *ac, SpaceAction *saction) ac->ads = &saction->ads; ac->dopesheet_mode = eAnimEdit_Context(saction->mode); + ac->active_action = ANIM_active_action_from_area( + ac->scene, ac->view_layer, ac->area, &ac->active_action_user); + /* sync settings with current view status, then return appropriate data */ switch (saction->mode) { case SACTCONT_ACTION: /* 'Action Editor' */ - /* if not pinned, sync with active object */ - if (/* `saction->pin == 0` */ true) { - if (ac->obact && ac->obact->adt) { - saction->action = ac->obact->adt->action; - } - else { - saction->action = nullptr; - } - } - ac->datatype = ANIMCONT_ACTION; - ac->data = saction->action; + ac->data = ac->active_action; + + if (saction->flag & SACTION_POSEMARKERS_SHOW) { + ac->markers = &ac->active_action->markers; + } return true; @@ -154,17 +200,10 @@ static bool actedit_get_context(bAnimContext *ac, SpaceAction *saction) ac->datatype = ANIMCONT_SHAPEKEY; ac->data = actedit_get_shapekeys(ac); - /* if not pinned, sync with active object */ - if (/* `saction->pin == 0` */ true) { - Key *key = static_cast(ac->data); - - if (key && key->adt) { - saction->action = key->adt->action; - } - else { - saction->action = nullptr; - } + if (saction->flag & SACTION_POSEMARKERS_SHOW) { + ac->markers = &ac->active_action->markers; } + return true; case SACTCONT_GPENCIL: /* Grease Pencil */ /* XXX review how this mode is handled... */ @@ -377,8 +416,8 @@ bool ANIM_animdata_get_context(const bContext *C, bAnimContext *ac) ac->scene = scene; ac->view_layer = CTX_data_view_layer(C); if (scene) { - ac->markers = ED_context_get_markers(C); - BKE_view_layer_synced_ensure(ac->scene, ac->view_layer); + /* This may be overwritten by actedit_get_context() when pose markers should be shown. */ + ac->markers = &scene->markers; } ac->depsgraph = CTX_data_depsgraph_pointer(C); ac->obact = BKE_view_layer_active_object_get(ac->view_layer); diff --git a/source/blender/editors/animation/anim_filter_test.cc b/source/blender/editors/animation/anim_filter_test.cc index 41df660d783..c0a0e9080c4 100644 --- a/source/blender/editors/animation/anim_filter_test.cc +++ b/source/blender/editors/animation/anim_filter_test.cc @@ -100,8 +100,6 @@ TEST_F(ActionFilterTest, slots_expanded_or_not) /* Mock an bAnimContext for the Animation editor, with the above Animation showing. */ SpaceAction saction = {nullptr}; - saction.action = action; - saction.action_slot_handle = slot_cube.handle; saction.ads.filterflag = eDopeSheet_FilterFlag(0); bAnimContext ac = {nullptr}; @@ -111,6 +109,8 @@ TEST_F(ActionFilterTest, slots_expanded_or_not) ac.spacetype = SPACE_ACTION; ac.sl = reinterpret_cast(&saction); ac.obact = cube; + ac.active_action = action; + ac.active_action_user = &cube->id; ac.ads = &saction.ads; { /* Test with collapsed slots. */ @@ -257,8 +257,6 @@ TEST_F(ActionFilterTest, layered_action_active_fcurves) /* Mock an bAnimContext for the Action editor. */ SpaceAction saction = {nullptr}; - saction.action = action; - saction.action_slot_handle = slot_cube.handle; saction.ads.filterflag = eDopeSheet_FilterFlag(0); bAnimContext ac = {nullptr}; @@ -268,6 +266,8 @@ TEST_F(ActionFilterTest, layered_action_active_fcurves) ac.spacetype = SPACE_ACTION; ac.sl = reinterpret_cast(&saction); ac.obact = cube; + ac.active_action = action; + ac.active_action_user = &cube->id; ac.ads = &saction.ads; { diff --git a/source/blender/editors/animation/anim_markers.cc b/source/blender/editors/animation/anim_markers.cc index f05f6d475a3..1f70dcfe84e 100644 --- a/source/blender/editors/animation/anim_markers.cc +++ b/source/blender/editors/animation/anim_markers.cc @@ -63,28 +63,38 @@ /** \name Marker API * \{ */ -ListBase *ED_scene_markers_get(Scene *scene, ScrArea *area) +ListBase *ED_scene_markers_get(const bContext *C, Scene *scene) { if (!scene) { return nullptr; } - /* local marker sets... */ - if (area) { - if (area->spacetype == SPACE_ACTION) { - SpaceAction *saction = static_cast(area->spacedata.first); - /* local markers can only be shown when there's only a single active action to grab them from - * - flag only takes effect when there's an action, otherwise it can get too confusing? - */ - if (ELEM(saction->mode, SACTCONT_ACTION, SACTCONT_SHAPEKEY) && (saction->action)) { - if (saction->flag & SACTION_POSEMARKERS_SHOW) { - return &saction->action->markers; - } - } + bAnimContext ac; + if (!ANIM_animdata_get_context(C, &ac)) { + return &scene->markers; + } + return ac.markers; +} + +ListBase *ED_scene_markers_get_from_area(Scene *scene, ViewLayer *view_layer, const ScrArea *area) +{ + if (!scene) { + return nullptr; + } + + /* If the area is the dopesheet, AND it is configured to show scene markers (instead of + * pose/action markers), directly go for the scene markers. */ + if (area->spacetype == SPACE_ACTION) { + const SpaceAction *saction = static_cast(area->spacedata.first); + if (!(saction->flag & SACTION_POSEMARKERS_SHOW)) { + return &scene->markers; } } - /* default to using the scene's markers */ + bAction *active_action = ANIM_active_action_from_area(scene, view_layer, area); + if (active_action) { + return &active_action->markers; + } return &scene->markers; } @@ -92,20 +102,12 @@ ListBase *ED_scene_markers_get(Scene *scene, ScrArea *area) ListBase *ED_context_get_markers(const bContext *C) { - return ED_scene_markers_get(CTX_data_scene(C), CTX_wm_area(C)); + return ED_scene_markers_get(C, CTX_data_scene(C)); } ListBase *ED_sequencer_context_get_markers(const bContext *C) { - return ED_scene_markers_get(CTX_data_sequencer_scene(C), CTX_wm_area(C)); -} - -ListBase *ED_animcontext_get_markers(const bAnimContext *ac) -{ - if (ac) { - return ED_scene_markers_get(ac->scene, ac->area); - } - return nullptr; + return ED_scene_markers_get(C, CTX_data_sequencer_scene(C)); } /* --------------------------------- */ diff --git a/source/blender/editors/animation/keyframes_keylist_test.cc b/source/blender/editors/animation/keyframes_keylist_test.cc index 421f9518770..fc9e5438dde 100644 --- a/source/blender/editors/animation/keyframes_keylist_test.cc +++ b/source/blender/editors/animation/keyframes_keylist_test.cc @@ -240,10 +240,9 @@ class KeylistSummaryTest : public testing::Test { * Fill in the common bits for the mock bAnimContext, for an Action editor. * * Tests should fill in: - * - saction.action_slot_handle * - ac.obact + * - ac.active_action_user (= &ac.obact.id) */ - saction.action = action; saction.ads.filterflag = eDopeSheet_FilterFlag(0); ac.bmain = bmain; ac.datatype = ANIMCONT_ACTION; @@ -251,12 +250,14 @@ class KeylistSummaryTest : public testing::Test { ac.spacetype = SPACE_ACTION; ac.sl = reinterpret_cast(&saction); ac.ads = &saction.ads; + ac.active_action = action; } void TearDown() override { - saction.action_slot_handle = blender::animrig::Slot::unassigned; ac.obact = nullptr; + ac.active_action = nullptr; + ac.active_action_user = nullptr; BKE_main_free(bmain); G_MAIN = nullptr; @@ -287,8 +288,8 @@ TEST_F(KeylistSummaryTest, slot_summary_simple) /* Generate slot summary keylist. */ AnimKeylist *keylist = ED_keylist_create(); - saction.action_slot_handle = slot_cube.handle; ac.obact = cube; + ac.active_action_user = &cube->id; 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); @@ -348,8 +349,8 @@ TEST_F(KeylistSummaryTest, slot_summary_bone_selection) /* 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; + ac.active_action_user = &armature->id; 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); diff --git a/source/blender/editors/include/ED_anim_api.hh b/source/blender/editors/include/ED_anim_api.hh index ae5e67be437..0bfb106f405 100644 --- a/source/blender/editors/include/ED_anim_api.hh +++ b/source/blender/editors/include/ED_anim_api.hh @@ -133,6 +133,15 @@ struct bAnimContext { Depsgraph *depsgraph; /** active object */ Object *obact; + + /** + * Active Action, only set when the Dope Sheet shows a single Action (in its + * Action and Shape Key modes). + */ + bAction *active_action; + /** The ID that is animated by `active_action`, and that was used to obtain the pointer. */ + ID *active_action_user; + /** active set of markers */ ListBase *markers; @@ -598,6 +607,11 @@ void ANIM_animdata_freelist(ListBase *anim_data); */ bool ANIM_animdata_can_have_greasepencil(const eAnimCont_Types type); +bAction *ANIM_active_action_from_area(Scene *scene, + ViewLayer *view_layer, + const ScrArea *area, + ID **r_action_user = nullptr); + /* ************************************************ */ /* ANIMATION CHANNELS LIST */ /* anim_channels_*.c */ diff --git a/source/blender/editors/include/ED_markers.hh b/source/blender/editors/include/ED_markers.hh index 8b669435992..76cbb5de0e8 100644 --- a/source/blender/editors/include/ED_markers.hh +++ b/source/blender/editors/include/ED_markers.hh @@ -41,7 +41,7 @@ void ED_markers_draw(const bContext *C, int flag); * * \return A #TimeMarker list. */ -ListBase *ED_scene_markers_get(Scene *scene, ScrArea *area); +ListBase *ED_scene_markers_get(const bContext *C, Scene *scene); /** * Public API for getting markers from context. @@ -50,12 +50,7 @@ ListBase *ED_scene_markers_get(Scene *scene, ScrArea *area); */ ListBase *ED_context_get_markers(const bContext *C); ListBase *ED_sequencer_context_get_markers(const bContext *C); -/** - * Public API for getting markers from "animation" context. - * - * \return A #TimeMarker list. - */ -ListBase *ED_animcontext_get_markers(const bAnimContext *ac); +ListBase *ED_scene_markers_get_from_area(Scene *scene, ViewLayer *view_layer, const ScrArea *area); /** * Apply some transformation to markers after the fact diff --git a/source/blender/editors/screen/screen_context.cc b/source/blender/editors/screen/screen_context.cc index 48f798e6684..4df63e0281a 100644 --- a/source/blender/editors/screen/screen_context.cc +++ b/source/blender/editors/screen/screen_context.cc @@ -51,6 +51,7 @@ #include "UI_interface.hh" #include "WM_api.hh" +#include "ANIM_action.hh" #include "ANIM_armature.hh" #include "ANIM_bone_collections.hh" @@ -840,12 +841,14 @@ static eContextResult screen_ctx_sel_actions_impl(const bContext *C, SpaceAction *saction = (SpaceAction *)ac.sl; if (ELEM(saction->mode, SACTCONT_ACTION, SACTCONT_SHAPEKEY)) { + ID *active_action_id = ac.active_action ? &ac.active_action->id : nullptr; + if (active_only) { - CTX_data_id_pointer_set(result, (ID *)saction->action); + CTX_data_id_pointer_set(result, active_action_id); } else { - if (saction->action && !(editable && !ID_IS_EDITABLE(saction->action))) { - CTX_data_id_list_add(result, &saction->action->id); + if (active_action_id && !(editable && !ID_IS_EDITABLE(active_action_id))) { + CTX_data_id_list_add(result, active_action_id); } CTX_data_type_set(result, CTX_DATA_TYPE_COLLECTION); diff --git a/source/blender/editors/space_action/action_data.cc b/source/blender/editors/space_action/action_data.cc index d2578b17878..177920f09b5 100644 --- a/source/blender/editors/space_action/action_data.cc +++ b/source/blender/editors/space_action/action_data.cc @@ -135,26 +135,6 @@ static bAction *action_create_new(bContext *C, bAction *oldact) return action; } -/* Change the active action used by the action editor */ -static void actedit_change_action(bContext *C, bAction *act) -{ - bScreen *screen = CTX_wm_screen(C); - SpaceAction *saction = (SpaceAction *)CTX_wm_space_data(C); - - PropertyRNA *prop; - - /* create RNA pointers and get the property */ - PointerRNA ptr = RNA_pointer_create_discrete(&screen->id, &RNA_SpaceDopeSheetEditor, saction); - prop = RNA_struct_find_property(&ptr, "action"); - - /* NOTE: act may be nullptr here, so better to just use a cast here */ - PointerRNA idptr = RNA_id_pointer_create((ID *)act); - - /* set the new pointer, and force a refresh */ - RNA_property_pointer_set(&ptr, prop, idptr, nullptr); - RNA_property_update(C, &ptr, prop); -} - /** \} */ /* -------------------------------------------------------------------- */ @@ -253,20 +233,7 @@ static wmOperatorStatus action_new_exec(bContext *C, wmOperator * /*op*/) if (adt && oldact) { BLI_assert(adt_id_owner != nullptr); /* stash the action */ - if (BKE_nla_action_stash({*adt_id_owner, *adt}, ID_IS_OVERRIDE_LIBRARY(adt_id_owner))) { - /* The stash operation will remove the user already - * (and unlink the action from the AnimData action slot). - * Hence, we must unset the ref to the action in the - * action editor too (if this is where we're being called from) - * first before setting the new action once it is created, - * or else the user gets decremented twice! - */ - if (ptr.type == &RNA_SpaceDopeSheetEditor) { - SpaceAction *saction = static_cast(ptr.data); - saction->action = nullptr; - } - } - else { + if (!BKE_nla_action_stash({*adt_id_owner, *adt}, ID_IS_OVERRIDE_LIBRARY(adt_id_owner))) { #if 0 printf("WARNING: Failed to stash %s. It may already exist in the NLA stack though\n", oldact->id.name); @@ -278,9 +245,7 @@ static wmOperatorStatus action_new_exec(bContext *C, wmOperator * /*op*/) action = action_create_new(C, oldact); if (prop) { - /* set this new action - * NOTE: we can't use actedit_change_action, as this function is also called from the NLA - */ + /* set this new action */ PointerRNA idptr = RNA_id_pointer_create(&action->id); RNA_property_pointer_set(&ptr, prop, idptr, nullptr); RNA_property_update(C, &ptr, prop); @@ -325,10 +290,8 @@ static bool action_pushdown_poll(bContext *C) return false; } - SpaceAction *saction = (SpaceAction *)CTX_wm_space_data(C); AnimData *adt = ED_actedit_animdata_from_context(C, nullptr); - - if (!adt || !saction->action) { + if (!adt || !adt->action) { return false; } @@ -340,7 +303,6 @@ static bool action_pushdown_poll(bContext *C) static wmOperatorStatus action_pushdown_exec(bContext *C, wmOperator * /*op*/) { - SpaceAction *saction = (SpaceAction *)CTX_wm_space_data(C); ID *adt_id_owner = nullptr; AnimData *adt = ED_actedit_animdata_from_context(C, &adt_id_owner); @@ -357,11 +319,6 @@ static wmOperatorStatus action_pushdown_exec(bContext *C, wmOperator * /*op*/) /* The action needs updating too, as FCurve modifiers are to be reevaluated. They won't extend * beyond the NLA strip after pushing down to the NLA. */ DEG_id_tag_update_ex(bmain, &action.id, ID_RECALC_ANIMATION); - - /* Stop displaying this action in this editor - * NOTE: The editor itself doesn't set a user... - */ - saction->action = nullptr; } /* Send notifiers that stuff has changed */ @@ -392,28 +349,20 @@ void ACTION_OT_push_down(wmOperatorType *ot) static wmOperatorStatus action_stash_exec(bContext *C, wmOperator *op) { - SpaceAction *saction = (SpaceAction *)CTX_wm_space_data(C); ID *adt_id_owner = nullptr; AnimData *adt = ED_actedit_animdata_from_context(C, &adt_id_owner); /* Perform stashing operation */ if (adt) { /* stash the action */ - if (BKE_nla_action_stash({*adt_id_owner, *adt}, ID_IS_OVERRIDE_LIBRARY(adt_id_owner))) { - /* The stash operation will remove the user already, - * so the flushing step later shouldn't double up - * the user-count fixes. Hence, we must unset this ref - * first before setting the new action. - */ - saction->action = nullptr; - } - else { + if (!BKE_nla_action_stash({*adt_id_owner, *adt}, ID_IS_OVERRIDE_LIBRARY(adt_id_owner))) { /* action has already been added - simply warn about this, and clear */ BKE_report(op->reports, RPT_ERROR, "Action+Slot has already been stashed"); } - /* clear action refs from editor, and then also the backing data (not necessary) */ - actedit_change_action(C, nullptr); + if (!blender::animrig::unassign_action({*adt_id_owner, *adt})) { + BKE_report(op->reports, RPT_ERROR, "Could not unassign the active Action"); + } } /* Send notifiers that stuff has changed */ @@ -488,15 +437,17 @@ static bool action_stash_create_poll(bContext *C) static wmOperatorStatus action_stash_create_exec(bContext *C, wmOperator *op) { - SpaceAction *saction = (SpaceAction *)CTX_wm_space_data(C); ID *adt_id_owner = nullptr; AnimData *adt = ED_actedit_animdata_from_context(C, &adt_id_owner); /* Check for no action... */ - if (saction->action == nullptr) { + if (adt->action == nullptr) { /* just create a new action */ bAction *action = action_create_new(C, nullptr); - actedit_change_action(C, action); + if (!blender::animrig::assign_action(action, {*adt_id_owner, *adt})) { + BKE_reportf( + op->reports, RPT_ERROR, "Could not assign a new Action to %s", adt_id_owner->name + 2); + } } else if (adt) { /* Perform stashing operation */ @@ -506,19 +457,18 @@ static wmOperatorStatus action_stash_create_exec(bContext *C, wmOperator *op) /* Create new action not based on the old one * (since the "new" operator already does that). */ new_action = action_create_new(C, nullptr); - - /* The stash operation will remove the user already, - * so the flushing step later shouldn't double up - * the user-count fixes. Hence, we must unset this ref - * first before setting the new action. - */ - saction->action = nullptr; - actedit_change_action(C, new_action); + if (!blender::animrig::assign_action(new_action, {*adt_id_owner, *adt})) { + BKE_reportf( + op->reports, RPT_ERROR, "Could not assign a new Action to %s", adt_id_owner->name + 2); + } } else { /* action has already been added - simply warn about this, and clear */ BKE_report(op->reports, RPT_ERROR, "Action+Slot has already been stashed"); - actedit_change_action(C, nullptr); + if (!blender::animrig::unassign_action({*adt_id_owner, *adt})) { + BKE_reportf( + op->reports, RPT_ERROR, "Could not un-assign Action from %s", adt_id_owner->name + 2); + } } } @@ -561,7 +511,6 @@ void ED_animedit_unlink_action( bContext *C, ID *id, AnimData *adt, bAction *act, ReportList *reports, bool force_delete) { BLI_assert(id); - ScrArea *area = CTX_wm_area(C); /* If the old action only has a single user (that it's about to lose), * warn user about it @@ -628,11 +577,6 @@ void ED_animedit_unlink_action( RNA_property_pointer_set(&ptr, prop, PointerRNA_NULL, nullptr); RNA_property_update(C, &ptr, prop); - - /* Also update the Action editor legacy Action pointer. */ - if (area->spacetype == SPACE_ACTION) { - actedit_change_action(C, nullptr); - } } } @@ -640,32 +584,15 @@ void ED_animedit_unlink_action( static bool action_unlink_poll(bContext *C) { - { - ID *animated_id = nullptr; - AnimData *adt = ED_actedit_animdata_from_context(C, &animated_id); - if (animated_id) { - if (!BKE_id_is_editable(CTX_data_main(C), animated_id)) { - return false; - } - if (!adt) { - return false; - } - return adt->action != nullptr; - } + ID *animated_id = nullptr; + AnimData *adt = ED_actedit_animdata_from_context(C, &animated_id); + if (!animated_id) { + return false; } - - if (ED_operator_action_active(C)) { - SpaceAction *saction = (SpaceAction *)CTX_wm_space_data(C); - AnimData *adt = ED_actedit_animdata_from_context(C, nullptr); - - /* Only when there's an active action, in the right modes... */ - if (saction->action && adt) { - return true; - } + if (!BKE_id_is_editable(CTX_data_main(C), animated_id)) { + return false; } - - /* something failed... */ - return false; + return adt && adt->action; } static wmOperatorStatus action_unlink_exec(bContext *C, wmOperator *op) @@ -892,10 +819,6 @@ static wmOperatorStatus action_layer_next_exec(bContext *C, wmOperator *op) /* TODO: Needs rest-pose flushing (when we get reference track) */ } } - - /* Update the action that this editor now uses - * NOTE: The calls above have already handled the user-count/anim-data side of things. */ - actedit_change_action(C, adt->action); return OPERATOR_FINISHED; } @@ -996,10 +919,6 @@ static wmOperatorStatus action_layer_prev_exec(bContext *C, wmOperator *op) break; } } - - /* Update the action that this editor now uses - * NOTE: The calls above have already handled the user-count/animdata side of things. */ - actedit_change_action(C, adt->action); return OPERATOR_FINISHED; } diff --git a/source/blender/editors/space_action/action_edit.cc b/source/blender/editors/space_action/action_edit.cc index 5e12a3d6f58..2fa5dfe0cfc 100644 --- a/source/blender/editors/space_action/action_edit.cc +++ b/source/blender/editors/space_action/action_edit.cc @@ -66,8 +66,8 @@ /* ensure that there is: * 1) an active action editor - * 2) that the mode will have an active action available - * 3) that the set of markers being shown are the scene markers, not the list we're merging + * 2) that the set of markers being shown are the scene markers, not the list we're merging + * 3) that the mode will have an active action available * 4) that there are some selected markers */ static bool act_markers_make_local_poll(bContext *C) @@ -80,15 +80,14 @@ static bool act_markers_make_local_poll(bContext *C) } /* 2) */ - if (ELEM(sact->mode, SACTCONT_ACTION, SACTCONT_SHAPEKEY) == 0) { - return false; - } - if (sact->action == nullptr) { + if (sact->flag & SACTION_POSEMARKERS_SHOW) { return false; } /* 3) */ - if (sact->flag & SACTION_POSEMARKERS_SHOW) { + bAction *active_action = ANIM_active_action_from_area( + CTX_data_scene(C), CTX_data_view_layer(C), CTX_wm_area(C)); + if (!active_action) { return false; } @@ -99,9 +98,8 @@ static bool act_markers_make_local_poll(bContext *C) static wmOperatorStatus act_markers_make_local_exec(bContext *C, wmOperator * /*op*/) { ListBase *markers = ED_context_get_markers(C); - - SpaceAction *sact = CTX_wm_space_action(C); - bAction *act = (sact) ? sact->action : nullptr; + bAction *act = ANIM_active_action_from_area( + CTX_data_scene(C), CTX_data_view_layer(C), CTX_wm_area(C)); TimeMarker *marker, *markern = nullptr; @@ -123,6 +121,7 @@ static wmOperatorStatus act_markers_make_local_exec(bContext *C, wmOperator * /* /* Now enable the "show pose-markers only" setting, * so that we can see that something did happen. */ + SpaceAction *sact = CTX_wm_space_action(C); sact->flag |= SACTION_POSEMARKERS_SHOW; /* notifiers - both sets, as this change affects both */ diff --git a/source/blender/editors/space_action/action_select.cc b/source/blender/editors/space_action/action_select.cc index 69a5d099da0..ac399c775b0 100644 --- a/source/blender/editors/space_action/action_select.cc +++ b/source/blender/editors/space_action/action_select.cc @@ -1566,9 +1566,8 @@ static void actkeys_select_leftright(bAnimContext *ac, if (select_mode == SELECT_ADD) { SpaceAction *saction = (SpaceAction *)ac->sl; - if ((saction) && (saction->flag & SACTION_MARKERS_MOVE)) { - ListBase *markers = ED_animcontext_get_markers(ac); - LISTBASE_FOREACH (TimeMarker *, marker, markers) { + if (saction && ac->markers && (saction->flag & SACTION_MARKERS_MOVE)) { + LISTBASE_FOREACH (TimeMarker *, marker, ac->markers) { if (((leftright == ACTKEYS_LRSEL_LEFT) && (marker->frame < scene->r.cfra)) || ((leftright == ACTKEYS_LRSEL_RIGHT) && (marker->frame >= scene->r.cfra))) { diff --git a/source/blender/editors/space_action/space_action.cc b/source/blender/editors/space_action/space_action.cc index 8f65e73e692..9c071fd08db 100644 --- a/source/blender/editors/space_action/space_action.cc +++ b/source/blender/editors/space_action/space_action.cc @@ -239,10 +239,9 @@ static void action_main_region_draw(const bContext *C, ARegion *region) } /* Draw the manually set intended playback frame range highlight in the Action editor. */ - if (ELEM(saction->mode, SACTCONT_ACTION, SACTCONT_SHAPEKEY) && saction->action) { - AnimData *adt = ED_actedit_animdata_from_context(C, nullptr); - - ANIM_draw_action_framerange(adt, saction->action, v2d, -FLT_MAX, FLT_MAX); + if (ac.active_action) { + AnimData *adt = BKE_animdata_from_id(ac.active_action_user); + ANIM_draw_action_framerange(adt, ac.active_action, v2d, -FLT_MAX, FLT_MAX); } /* data */ @@ -877,7 +876,6 @@ static void action_id_remap(ScrArea * /*area*/, { SpaceAction *sact = (SpaceAction *)slink; - mappings.apply(reinterpret_cast(&sact->action), ID_REMAP_APPLY_DEFAULT); mappings.apply(reinterpret_cast(&sact->ads.filter_grp), ID_REMAP_APPLY_DEFAULT); mappings.apply(&sact->ads.source, ID_REMAP_APPLY_DEFAULT); } @@ -888,8 +886,6 @@ static void action_foreach_id(SpaceLink *space_link, LibraryForeachIDData *data) const int data_flags = BKE_lib_query_foreachid_process_flags_get(data); const bool is_readonly = (data_flags & IDWALK_READONLY) != 0; - BKE_LIB_FOREACHID_PROCESS_IDSUPER(data, sact->action, IDWALK_CB_DIRECT_WEAK_LINK); - /* NOTE: Could be deduplicated with the #bDopeSheet handling of #SpaceNla and #SpaceGraph. */ BKE_LIB_FOREACHID_PROCESS_ID(data, sact->ads.source, IDWALK_CB_DIRECT_WEAK_LINK); BKE_LIB_FOREACHID_PROCESS_IDSUPER(data, sact->ads.filter_grp, IDWALK_CB_DIRECT_WEAK_LINK); diff --git a/source/blender/makesdna/DNA_action_types.h b/source/blender/makesdna/DNA_action_types.h index c9b1a4f94fd..4e97b7845bb 100644 --- a/source/blender/makesdna/DNA_action_types.h +++ b/source/blender/makesdna/DNA_action_types.h @@ -1001,10 +1001,8 @@ typedef struct SpaceAction { /** Copied to region. */ View2D v2d DNA_DEPRECATED; - /** The currently active action and its slot. */ - bAction *action; - int32_t action_slot_handle; - char _pad2[4]; + /** The currently active action (deprecated). */ + bAction *action DNA_DEPRECATED; /** The currently active context (when not showing action). */ bDopeSheet ads; @@ -1299,6 +1297,4 @@ typedef struct ActionChannelbag { static_assert(std::is_same_v); static_assert( std::is_same_v); -static_assert( - std::is_same_v); #endif diff --git a/source/blender/makesrna/intern/rna_space.cc b/source/blender/makesrna/intern/rna_space.cc index 98a9cf756c4..de33d8b6f85 100644 --- a/source/blender/makesrna/intern/rna_space.cc +++ b/source/blender/makesrna/intern/rna_space.cc @@ -2294,127 +2294,6 @@ static void rna_ConsoleLine_current_character_set(PointerRNA *ptr, const int ind /* Space Dope-sheet */ -static void rna_SpaceDopeSheetEditor_action_set(PointerRNA *ptr, - PointerRNA value, - ReportList * /*reports*/) -{ - SpaceAction *saction = (SpaceAction *)(ptr->data); - bAction *act = (bAction *)value.data; - - if ((act == nullptr) || (act->idroot == 0)) { - /* just set if we're clearing the action or if the action is "amorphous" still */ - saction->action = act; - } - else { - /* action to set must strictly meet the mode criteria... */ - if (saction->mode == SACTCONT_ACTION) { - /* currently, this is "object-level" only, until we have some way of specifying this */ - if (act->idroot == ID_OB) { - saction->action = act; - } - else { - printf( - "ERROR: cannot assign Action '%s' to Action Editor, as action is not object-level " - "animation\n", - act->id.name + 2); - } - } - else if (saction->mode == SACTCONT_SHAPEKEY) { - /* As the name says, "shape-key level" only. */ - if (act->idroot == ID_KE) { - saction->action = act; - } - else { - printf( - "ERROR: cannot assign Action '%s' to Shape Key Editor, as action doesn't animate " - "Shape Keys\n", - act->id.name + 2); - } - } - else { - printf( - "ACK: who's trying to set an action while not in a mode displaying a single Action " - "only?\n"); - } - } -} - -static void rna_SpaceDopeSheetEditor_action_update(bContext *C, PointerRNA *ptr) -{ - SpaceAction *saction = (SpaceAction *)(ptr->data); - const Scene *scene = CTX_data_scene(C); - ViewLayer *view_layer = CTX_data_view_layer(C); - Main *bmain = CTX_data_main(C); - - BKE_view_layer_synced_ensure(scene, view_layer); - Object *obact = BKE_view_layer_active_object_get(view_layer); - if (obact == nullptr) { - return; - } - - AnimData *adt = nullptr; - ID *id = nullptr; - switch (saction->mode) { - case SACTCONT_ACTION: - /* TODO: context selector could help decide this with more control? */ - adt = BKE_animdata_ensure_id(&obact->id); - id = &obact->id; - break; - case SACTCONT_SHAPEKEY: { - Key *key = BKE_key_from_object(obact); - if (key == nullptr) { - return; - } - adt = BKE_animdata_ensure_id(&key->id); - id = &key->id; - break; - } - case SACTCONT_GPENCIL: - case SACTCONT_DOPESHEET: - case SACTCONT_MASK: - case SACTCONT_CACHEFILE: - case SACTCONT_TIMELINE: - return; - } - - if (adt == nullptr) { - /* No animdata was added, so the depsgraph also doesn't need tagging. */ - return; - } - - /* Don't do anything if old and new actions are the same... */ - if (adt->action == saction->action) { - return; - } - - /* Exit editmode first - we cannot change actions while in tweak-mode. */ - BKE_nla_tweakmode_exit({*id, *adt}); - - /* To prevent data loss (i.e. if users flip between actions using the Browse menu), - * stash this action if nothing else uses it. - * - * EXCEPTION: - * This callback runs when unlinking actions. In that case, we don't want to - * stash the action, as the user is signaling that they want to detach it. - * This can be reviewed again later, - * but it could get annoying if we keep these instead. - */ - if (adt->action != nullptr && adt->action->id.us <= 0 && saction->action != nullptr) { - /* XXX: Things here get dodgy if this action is only partially completed, - * and the user then uses the browse menu to get back to this action, - * assigning it as the active action (i.e. the stash strip gets out of sync) - */ - BKE_nla_action_stash({*id, *adt}, ID_IS_OVERRIDE_LIBRARY(id)); - } - - BKE_animdata_set_action(nullptr, id, saction->action); - - DEG_id_tag_update(&obact->id, ID_RECALC_ANIMATION | ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY); - - /* Update relations as well, so new time source dependency is added. */ - DEG_relations_tag_update(bmain); -} - static void rna_SpaceDopeSheetEditor_mode_update(bContext *C, PointerRNA *ptr) { SpaceAction *saction = (SpaceAction *)(ptr->data); @@ -2422,31 +2301,6 @@ static void rna_SpaceDopeSheetEditor_mode_update(bContext *C, PointerRNA *ptr) const Scene *scene = CTX_data_scene(C); ViewLayer *view_layer = CTX_data_view_layer(C); BKE_view_layer_synced_ensure(scene, view_layer); - Object *obact = BKE_view_layer_active_object_get(view_layer); - - /* special exceptions for ShapeKey Editor mode */ - if (saction->mode == SACTCONT_SHAPEKEY) { - Key *key = BKE_key_from_object(obact); - - /* 1) update the action stored for the editor */ - if (key) { - saction->action = (key->adt) ? key->adt->action : nullptr; - } - else { - saction->action = nullptr; - } - } - /* make sure action stored is valid */ - else if (saction->mode == SACTCONT_ACTION) { - /* 1) update the action stored for the editor */ - /* TODO: context selector could help decide this with more control? */ - if (obact) { - saction->action = (obact->adt) ? obact->adt->action : nullptr; - } - else { - saction->action = nullptr; - } - } /* Collapse (and show) summary channel and hide channel list for timeline */ if (saction->mode == SACTCONT_TIMELINE) { @@ -6763,19 +6617,6 @@ static void rna_def_space_dopesheet(BlenderRNA *brna) (1 << RGN_TYPE_FOOTER) | (1 << RGN_TYPE_UI) | (1 << RGN_TYPE_HUD) | (1 << RGN_TYPE_CHANNELS)); - /* data */ - prop = RNA_def_property(srna, "action", PROP_POINTER, PROP_NONE); - RNA_def_property_flag(prop, PROP_EDITABLE); - RNA_def_property_pointer_funcs(prop, - nullptr, - "rna_SpaceDopeSheetEditor_action_set", - nullptr, - "rna_Action_actedit_assign_poll"); - RNA_def_property_ui_text(prop, "Action", "Action displayed and edited in this space"); - RNA_def_property_flag(prop, PROP_CONTEXT_UPDATE); - RNA_def_property_update( - prop, NC_ANIMATION | ND_KEYFRAME | NA_EDITED, "rna_SpaceDopeSheetEditor_action_update"); - /* mode (hidden in the UI, see 'ui_mode') */ prop = RNA_def_property(srna, "mode", PROP_ENUM, PROP_NONE); RNA_def_property_enum_sdna(prop, nullptr, "mode"); diff --git a/source/blender/windowmanager/intern/wm_event_system.cc b/source/blender/windowmanager/intern/wm_event_system.cc index ddf6af8e4d4..5e19deacb23 100644 --- a/source/blender/windowmanager/intern/wm_event_system.cc +++ b/source/blender/windowmanager/intern/wm_event_system.cc @@ -4980,8 +4980,8 @@ bool WM_event_handler_region_marker_poll(const wmWindow *win, break; } - const ListBase *markers = ED_scene_markers_get(WM_window_get_active_scene(win), - const_cast(area)); + const ListBase *markers = ED_scene_markers_get_from_area( + WM_window_get_active_scene(win), WM_window_get_active_view_layer(win), area); if (BLI_listbase_is_empty(markers)) { return false; } @@ -5002,8 +5002,8 @@ bool WM_event_handler_region_v2d_mask_no_marker_poll(const wmWindow *win, return false; } /* Casting away `const` is only needed for a non-constant return value. */ - const ListBase *markers = ED_scene_markers_get(WM_window_get_active_scene(win), - const_cast(area)); + const ListBase *markers = ED_scene_markers_get_from_area( + WM_window_get_active_scene(win), WM_window_get_active_view_layer(win), area); if (markers && !BLI_listbase_is_empty(markers)) { return !WM_event_handler_region_marker_poll(win, area, region, event); }