tmaq
/
tornavis
1
0
Fork 0
Code Issues 2 Pull Requests 29 Packages Projects 24 Releases Wiki Activity

Fix #115655: Removing transition keys is buggy 

This also fixes crash when deleting keys.

Issue was caused by incorrect implementation of batch deleting with
`SEQ_retiming_remove_multiple_keys()` function. It tried to remove data
from different strips, when it should work with one strip at the time.
Also transitions and freeze frames were treated as normal keys, but they
do need special handling.

Pull Request: https://projects.blender.org/blender/blender/pulls/116722
This commit is contained in:
Richard Antalik 2024-01-03 17:01:53 +01:00 committed by Richard Antalik
parent f0d6346c9a
commit 534a1c9ecd
2 changed files with 82 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
source/blender/editors/space_sequencer/sequencer_retiming.cc
View File

@ -901,22 +901,26 @@ int sequencer_retiming_key_remove_exec(bContext *C, wmOperator * /* op */)
{
Scene *scene = CTX_data_scene(C);
blender::Vector<Sequence *> strips_to_handle;
blender::Vector<SeqRetimingKey *> keys_to_delete;
blender::Map selection = SEQ_retiming_selection_get(SEQ_editing_get(scene));
blender::Vector<Sequence *> strips_to_handle;
for (auto item : selection.items()) {
/* First and last key can not be removed. */
if (item.key->strip_frame_index == 0 || SEQ_retiming_is_last_key(item.value, item.key)) {
continue;
}
strips_to_handle.append_non_duplicates(item.value);
keys_to_delete.append(item.key);
for (Sequence *seq : selection.values()) {
strips_to_handle.append_non_duplicates(seq);
}
for (Sequence *seq : strips_to_handle) {
blender::Vector<SeqRetimingKey *> keys_to_delete;
for (auto item : selection.items()) {
if (item.value != seq) {
continue;
}
keys_to_delete.append(item.key);
}
SEQ_retiming_remove_multiple_keys(seq, keys_to_delete);
}
for (Sequence *seq : strips_to_handle) {
SEQ_relations_invalidate_cache_raw(scene, seq);
}

115
source/blender/sequencer/intern/strip_retiming.cc
View File

@ -369,45 +369,73 @@ void SEQ_retiming_offset_transition_key(const Scene *scene,
key_end->retiming_factor -= corrected_offset * next_segment_step;
}
void SEQ_retiming_remove_multiple_keys(Sequence *seq, blender::Vector<SeqRetimingKey *> &keys)
static void seq_retiming_cleanup_freeze_frame(SeqRetimingKey *key)
{
if ((*keys.begin())->strip_frame_index == 0) {
keys.remove(0);
if ((key->flag & SEQ_FREEZE_FRAME_IN) != 0) {
SeqRetimingKey *next_key = key + 1;
key->flag &= ~SEQ_FREEZE_FRAME_IN;
next_key->flag &= ~SEQ_FREEZE_FRAME_OUT;
}
if (SEQ_retiming_is_last_key(seq, *keys.end())) {
keys.remove(keys.size() - 1);
if ((key->flag & SEQ_FREEZE_FRAME_OUT) != 0) {
SeqRetimingKey *previous_key = key - 1;
key->flag &= ~SEQ_FREEZE_FRAME_OUT;
previous_key->flag &= ~SEQ_FREEZE_FRAME_IN;
}
}
void SEQ_retiming_remove_multiple_keys(Sequence *seq,
blender::Vector<SeqRetimingKey *> &keys_to_remove)
{
/* Transitions need special treatment, so separate these from `keys_to_remove`. */
blender::Vector<SeqRetimingKey *> transitions;
/* Cleanup freeze frames and extract transition keys. */
for (SeqRetimingKey *key : keys_to_remove) {
if (SEQ_retiming_key_is_freeze_frame(key)) {
seq_retiming_cleanup_freeze_frame(key);
}
if ((key->flag & SEQ_SPEED_TRANSITION_IN) != 0) {
transitions.append_non_duplicates(key);
transitions.append_non_duplicates(key + 1);
}
if ((key->flag & SEQ_SPEED_TRANSITION_OUT) != 0) {
transitions.append_non_duplicates(key);
transitions.append_non_duplicates(key - 1);
}
}
/* Sanitize keys to be removed. */
keys_to_remove.remove_if([&](SeqRetimingKey *key) {
return key->strip_frame_index == 0 || SEQ_retiming_is_last_key(seq, key) ||
SEQ_retiming_key_is_transition_type(key);
});
const size_t keys_count = SEQ_retiming_keys_count(seq);
size_t new_keys_count = keys_count - keys.size();
size_t new_keys_count = keys_count - keys_to_remove.size() - transitions.size() / 2;
SeqRetimingKey *new_keys = (SeqRetimingKey *)MEM_callocN(new_keys_count * sizeof(SeqRetimingKey),
__func__);
int next_index_to_copy_from = 0;
int keys_copied = 0;
for (SeqRetimingKey *key : keys) {
const int key_index = key - seq->retiming_keys;
const int copy_num = key_index - next_index_to_copy_from;
memcpy(new_keys + keys_copied,
seq->retiming_keys + next_index_to_copy_from,
copy_num * sizeof(SeqRetimingKey));
keys_copied += copy_num;
next_index_to_copy_from = key_index + 1;
/* Copy keys to new array. */
for (SeqRetimingKey &key : SEQ_retiming_keys_get(seq)) {
/* Create key that was used to make transition in new array. */
if (transitions.contains(&key) && SEQ_retiming_key_is_transition_start(&key)) {
SeqRetimingKey *new_key = new_keys + keys_copied;
new_key->strip_frame_index = key.original_strip_frame_index;
new_key->retiming_factor = key.original_retiming_factor;
keys_copied++;
continue;
}
if (keys_to_remove.contains(&key) || transitions.contains(&key)) {
continue;
}
memcpy(new_keys + keys_copied, &key, sizeof(SeqRetimingKey));
keys_copied++;
}
/* Copy remaining keys. */
const int copy_num = seq->retiming_keys_num - next_index_to_copy_from;
memcpy(new_keys + keys_copied,
seq->retiming_keys + next_index_to_copy_from,
copy_num * sizeof(SeqRetimingKey));
MEM_freeN(seq->retiming_keys);
seq->retiming_keys = new_keys;
seq->retiming_keys_num = keys_copied + copy_num;
seq->retiming_keys_num = new_keys_count;
}
static void seq_retiming_remove_key_ex(Sequence *seq, SeqRetimingKey *key)
@ -416,6 +444,10 @@ static void seq_retiming_remove_key_ex(Sequence *seq, SeqRetimingKey *key)
return; /* First and last key can not be removed. */
}
if (SEQ_retiming_key_is_freeze_frame(key)) {
seq_retiming_cleanup_freeze_frame(key);
}
size_t keys_count = SEQ_retiming_keys_count(seq);
SeqRetimingKey *keys = (SeqRetimingKey *)MEM_callocN((keys_count - 1) * sizeof(SeqRetimingKey),
__func__);
@ -435,12 +467,17 @@ static SeqRetimingKey *seq_retiming_remove_transition(const Scene *scene,
Sequence *seq,
SeqRetimingKey *key)
{
const int orig_frame_index = key->original_strip_frame_index;
const float orig_retiming_factor = key->original_retiming_factor;
SeqRetimingKey *transition_start = key;
if ((key->flag & SEQ_SPEED_TRANSITION_OUT) != 0) {
transition_start = key - 1;
}
const int orig_frame_index = transition_start->original_strip_frame_index;
const float orig_retiming_factor = transition_start->original_retiming_factor;
/* Remove both keys defining transition. */
int key_index = SEQ_retiming_key_index_get(seq, key);
seq_retiming_remove_key_ex(seq, key);
int key_index = SEQ_retiming_key_index_get(seq, transition_start);
seq_retiming_remove_key_ex(seq, transition_start);
seq_retiming_remove_key_ex(seq, seq->retiming_keys + key_index);
/* Create original linear key. */
@ -452,28 +489,12 @@ static SeqRetimingKey *seq_retiming_remove_transition(const Scene *scene,
void SEQ_retiming_remove_key(const Scene *scene, Sequence *seq, SeqRetimingKey *key)
{
SeqRetimingKey *previous_key = key - 1;
if ((key->flag & SEQ_SPEED_TRANSITION_IN) != 0) {
if (SEQ_retiming_key_is_transition_type(key)) {
seq_retiming_remove_transition(scene, seq, key);
return;
}
if ((key->flag & SEQ_SPEED_TRANSITION_OUT) != 0) {
seq_retiming_remove_transition(scene, seq, previous_key);
return;
}
if ((key->flag & SEQ_FREEZE_FRAME_IN) != 0) {
SeqRetimingKey *next_key = key - 1;
key->flag &= ~SEQ_FREEZE_FRAME_IN;
next_key->flag &= ~SEQ_FREEZE_FRAME_OUT;
}
if ((key->flag & SEQ_FREEZE_FRAME_OUT) != 0) {
key->flag &= ~SEQ_FREEZE_FRAME_OUT;
previous_key->flag &= ~SEQ_FREEZE_FRAME_IN;
}
seq_retiming_remove_key_ex(seq, key);
}