blender
/
blender
134
400
Fork 583
Code Issues 6.3k Pull Requests 759 Projects 19 Wiki Activity

Cleanup: Refactor BKE_nlatrack_add to multiple methods to handle adding a new NLA track to the track list. Insert before, after, head, and tail #104929

Merged
Nate Rupsis merged 8 commits from nrupsis/blender:T82241-cleanup-NLATrack_insert into main 2023-03-02 15:02:42 +01:00
Conversation 2 Commits 8 Files Changed 8 +212 -52
8 changed files with 212 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
source/blender/blenkernel/BKE_nla.h
View File

@ -35,6 +35,12 @@ struct PropertyRNA;
/* ----------------------------- */
/* Data Management */
/**
* Create new NLA Track.
* The returned pointer is owned by the caller.
nrupsis marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-20 11:01:40 +01:00
Review
Copy Link

This is one of those moments where I'm like.... wait, there was no function for this yet? 🤯

Anyway, also add a sentence like "The caller owns the returned pointer" for extra clarity.

Might also be good to document this with the functions that take such a pointer and add it to some list. What happens to the ownership of that pointer? Does it get owned by the owner of the list?

This is one of those moments where I'm like.... wait, there was no function for this yet? 🤯 Anyway, also add a sentence like "The caller owns the returned pointer" for extra clarity. Might also be good to document this with the functions that take such a pointer and add it to some list. What happens to the ownership of that pointer? Does it get owned by the owner of the list?
*/
struct NlaTrack *BKE_nlatrack_new(void);
/**
* Frees the given NLA strip, and calls #BKE_nlastrip_remove_and_free to
* remove and free all children strips.
@ -88,12 +94,50 @@ void BKE_nla_tracks_copy_from_adt(struct Main *bmain,
int flag);
/**
* Add a NLA Track to the given AnimData.
* \param prev: NLA-Track to add the new one after.
* Inserts a given NLA track before a specified NLA track within the
* passed NLA track list.
*/
struct NlaTrack *BKE_nlatrack_add(struct AnimData *adt,
struct NlaTrack *prev,
bool is_liboverride);
void BKE_nlatrack_insert_before(ListBase *nla_tracks,
struct NlaTrack *next,
struct NlaTrack *new_track,
bool is_liboverride);
/**
* Inserts a given NLA track after a specified NLA track within the
* passed NLA track list.
*/
void BKE_nlatrack_insert_after(ListBase *nla_tracks,
struct NlaTrack *prev,
struct NlaTrack *new_track,
bool is_liboverride);
/**
* Calls #BKE_nlatrack_new to create a new NLA track, inserts it before the
* given NLA track with #BKE_nlatrack_insert_before.
nrupsis marked this conversation as resolved
Sybren A. Stüvel commented 2023-03-02 11:54:58 +01:00
Review
Copy Link

No longer sets as active ;-)

No longer sets as active ;-)
*/
struct NlaTrack *BKE_nlatrack_new_before(ListBase *nla_tracks,
struct NlaTrack *next,
bool is_liboverride);
/**
* Calls #BKE_nlatrack_new to create a new NLA track, inserts it after the
* given NLA track with #BKE_nlatrack_insert_after.
*/
struct NlaTrack *BKE_nlatrack_new_after(ListBase *nla_tracks,
struct NlaTrack *prev,
bool is_liboverride);
/**
* Calls #BKE_nlatrack_new to create a new NLA track, inserts it as the head of the
* NLA track list with #BKE_nlatrack_new_before.
*/
nrupsis marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-20 11:07:21 +01:00
Review
Copy Link

API-wise I think it's nicer to separate the BKE_nlatrack_new_xxx functions from the _and_set_active part. Having the functions a bit more atomic makes the calling part a bit more verbose, but I think it's a good call. Also the fact that the unit test doesn't cover the "and set active" part of the functionality might be an indication that the function is doing too much.

API-wise I think it's nicer to separate the `BKE_nlatrack_new_xxx` functions from the `_and_set_active` part. Having the functions a bit more atomic makes the calling part a bit more verbose, but I think it's a good call. Also the fact that the unit test doesn't cover the "and set active" part of the functionality might be an indication that the function is doing too much.
Nate Rupsis commented 2023-02-21 02:08:15 +01:00
Review
Copy Link

Considering we already have the BKE_nlatrack_set_active method, it would be easy to refactor that.

Would it be worth modifying:

  • BKE_nlatrack_new_before_and_set_active -> BKE_nlatrack_new_before
  • BKE_nlatrack_new_after_and_set_active -> BKE_nlatrack_new_after
    etc?

The only difference between these modified methods, and BKE_nlatrack_insert_before / after, is that we call BKE_nlatrack_new. Would it also be worth removing those compositional methods and leaving it on the user to create a new NLA Track?

Alternatively, we can create a unit test for BKE_nlatrack_set_active, and from the documentation for BKE_nlatrack_new_before_and_set_active, etc reference that method?

Considering we already have the `BKE_nlatrack_set_active method`, it would be easy to refactor that. Would it be worth modifying: * `BKE_nlatrack_new_before_and_set_active` -> `BKE_nlatrack_new_before` * `BKE_nlatrack_new_after_and_set_active` -> `BKE_nlatrack_new_after` etc? The only difference between these modified methods, and `BKE_nlatrack_insert_before` / after, is that we call `BKE_nlatrack_new`. Would it also be worth removing those compositional methods and leaving it on the user to create a new NLA Track? Alternatively, we can create a unit test for `BKE_nlatrack_set_active`, and from the documentation for `BKE_nlatrack_new_before_and_set_active`, etc reference that method?
Sybren A. Stüvel commented 2023-02-21 16:41:02 +01:00
Review
Copy Link

The only difference between these modified methods, and BKE_nlatrack_insert_before / after, is that we call BKE_nlatrack_new. Would it also be worth removing those compositional methods and leaving it on the user to create a new NLA Track?

Hmmm that does make me think (in both directions -- i.e. doubting myself whether my original comment maybe pushed things too far, or whether we should push it even further than that). I think the important aspect here is that (as least AFAIK) an NLA track by itself doesn't do much. It should be in the NLA and not left 'floating' somewhere on the heap. As such, I think composite functions 'create & put in a useful place' are still good to keep around.

'Being the active one' makes no sense if it's a track by itself -- it only has meaning in the context of a list of tracks. You wouldn't use BKE_nlatrack_set_active() without BKE_nlatrack_new_before(), but I can see a use for BKE_nlatrack_new_before() without BKE_nlatrack_set_active().

Would that be a good enough reason to keep the 'new-and-insert' composites, and pull out the 'set active' part?

> The only difference between these modified methods, and `BKE_nlatrack_insert_before` / after, is that we call `BKE_nlatrack_new`. Would it also be worth removing those compositional methods and leaving it on the user to create a new NLA Track? Hmmm that does make me think (in both directions -- i.e. doubting myself whether my original comment maybe pushed things too far, or whether we should push it even further than that). I think the important aspect here is that (as least AFAIK) an NLA track by itself doesn't do much. It should be in the NLA and not left 'floating' somewhere on the heap. As such, I think composite functions 'create & put in a useful place' are still good to keep around. 'Being the active one' makes no sense if it's a track by itself -- it only has meaning in the context of a list of tracks. You wouldn't use `BKE_nlatrack_set_active()` without `BKE_nlatrack_new_before()`, but I can see a use for `BKE_nlatrack_new_before()` without `BKE_nlatrack_set_active()`. Would that be a good enough reason to keep the 'new-and-insert' composites, and pull out the 'set active' part?
Nate Rupsis commented 2023-02-21 17:01:05 +01:00
Review
Copy Link

I've had a lot of back and forth on this myself.

I think ultimately you're right. The and_set_active aspect of these methods are doing too much. I think having it be the responsibility of the caller isn't too much to ask for, especially if it's how we do it everywhere, it'll be easy to understand the pattern.

I also think the BKE_nlatrack_new_before/after/head/tail methods are a good way of easily understanding where a new track is going to placed. In theory, it could also allow us to add some more "placement" option when adding in new tracks (without needing them active), but unsure if we'd ever want that.

I've had a lot of back and forth on this myself. I think ultimately you're right. The `and_set_active` aspect of these methods are doing too much. I think having it be the responsibility of the caller isn't too much to ask for, especially if it's how we do it everywhere, it'll be easy to understand the pattern. I also think the `BKE_nlatrack_new_before/after/head/tail` methods are a good way of easily understanding where a new track is going to placed. In theory, it could also allow us to add some more "placement" option when adding in new tracks (without needing them active), but unsure if we'd ever want that.
struct NlaTrack *BKE_nlatrack_new_head(ListBase *nla_tracks, bool is_liboverride);
/**
* Calls #BKE_nlatrack_new to create a new NLA track, inserts it as the tail of the
* NLA track list with #BKE_nlatrack_new_after.
*/
struct NlaTrack *BKE_nlatrack_new_tail(ListBase *nla_tracks, const bool is_liboverride);
/**
* Removes the given NLA track from the list of tracks provided.

3
source/blender/blenkernel/intern/ipo.c
View File

@ -2005,7 +2005,8 @@ static void nlastrips_to_animdata(ID *id, ListBase *strips)
/* trying to add to the current failed (no space),
* so add a new track to the stack, and add to that...
*/
nlt = BKE_nlatrack_add(adt, NULL, false);
nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, false);
BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
BKE_nlatrack_add_strip(nlt, strip, false);
}

129
source/blender/blenkernel/intern/nla.c
View File

@ -336,21 +336,62 @@ void BKE_nla_tracks_copy_from_adt(Main *bmain,
/* Adding ------------------------------------------- */
NlaTrack *BKE_nlatrack_add(AnimData *adt, NlaTrack *prev, const bool is_liboverride)
NlaTrack *BKE_nlatrack_new()
{
NlaTrack *nlt;
/* sanity checks */
if (adt == NULL) {
return NULL;
}
/* allocate new track */
nlt = MEM_callocN(sizeof(NlaTrack), "NlaTrack");
NlaTrack *nlt = MEM_callocN(sizeof(NlaTrack), "NlaTrack");
/* set settings requiring the track to not be part of the stack yet */
nlt->flag = NLATRACK_SELECTED | NLATRACK_OVERRIDELIBRARY_LOCAL;
nlt->index = BLI_listbase_count(&adt->nla_tracks);
return nlt;
}
void BKE_nlatrack_insert_before(ListBase *nla_tracks,
struct NlaTrack *next,
struct NlaTrack *new_track,
bool is_liboverride)
{
if (is_liboverride) {
/* Currently, all library override tracks are assumed to be grouped together at the start of
nrupsis marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-20 11:08:28 +01:00
Review
Copy Link

No need to use doxygen (/**) inside functions.

No need to use doxygen (`/**`) inside functions.
* the list. Non overridden must be placed after last library track. */
if (next != NULL && (next->flag & NLATRACK_OVERRIDELIBRARY_LOCAL) == 0) {
BKE_nlatrack_insert_after(nla_tracks, next, new_track, is_liboverride);
return;
}
}
nrupsis marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-20 11:11:48 +01:00
Review
Copy Link

These nested conditions are getting complicated. The "So we can only add" seems to suggest that if this is not the case, the track cannot be added. However, there is no return then, so that means that it gets added as usual.

On one hand it's good that the track always gets added, because that's what this function is for, but I'm not 100% convinced it's the right thing to do here.

It would also be good to have the special-casing for liboverrides documented in the header file.

These nested conditions are getting complicated. The "So we can **only** add" seems to suggest that if this is **not** the case, the track cannot be added. However, there is no `return` then, so that means that it gets added as usual. On one hand it's good that the track always gets added, because that's what this function is for, but I'm not 100% convinced it's the right thing to do here. It would also be good to have the special-casing for liboverrides documented in the header file.
BLI_insertlinkbefore(nla_tracks, next, new_track);
new_track->index = BLI_findindex(nla_tracks, new_track);
/* Must have unique name, but we need to seed this. */
strcpy(new_track->name, "NlaTrack");
BLI_uniquename(nla_tracks,
new_track,
DATA_("NlaTrack"),
'.',
offsetof(NlaTrack, name),
sizeof(new_track->name));
}
void BKE_nlatrack_insert_after(ListBase *nla_tracks,
struct NlaTrack *prev,
struct NlaTrack *new_track,
const bool is_liboverride)
{
BLI_assert(nla_tracks);
nrupsis marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-20 11:25:58 +01:00
Review
Copy Link

I think it's better to separate those out, so that when things go wrong it's clear which pointer was NULL:

BLI_assert(nla_tracks);
BLI_assert(new_track);
I think it's better to separate those out, so that when things go wrong it's clear which pointer was `NULL`: ```c BLI_assert(nla_tracks); BLI_assert(new_track); ```
BLI_assert(new_track);
/** If NULL, then caller intends to insert a new head. But, tracks are not allowed to be placed
* before library overrides. So it must inserted after the last override. */
if (prev == NULL) {
NlaTrack *first_track = (NlaTrack *)nla_tracks->first;
if (first_track != NULL && (first_track->flag & NLATRACK_OVERRIDELIBRARY_LOCAL) == 0) {
prev = first_track;
}
}
/* In liboverride case, we only add local tracks after all those coming from the linked data,
* so we need to find the first local track. */
@ -361,22 +402,46 @@ NlaTrack *BKE_nlatrack_add(AnimData *adt, NlaTrack *prev, const bool is_liboverr
}
prev = first_local != NULL ? first_local->prev : NULL;
}
/* Add track to stack, and make it the active one. */
if (prev != NULL) {
BLI_insertlinkafter(&adt->nla_tracks, prev, nlt);
}
else {
BLI_addtail(&adt->nla_tracks, nlt);
}
BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
BLI_insertlinkafter(nla_tracks, prev, new_track);
new_track->index = BLI_findindex(nla_tracks, new_track);
/* must have unique name, but we need to seed this */
strcpy(nlt->name, "NlaTrack");
BLI_uniquename(
&adt->nla_tracks, nlt, DATA_("NlaTrack"), '.', offsetof(NlaTrack, name), sizeof(nlt->name));
BLI_uniquename(nla_tracks,
new_track,
DATA_("NlaTrack"),
'.',
offsetof(NlaTrack, name),
sizeof(new_track->name));
}
/* return the new track */
return nlt;
NlaTrack *BKE_nlatrack_new_before(ListBase *nla_tracks, struct NlaTrack *next, bool is_liboverride)
{
NlaTrack *new_track = BKE_nlatrack_new();
BKE_nlatrack_insert_before(nla_tracks, next, new_track, is_liboverride);
return new_track;
}
NlaTrack *BKE_nlatrack_new_after(ListBase *nla_tracks, struct NlaTrack *prev, bool is_liboverride)
{
NlaTrack *new_track = BKE_nlatrack_new();
BKE_nlatrack_insert_after(nla_tracks, prev, new_track, is_liboverride);
return new_track;
}
NlaTrack *BKE_nlatrack_new_head(ListBase *nla_tracks, bool is_liboverride)
{
return BKE_nlatrack_new_before(nla_tracks, (NlaTrack *)nla_tracks->first, is_liboverride);
}
NlaTrack *BKE_nlatrack_new_tail(ListBase *nla_tracks, const bool is_liboverride)
{
return BKE_nlatrack_new_after(nla_tracks, (NlaTrack *)nla_tracks->last, is_liboverride);
}
NlaStrip *BKE_nlastrip_new(bAction *act)
@ -448,7 +513,8 @@ NlaStrip *BKE_nlastack_add_strip(AnimData *adt, bAction *act, const bool is_libo
/* trying to add to the last track failed (no track or no space),
* so add a new track to the stack, and add to that...
*/
nlt = BKE_nlatrack_add(adt, NULL, is_liboverride);
nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
BKE_nlatrack_add_strip(nlt, strip, is_liboverride);
BLI_strncpy(nlt->name, act->id.name + 2, sizeof(nlt->name));
}
@ -848,8 +914,8 @@ void BKE_nlastrips_make_metas(ListBase *strips, bool is_temp)
mstrip->end = strip->end;
}
else {
/* current strip wasn't selected, so the end of 'island' of selected strips has been reached,
* so stop adding strips to the current meta
/* current strip wasn't selected, so the end of 'island' of selected strips has been
* reached, so stop adding strips to the current meta.
*/
mstrip = NULL;
}
@ -1180,7 +1246,9 @@ bool BKE_nlatrack_add_strip(NlaTrack *nlt, NlaStrip *strip, const bool is_libove
return false;
}
/* Do not allow adding strips if this track is locked, or not a local one in liboverride case. */
/*
* Do not allow adding strips if this track is locked, or not a local one in liboverride case.
*/
if (nlt->flag & NLATRACK_PROTECTED ||
(is_liboverride && (nlt->flag & NLATRACK_OVERRIDELIBRARY_LOCAL) == 0)) {
return false;
@ -1952,7 +2020,8 @@ bool BKE_nla_action_stash(AnimData *adt, const bool is_liboverride)
}
}
nlt = BKE_nlatrack_add(adt, prev_track, is_liboverride);
nlt = BKE_nlatrack_new_after(&adt->nla_tracks, prev_track, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
BLI_assert(nlt != NULL);
/* We need to ensure that if there wasn't any previous instance,
@ -2075,7 +2144,8 @@ static void nla_tweakmode_find_active(const ListBase /* NlaTrack */ *nla_tracks,
/* There are situations where we may have multiple strips selected and we want to enter
* tweak-mode on all of those at once. Usually in those cases,
* it will usually just be a single strip per AnimData.
* In such cases, compromise and take the last selected track and/or last selected strip, #28468.
* In such cases, compromise and take the last selected track and/or last selected strip,
* #28468.
*/
if (activeTrack == NULL) {
/* try last selected track for active strip */
@ -2150,7 +2220,8 @@ bool BKE_nla_tweakmode_enter(AnimData *adt)
activeStrip->flag &= ~NLASTRIP_FLAG_TWEAKUSER;
/* go over all the tracks after AND INCLUDING the active one, tagging them as being disabled
* - the active track needs to also be tagged, otherwise, it'll overlap with the tweaks going on
* - the active track needs to also be tagged, otherwise, it'll overlap with the tweaks going
* on.
*/
activeTrack->flag |= NLATRACK_DISABLED;
if ((adt->flag & ADT_NLA_EVAL_UPPER_TRACKS) == 0) {

37
source/blender/blenkernel/intern/nla_test.cc
View File

@ -94,12 +94,10 @@ TEST(nla_track, BKE_nlatrack_remove_strip)
TEST(nla_track, BKE_nlatrack_remove_and_free)
{
AnimData adt{};
NlaTrack *track1;
NlaTrack *track2;
/* Add NLA tracks to the Animation Data. */
track1 = BKE_nlatrack_add(&adt, NULL, false);
track2 = BKE_nlatrack_add(&adt, track1, false);
NlaTrack *track1 = BKE_nlatrack_new_tail(&adt.nla_tracks, false);
NlaTrack *track2 = BKE_nlatrack_new_tail(&adt.nla_tracks, false);
/* Ensure we have 2 tracks in the track. */
EXPECT_EQ(2, BLI_listbase_count(&adt.nla_tracks));
@ -116,4 +114,35 @@ TEST(nla_track, BKE_nlatrack_remove_and_free)
EXPECT_EQ(-1, BLI_findindex(&adt.nla_tracks, track1));
}
TEST(nla_track, BKE_nlatrack_new_tail)
nrupsis marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-20 11:31:11 +01:00
Review
Copy Link

There should be separate tests for situations with liboverride tracks. The code to handle those is quite complex, so should be covered by tests.

There should be separate tests for situations with liboverride tracks. The code to handle those is quite complex, so should be covered by tests.
{
AnimData adt{};
NlaTrack *trackB = BKE_nlatrack_new_tail(&adt.nla_tracks, false);
NlaTrack *trackA = BKE_nlatrack_new_tail(&adt.nla_tracks, false);
// Expect that Track B was added before track A
EXPECT_EQ(1, BLI_findindex(&adt.nla_tracks, trackA));
EXPECT_EQ(0, BLI_findindex(&adt.nla_tracks, trackB));
// Free the tracks
BKE_nlatrack_remove_and_free(&adt.nla_tracks, trackA, false);
BKE_nlatrack_remove_and_free(&adt.nla_tracks, trackB, false);
}
TEST(nla_track, BKE_nlatrack_new_head)
{
AnimData adt{};
NlaTrack *trackB = BKE_nlatrack_new_head(&adt.nla_tracks, false);
NlaTrack *trackA = BKE_nlatrack_new_head(&adt.nla_tracks, false);
// Expect that Track A was added before track B
EXPECT_EQ(0, BLI_findindex(&adt.nla_tracks, trackA));
EXPECT_EQ(1, BLI_findindex(&adt.nla_tracks, trackB));
// Free the tracks
BKE_nlatrack_remove_and_free(&adt.nla_tracks, trackA, false);
BKE_nlatrack_remove_and_free(&adt.nla_tracks, trackB, false);
}
} // namespace blender::bke::tests

3
source/blender/editors/object/object_add.cc
View File

@ -1993,7 +1993,8 @@ static int object_speaker_add_exec(bContext *C, wmOperator *op)
{
/* create new data for NLA hierarchy */
AnimData *adt = BKE_animdata_ensure_id(&ob->id);
NlaTrack *nlt = BKE_nlatrack_add(adt, nullptr, is_liboverride);
NlaTrack *nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
NlaStrip *strip = BKE_nla_add_soundstrip(bmain, scene, static_cast<Speaker *>(ob->data));
strip->start = scene->r.cfra;
strip->end += strip->start;

11
source/blender/editors/space_nla/nla_channels.c
View File

@ -573,6 +573,7 @@ bool nlaedit_add_tracks_existing(bAnimContext *ac, bool above_sel)
if (ale->type == ANIMTYPE_NLATRACK) {
NlaTrack *nlt = (NlaTrack *)ale->data;
AnimData *adt = ale->adt;
NlaTrack *new_track = NULL;
const bool is_liboverride = ID_IS_OVERRIDE_LIBRARY(ale->id);
@ -581,14 +582,16 @@ bool nlaedit_add_tracks_existing(bAnimContext *ac, bool above_sel)
*/
if (above_sel) {
/* just add a new one above this one */
BKE_nlatrack_add(adt, nlt, is_liboverride);
new_track = BKE_nlatrack_new_after(&adt->nla_tracks, nlt, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, new_track);
ale->update = ANIM_UPDATE_DEPS;
added = true;
}
else if ((lastAdt == NULL) || (adt != lastAdt)) {
/* add one track to the top of the owning AnimData's stack,
* then don't add anymore to this stack */
BKE_nlatrack_add(adt, NULL, is_liboverride);
new_track = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, new_track);
lastAdt = adt;
ale->update = ANIM_UPDATE_DEPS;
added = true;
@ -618,6 +621,7 @@ bool nlaedit_add_tracks_empty(bAnimContext *ac)
/* check if selected AnimData blocks are empty, and add tracks if so... */
for (ale = anim_data.first; ale; ale = ale->next) {
AnimData *adt = ale->adt;
NlaTrack *new_track;
/* sanity check */
BLI_assert(adt->flag & ADT_UI_SELECTED);
@ -625,7 +629,8 @@ bool nlaedit_add_tracks_empty(bAnimContext *ac)
/* ensure it is empty */
if (BLI_listbase_is_empty(&adt->nla_tracks)) {
/* add new track to this AnimData block then */
BKE_nlatrack_add(adt, NULL, ID_IS_OVERRIDE_LIBRARY(ale->id));
new_track = BKE_nlatrack_new_tail(&adt->nla_tracks, ID_IS_OVERRIDE_LIBRARY(ale->id));
BKE_nlatrack_set_active(&adt->nla_tracks, new_track);
ale->update = ANIM_UPDATE_DEPS;
added = true;
}

16
source/blender/editors/space_nla/nla_edit.c
View File

@ -717,7 +717,8 @@ static int nlaedit_add_actionclip_exec(bContext *C, wmOperator *op)
/* trying to add to the current failed (no space),
* so add a new track to the stack, and add to that...
*/
nlt = BKE_nlatrack_add(adt, NULL, is_liboverride);
nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
BKE_nlatrack_add_strip(nlt, strip, is_liboverride);
}
@ -955,7 +956,8 @@ static int nlaedit_add_sound_exec(bContext *C, wmOperator *UNUSED(op))
/* trying to add to the current failed (no space),
* so add a new track to the stack, and add to that...
*/
nlt = BKE_nlatrack_add(adt, NULL, is_liboverride);
nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
BKE_nlatrack_add_strip(nlt, strip, is_liboverride);
}
@ -1191,11 +1193,8 @@ static int nlaedit_duplicate_exec(bContext *C, wmOperator *op)
/* in case there's no space in the track above,
* or we haven't got a reference to it yet, try adding */
if (BKE_nlatrack_add_strip(nlt->next, nstrip, is_liboverride) == 0) {
/* need to add a new track above the one above the current one
* - if the current one is the last one, nlt->next will be NULL, which defaults to adding
* at the top of the stack anyway...
*/
track = BKE_nlatrack_add(adt, nlt->next, is_liboverride);
track = BKE_nlatrack_new_after(&adt->nla_tracks, nlt->next, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, track);
BKE_nlatrack_add_strip(track, nstrip, is_liboverride);
}
@ -2459,7 +2458,8 @@ static int nlaedit_snap_exec(bContext *C, wmOperator *op)
/* in case there's no space in the current track, try adding */
if (BKE_nlatrack_add_strip(nlt, strip, is_liboverride) == 0) {
/* need to add a new track above the current one */
track = BKE_nlatrack_add(adt, nlt, is_liboverride);
track = BKE_nlatrack_new_after(&adt->nla_tracks, nlt, is_liboverride);
BKE_nlatrack_set_active(&adt->nla_tracks, track);
BKE_nlatrack_add_strip(track, strip, is_liboverride);
/* clear temp meta-strips on this new track,

11
source/blender/makesrna/intern/rna_animation.c
View File

@ -586,7 +586,16 @@ static void rna_KeyingSet_paths_clear(KeyingSet *keyingset, ReportList *reports)
/* needs wrapper function to push notifier */
static NlaTrack *rna_NlaTrack_new(ID *id, AnimData *adt, Main *bmain, bContext *C, NlaTrack *track)
{
NlaTrack *new_track = BKE_nlatrack_add(adt, track, ID_IS_OVERRIDE_LIBRARY(id));
NlaTrack *new_track;
if (track == NULL) {
new_track = BKE_nlatrack_new_tail(&adt->nla_tracks, ID_IS_OVERRIDE_LIBRARY(id));
}
else {
new_track = BKE_nlatrack_new_after(&adt->nla_tracks, track, ID_IS_OVERRIDE_LIBRARY(id));
}
BKE_nlatrack_set_active(&adt->nla_tracks, new_track);
WM_event_add_notifier(C, NC_ANIMATION | ND_NLA | NA_ADDED, NULL);