From c4eab49b9c873b27683a32e8f99b8c4b9b1e0f87 Mon Sep 17 00:00:00 2001 From: Richard Antalik Date: Fri, 23 May 2025 06:51:36 +0200 Subject: [PATCH] Fix #135930: Strip can be added to locked or muted channel When strip is added, operator first finds "best" channel to place a strip in, then adds new strip to that channel and finally does check if the strip is overlapping other strips. If it does, it handles the overlap. To prevent adding strip to locked or muted channel, fix has to be done at 2 places: - `sequencer_generic_invoke_xy_guess_channel` which finds the "best" channel - `transform_seqbase_shuffle_ex` which handles overlap. Further check for free channels was added to all strip add operators. If there is no space above channel, that user selected, error is displayed and operator is cancelled. Note, that `transform_seqbase_shuffle_ex` is used only for resolving overlaps where strip position in time must stay constant, so it moves the strips in Y axis. This PR does not affect user selectable `overlap_mode`. Pull Request: https://projects.blender.org/blender/blender/pulls/136016 --- .../editors/space_sequencer/sequencer_add.cc | 153 +++++++++++++++++- .../space_sequencer/sequencer_drag_drop.cc | 1 + .../sequencer/intern/strip_transform.cc | 9 +- 3 files changed, 158 insertions(+), 5 deletions(-) diff --git a/source/blender/editors/space_sequencer/sequencer_add.cc b/source/blender/editors/space_sequencer/sequencer_add.cc index 0060d17a743..4c5dab06ddd 100644 --- a/source/blender/editors/space_sequencer/sequencer_add.cc +++ b/source/blender/editors/space_sequencer/sequencer_add.cc @@ -10,6 +10,7 @@ #include #include +#include "DNA_sequence_types.h" #include "MEM_guardedalloc.h" #include "BLI_listbase.h" @@ -30,6 +31,7 @@ #include "IMB_imbuf_enums.h" +#include "SEQ_channels.hh" #include "WM_api.hh" #include "WM_types.hh" @@ -144,6 +146,13 @@ static void sequencer_generic_props__internal(wmOperatorType *ot, int flag) "Use the overlap_mode tool settings to determine how to shuffle overlapping strips"); RNA_def_property_flag(prop, PROP_HIDDEN | PROP_SKIP_SAVE); + prop = RNA_def_boolean(ot->srna, + "skip_locked_or_muted_channels", + true, + "Skip Locked or Muted Channels", + "Add strips to muted or locked channels when adding movie strips"); + RNA_def_property_flag(prop, PROP_HIDDEN | PROP_SKIP_SAVE); + if (flag & SEQPROP_FIT_METHOD) { ot->prop = RNA_def_enum(ot->srna, "fit_method", @@ -187,6 +196,21 @@ static void sequencer_generic_invoke_path__internal(bContext *C, } } +static int find_unlocked_unmuted_channel(const Editing *ed, int channel_index) +{ + const ListBase *channels = seq::channels_displayed_get(ed); + + while (channel_index < seq::MAX_CHANNELS) { + SeqTimelineChannel *channel = seq::channel_get_by_index(channels, channel_index); + if (!seq::channel_is_muted(channel) && !seq::channel_is_locked(channel)) { + break; + } + channel_index++; + } + + return channel_index; +} + static int sequencer_generic_invoke_xy_guess_channel(bContext *C, int type) { Strip *tgt = nullptr; @@ -209,10 +233,52 @@ static int sequencer_generic_invoke_xy_guess_channel(bContext *C, int type) } } + int best_channel = 1; if (tgt) { - return (type == STRIP_TYPE_MOVIE) ? tgt->channel - 1 : tgt->channel; + best_channel = (type == STRIP_TYPE_MOVIE) ? tgt->channel - 1 : tgt->channel; } - return 1; + + best_channel = find_unlocked_unmuted_channel(ed, best_channel); + + return math::clamp(best_channel, 0, seq::MAX_CHANNELS); +} + +static bool have_free_channels(bContext *C, + wmOperator *op, + int need_channels, + const char **r_error_msg) +{ + const int channel = RNA_int_get(op->ptr, "channel"); + const int frame_start = RNA_int_get(op->ptr, "frame_start"); + + /* First check simple case - strip is added to very top of timeline. */ + const int max_channel = seq::MAX_CHANNELS - need_channels + 1; + if (channel > max_channel) { + *r_error_msg = RPT_("No available channel for the current frame."); + return false; + } + + /* When adding strip(s) to lower channels, we must count number of free channels. There can be + * gaps. */ + Set used_channels; + for (Strip *strip : all_strips_from_context(C)) { + if (seq::time_strip_intersects_frame(CTX_data_scene(C), strip, frame_start)) { + used_channels.add(strip->channel); + } + } + + int free_channels = 0; + for (int i : IndexRange(channel, seq::MAX_CHANNELS - channel + 1)) { + if (!used_channels.contains(i)) { + free_channels++; + } + if (free_channels == need_channels) { + return true; + } + } + + *r_error_msg = RPT_("No available channel for the current frame."); + return false; } /* Sets `channel` and `frame_start` properties when the operator is likely to have been invoked @@ -465,6 +531,12 @@ static wmOperatorStatus sequencer_add_scene_strip_exec(bContext *C, wmOperator * return OPERATOR_CANCELLED; } + const char *error_msg; + if (!have_free_channels(C, op, 1, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + if (RNA_boolean_get(op->ptr, "replace_sel")) { deselect_all_strips(scene); } @@ -560,6 +632,12 @@ static wmOperatorStatus sequencer_add_scene_strip_new_exec(bContext *C, wmOperat Scene *scene = CTX_data_scene(C); const Editing *ed = seq::editing_ensure(scene); + const char *error_msg; + if (!have_free_channels(C, op, 1, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + if (RNA_boolean_get(op->ptr, "replace_sel")) { deselect_all_strips(scene); } @@ -666,6 +744,12 @@ static wmOperatorStatus sequencer_add_movieclip_strip_exec(bContext *C, wmOperat return OPERATOR_CANCELLED; } + const char *error_msg; + if (!have_free_channels(C, op, 1, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + if (RNA_boolean_get(op->ptr, "replace_sel")) { deselect_all_strips(scene); } @@ -734,6 +818,12 @@ static wmOperatorStatus sequencer_add_mask_strip_exec(bContext *C, wmOperator *o return OPERATOR_CANCELLED; } + const char *error_msg; + if (!have_free_channels(C, op, 1, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + if (RNA_boolean_get(op->ptr, "replace_sel")) { deselect_all_strips(scene); } @@ -915,7 +1005,8 @@ static void sequencer_add_movie_multiple_strips(bContext *C, /* The video has sound, shift the video strip up a channel to make room for the sound * strip. */ added_strips.append(strip_sound); - seq::strip_channel_set(strip_movie, strip_movie->channel + 1); + seq::strip_channel_set(strip_movie, + find_unlocked_unmuted_channel(ed, strip_movie->channel + 1)); } } @@ -975,9 +1066,16 @@ static bool sequencer_add_movie_single_strip(bContext *C, if (strip_sound) { added_strips.append(strip_sound); + /* The video has sound, shift the video strip up a channel to make room for the sound * strip. */ - seq::strip_channel_set(strip_movie, strip_movie->channel + 1); + int movie_channel = strip_movie->channel + 1; + + if (RNA_boolean_get(op->ptr, "skip_locked_or_muted_channels")) { + movie_channel = find_unlocked_unmuted_channel(ed, strip_movie->channel + 1); + } + + seq::strip_channel_set(strip_movie, movie_channel); } } @@ -1020,6 +1118,14 @@ static wmOperatorStatus sequencer_add_movie_strip_exec(bContext *C, wmOperator * return OPERATOR_CANCELLED; } + sequencer_generic_invoke_xy__internal(C, op, 0, STRIP_TYPE_MOVIE); + + const char *error_msg; + if (!have_free_channels(C, op, 2, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + if (RNA_boolean_get(op->ptr, "replace_sel")) { deselect_all_strips(scene); } @@ -1089,6 +1195,13 @@ static wmOperatorStatus sequencer_add_movie_strip_invoke(bContext *C, RNA_struct_property_is_set(op->ptr, "filepath")) { sequencer_generic_invoke_xy__internal(C, op, SEQPROP_NOPATHS, STRIP_TYPE_MOVIE, event); + + const char *error_msg; + if (!have_free_channels(C, op, 2, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + return sequencer_add_movie_strip_exec(C, op); } @@ -1215,6 +1328,12 @@ static wmOperatorStatus sequencer_add_sound_strip_exec(bContext *C, wmOperator * seq::LoadData load_data; load_data_init_from_operator(&load_data, C, op); + const char *error_msg; + if (!have_free_channels(C, op, 1, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + if (RNA_boolean_get(op->ptr, "replace_sel")) { deselect_all_strips(scene); } @@ -1250,6 +1369,13 @@ static wmOperatorStatus sequencer_add_sound_strip_invoke(bContext *C, RNA_struct_property_is_set(op->ptr, "filepath")) { sequencer_generic_invoke_xy__internal(C, op, SEQPROP_NOPATHS, STRIP_TYPE_SOUND_RAM, event); + + const char *error_msg; + if (!have_free_channels(C, op, 1, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + return sequencer_add_sound_strip_exec(C, op); } @@ -1400,6 +1526,12 @@ static wmOperatorStatus sequencer_add_image_strip_exec(bContext *C, wmOperator * return OPERATOR_CANCELLED; } + const char *error_msg; + if (!have_free_channels(C, op, 1, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + int minframe, numdigits; load_data.image.len = sequencer_add_image_strip_calculate_length( op, load_data.start_frame, &minframe, &numdigits); @@ -1459,6 +1591,13 @@ static wmOperatorStatus sequencer_add_image_strip_invoke(bContext *C, if (RNA_struct_property_is_set(op->ptr, "files") && !RNA_collection_is_empty(op->ptr, "files")) { sequencer_generic_invoke_xy__internal( C, op, SEQPROP_ENDFRAME | SEQPROP_NOPATHS, STRIP_TYPE_IMAGE, event); + + const char *error_msg; + if (!have_free_channels(C, op, 1, &error_msg)) { + BKE_report(op->reports, RPT_ERROR, error_msg); + return OPERATOR_CANCELLED; + } + return sequencer_add_image_strip_exec(C, op); } @@ -1514,6 +1653,12 @@ static wmOperatorStatus sequencer_add_effect_strip_exec(bContext *C, wmOperator Scene *scene = CTX_data_scene(C); Editing *ed = seq::editing_ensure(scene); + const char *error; + if (!have_free_channels(C, op, 1, &error)) { + BKE_report(op->reports, RPT_ERROR, error); + return OPERATOR_CANCELLED; + } + seq::LoadData load_data; load_data_init_from_operator(&load_data, C, op); load_data.effect.type = RNA_enum_get(op->ptr, "type"); diff --git a/source/blender/editors/space_sequencer/sequencer_drag_drop.cc b/source/blender/editors/space_sequencer/sequencer_drag_drop.cc index 1b6f7444efb..05822b0d933 100644 --- a/source/blender/editors/space_sequencer/sequencer_drag_drop.cc +++ b/source/blender/editors/space_sequencer/sequencer_drag_drop.cc @@ -260,6 +260,7 @@ static void sequencer_drop_copy(bContext *C, wmDrag *drag, wmDropBox *drop) RNA_int_set(drop->ptr, "frame_start", g_drop_coords.start_frame); RNA_int_set(drop->ptr, "channel", g_drop_coords.channel); RNA_boolean_set(drop->ptr, "overlap_shuffle_override", true); + RNA_boolean_set(drop->ptr, "skip_locked_or_muted_channels", false); } else { /* We are dropped inside the preview region. Put the strip on top of the diff --git a/source/blender/sequencer/intern/strip_transform.cc b/source/blender/sequencer/intern/strip_transform.cc index 568bc3cab71..597f9352ad9 100644 --- a/source/blender/sequencer/intern/strip_transform.cc +++ b/source/blender/sequencer/intern/strip_transform.cc @@ -105,12 +105,19 @@ bool transform_seqbase_shuffle_ex(ListBase *seqbasep, BLI_assert(ELEM(channel_delta, -1, 1)); strip_channel_set(test, test->channel + channel_delta); - while (transform_test_overlap(evil_scene, seqbasep, test)) { + + const ListBase *channels = channels_displayed_get(editing_get(evil_scene)); + SeqTimelineChannel *channel = channel_get_by_index(channels, test->channel); + + while (transform_test_overlap(evil_scene, seqbasep, test) || channel_is_muted(channel) || + channel_is_locked(channel)) + { if ((channel_delta > 0) ? (test->channel >= MAX_CHANNELS) : (test->channel < 1)) { break; } strip_channel_set(test, test->channel + channel_delta); + channel = channel_get_by_index(channels, test->channel); } if (!is_valid_strip_channel(test)) {