Page MenuHome
Differential D6372 Diff 21552 source/blender/editors/space_sequencer/sequencer_select.c

Changeset View

Standalone View

View Options

source/blender/editors/space_sequencer/sequencer_select.c

Show First 20 Lines • Show All 1,012 Lines • ▼ Show 20 Lines RNA_def_enum(ot->srna,
"Side", "Side",
"The side to which the selection is applied"); "The side to which the selection is applied");
} }
/* box_select operator */ /* box_select operator */
static int sequencer_box_select_exec(bContext *C, wmOperator *op) static int sequencer_box_select_exec(bContext *C, wmOperator *op)
{ {
Scene *scene = CTX_data_scene(C); Scene *scene = CTX_data_scene(C);
View2D *v2d = UI_view2d_fromcontext(C);
Editing *ed = BKE_sequencer_editing_get(scene, false); Editing *ed = BKE_sequencer_editing_get(scene, false);
if (ed == NULL) { if (ed == NULL) {
return OPERATOR_CANCELLED; return OPERATOR_CANCELLED;
} }
View2D *v2d = UI_view2d_fromcontext(C);
const eSelectOp sel_op = RNA_enum_get(op->ptr, "mode"); const eSelectOp sel_op = RNA_enum_get(op->ptr, "mode");
const bool handles = RNA_boolean_get(op->ptr, "handles");
const bool select = (sel_op != SEL_OP_SUB); const bool select = (sel_op != SEL_OP_SUB);
if (SEL_OP_USE_PRE_DESELECT(sel_op)) { if (SEL_OP_USE_PRE_DESELECT(sel_op)) {
ED_sequencer_deselect_all(scene); ED_sequencer_deselect_all(scene);
} }
rctf rectf; rctf rectf;
WM_operator_properties_border_to_rctf(op, &rectf); WM_operator_properties_border_to_rctf(op, &rectf);
UI_view2d_region_to_view_rctf(v2d, &rectf, &rectf); UI_view2d_region_to_view_rctf(v2d, &rectf, &rectf);
for (Sequence *seq = ed->seqbasep->first; seq; seq = seq->next) { for (Sequence *seq = ed->seqbasep->first; seq; seq = seq->next) {
rctf rq; rctf rq;
seq_rectf(seq, &rq); seq_rectf(seq, &rq);
if (BLI_rctf_isect(&rq, &rectf, NULL)) { if (BLI_rctf_isect(&rq, &rectf, NULL)) {
if (handles) {
/* Get the handles draw size. */
float pixelx = BLI_rctf_size_x(&v2d->cur) / BLI_rcti_size_x(&v2d->mask);
float handsize = sequence_handle_size_get_clamped(seq, pixelx) * 0.75f;
ISSUnsubmitted
Not Done
Inline Actions

This 0.75f factor could be defined by some name.

ISS: This 0.75f factor could be defined by some name.
a.montiAuthorUnsubmitted
Not Done
Inline Actions

This comes from here: https://developer.blender.org/diffusion/B/browse/master/source/blender/editors/space_sequencer/sequencer_draw.c$482

The original handsize_clamped value is used to define whether to draw text and its position, and for the actual handles drawing it gets cropped to a square and used to get the correct proportions for the small triangles: https://developer.blender.org/diffusion/B/browse/master/source/blender/editors/space_sequencer/sequencer_draw.c$965

I don't honestly know how to define that 0.75 factor here in a meaningful way...

a.monti: This comes from here: https://developer.blender.org/diffusion/B/browse/master/source/blender/ed…
ISSUnsubmitted
Not Done
Inline Actions

When I read that line, I had to look elswhere in the code to see if same factor is used just to see what it means and still it wasn't obvious.

So if it is used as drawing aid it could be #define SEQ_HANDLE_SIZE_DRAW_FACTOR 0.75f with comment further explaining usage if not obvious.

ISS: When I read that line, I had to look elswhere in the code to see if same factor is used just to…
a.montiAuthorUnsubmitted
Not Done
Inline Actions

I think it could be better to just invert the logic in the drawing function, so that handsize_clamped is the correct draw size and you wouldn't need to divide it, and then you'd add a margin for the text drawing, like this: P1190

What do you think about it?

a.monti: I think it could be better to just invert the logic in the drawing function, so that…
/* Right handle. */
if (rectf.xmax > (seq->enddisp - handsize)) {
ISSUnsubmitted
Not Done
Inline Actions

if (box_rectf.xmax > right_handle_xmin) would be self-explanatory

same for left

ISS: `if (box_rectf.xmax > right_handle_xmin)` would be self-explanatory same for left
a.montiAuthorUnsubmitted
Not Done
Inline Actions

I've changed the selection box name to box_rectf and also the sequence one to seq_rectf (was called rq), but I'm not sure it's worth to define two floats that are going to be used only once.

What if the comments above were something like this?

/* Right Handle - Selection box extends past the right handle's xmin. */
/* Left Handle - Selection box starts before the left handle's xmax. */

Do you think it would be clear enough?

a.monti: I've changed the selection box name to `box_rectf` and also the sequence one to `seq_rectf`…
ISSUnsubmitted
Not Done
Inline Actions

From performance perspective, it likely doesn't matter here. If code reads better with those 2 floats, than it's an improvement.
My point was to make comments unnecessary if possible(reasonable) by making code more readable.

ISS: From performance perspective, it likely doesn't matter here. If code reads better with those 2…
a.montiAuthorUnsubmitted
Not Done
Inline Actions

Ok, I will add those then.

a.monti: Ok, I will add those then.
if (select) {
seq->flag |= SELECT | SEQ_RIGHTSEL;
}
else {
/* Deselect the strip if it's left with no handles selected. */
if ((seq->flag & SEQ_RIGHTSEL) && ((seq->flag & SEQ_LEFTSEL) == 0)) {
seq->flag &= ~SELECT;
}
seq->flag &= ~SEQ_RIGHTSEL;
}
}
/* Left handle. */
if (rectf.xmin < (seq->startdisp + handsize)) {
if (select) {
seq->flag |= SELECT | SEQ_LEFTSEL;
}
else {
/* Deselect the strip if it's left with no handles selected. */
if ((seq->flag & SEQ_LEFTSEL) && ((seq->flag & SEQ_RIGHTSEL) == 0)) {
seq->flag &= ~SELECT;
}
seq->flag &= ~SEQ_LEFTSEL;
}
}
}
/* Regular box selection. */
else {
SET_FLAG_FROM_TEST(seq->flag, select, SELECT); SET_FLAG_FROM_TEST(seq->flag, select, SELECT);
recurs_sel_seq(seq); seq->flag &= ~(SEQ_LEFTSEL | SEQ_RIGHTSEL);
}
} }
} }
ED_outliner_select_sync_from_sequence_tag(C); ED_outliner_select_sync_from_sequence_tag(C);
WM_event_add_notifier(C, NC_SCENE | ND_SEQUENCER | NA_SELECTED, scene); WM_event_add_notifier(C, NC_SCENE | ND_SEQUENCER | NA_SELECTED, scene);
return OPERATOR_FINISHED; return OPERATOR_FINISHED;
Show All 15 Lines if (tweak) {
} }
} }
return WM_gesture_box_invoke(C, op, event); return WM_gesture_box_invoke(C, op, event);
} }
void SEQUENCER_OT_select_box(wmOperatorType *ot) void SEQUENCER_OT_select_box(wmOperatorType *ot)
{ {
PropertyRNA *prop;
/* identifiers */ /* identifiers */
ot->name = "Box Select"; ot->name = "Box Select";
ot->idname = "SEQUENCER_OT_select_box"; ot->idname = "SEQUENCER_OT_select_box";
ot->description = "Select strips using box selection"; ot->description = "Select strips using box selection";
/* api callbacks */ /* api callbacks */
ot->invoke = sequencer_box_select_invoke; ot->invoke = sequencer_box_select_invoke;
ot->exec = sequencer_box_select_exec; ot->exec = sequencer_box_select_exec;
ot->modal = WM_gesture_box_modal; ot->modal = WM_gesture_box_modal;
ot->cancel = WM_gesture_box_cancel; ot->cancel = WM_gesture_box_cancel;
ot->poll = ED_operator_sequencer_active; ot->poll = ED_operator_sequencer_active;
/* flags */ /* flags */
ot->flag = OPTYPE_UNDO; ot->flag = OPTYPE_UNDO;
/* properties */ /* properties */
WM_operator_properties_gesture_box(ot); WM_operator_properties_gesture_box(ot);
WM_operator_properties_select_operation_simple(ot); WM_operator_properties_select_operation_simple(ot);
PropertyRNA *prop = RNA_def_boolean( prop = RNA_def_boolean(
ot->srna, "tweak", 0, "Tweak", "Operator has been activated using a tweak event"); ot->srna, "tweak", 0, "Tweak", "Operator has been activated using a tweak event");
RNA_def_property_flag(prop, PROP_SKIP_SAVE); RNA_def_property_flag(prop, PROP_SKIP_SAVE);
prop = RNA_def_boolean(ot->srna, "handles", 0, "Select Handles", "Select the strips' handles");
RNA_def_property_flag(prop, PROP_SKIP_SAVE);
} }
/* ****** Selected Grouped ****** */ /* ****** Selected Grouped ****** */
enum { enum {
SEQ_SELECT_GROUP_TYPE, SEQ_SELECT_GROUP_TYPE,
SEQ_SELECT_GROUP_TYPE_BASIC, SEQ_SELECT_GROUP_TYPE_BASIC,
SEQ_SELECT_GROUP_TYPE_EFFECT, SEQ_SELECT_GROUP_TYPE_EFFECT,
▲ Show 20 Lines • Show All 356 Lines • Show Last 20 Lines