VSE: improved handle tweaking #109522

Merged
Richard Antalik merged 30 commits from iss/blender:tweak-experiment into main 2024-06-03 23:17:51 +02:00
23 changed files with 737 additions and 59 deletions

View File

@ -237,6 +237,8 @@ const UserDef U_default = {
.statusbar_flag = STATUSBAR_SHOW_VERSION,
.file_preview_type = USER_FILE_PREVIEW_AUTO,
.sequencer_editor_flag = USER_SEQ_ED_SIMPLE_TWEAKING,
.runtime =
{
.is_dirty = 0,

View File

@ -3037,7 +3037,7 @@ def km_sequencer(params):
("transform.seq_slide", {"type": 'G', "value": 'PRESS'},
{"properties": [("view2d_edge_pan", True)]}),
("transform.seq_slide", {"type": params.select_mouse, "value": 'CLICK_DRAG'},
{"properties": [("view2d_edge_pan", True)]}),
{"properties": [("view2d_edge_pan", True), ("use_restore_handle_selection", True)]}),
("transform.transform", {"type": 'E', "value": 'PRESS'},
{"properties": [("mode", 'TIME_EXTEND')]}),
("marker.add", {"type": 'M', "value": 'PRESS'}, None),
@ -8841,10 +8841,11 @@ def km_3d_view_tool_sculpt_gpencil_select_lasso(params):
def km_sequencer_editor_tool_generic_select_timeline_rcs(params, fallback):
return [
("sequencer.select_handle", {"type": 'LEFTMOUSE', "value": 'PRESS'}, None),
*_template_items_change_frame(params),
# Frame change can be canceled if click happens on strip handle. In such case move the handle.
("transform.seq_slide", {"type": 'LEFTMOUSE', "value": 'PRESS'},
{"properties": [("view2d_edge_pan", True)]}),
{"properties": [("view2d_edge_pan", True), ("use_restore_handle_selection", True)]}),
]

View File

@ -535,6 +535,17 @@ class USERPREF_PT_edit_node_editor(EditingPanel, CenterAlignMixIn, Panel):
layout.prop(edit, "node_preview_resolution", text="Preview Resolution")
class USERPREF_PT_edit_sequence_editor(EditingPanel, CenterAlignMixIn, Panel):
bl_label = "Video Sequencer"
bl_options = {'DEFAULT_CLOSED'}
def draw_centered(self, context, layout):
prefs = context.preferences
edit = prefs.edit
layout.prop(edit, "use_sequencer_simplified_tweaking")
class USERPREF_PT_edit_misc(EditingPanel, CenterAlignMixIn, Panel):
bl_label = "Miscellaneous"
bl_options = {'DEFAULT_CLOSED'}
@ -2841,6 +2852,7 @@ classes = (
USERPREF_PT_edit_gpencil,
USERPREF_PT_edit_text_editor,
USERPREF_PT_edit_node_editor,
USERPREF_PT_edit_sequence_editor,
USERPREF_PT_edit_misc,
USERPREF_PT_animation_timeline,

View File

@ -29,7 +29,7 @@ extern "C" {
/* Blender file format version. */
#define BLENDER_FILE_VERSION BLENDER_VERSION
#define BLENDER_FILE_SUBVERSION 50
#define BLENDER_FILE_SUBVERSION 51
/* Minimum Blender version that supports reading file written with the current
* version. Older Blender versions will test this and cancel loading the file, showing a warning to

View File

@ -969,6 +969,10 @@ void blo_do_versions_userdef(UserDef *userdef)
userdef->statusbar_flag |= STATUSBAR_SHOW_EXTENSIONS_UPDATES;
}
if (!USER_VERSION_ATLEAST(402, 51)) {
userdef->sequencer_editor_flag |= USER_SEQ_ED_SIMPLE_TWEAKING;
}
/**
* Always bump subversion in BKE_blender_version.h when adding versioning
* code here, and wrap it inside a USER_VERSION_ATLEAST check.

View File

@ -225,6 +225,23 @@ static bool use_sequencer_snapping(bContext *C)
(snap_flag & SEQ_SNAP_CURRENT_FRAME_TO_STRIPS);
}
static bool sequencer_skip_for_handle_tweak(const bContext *C, const wmEvent *event)
{
if ((U.sequencer_editor_flag & USER_SEQ_ED_SIMPLE_TWEAKING) == 0) {
return false;
}
const Scene *scene = CTX_data_scene(C);
const View2D *v2d = UI_view2d_fromcontext(C);
float mouse_co[2];
UI_view2d_region_to_view(v2d, event->mval[0], event->mval[1], &mouse_co[0], &mouse_co[1]);
StripSelection selection = ED_sequencer_pick_strip_and_handle(scene, v2d, mouse_co);
return selection.handle != SEQ_HANDLE_NONE;
}
/* Modal Operator init */
static int change_frame_invoke(bContext *C, wmOperator *op, const wmEvent *event)
{
@ -233,6 +250,9 @@ static int change_frame_invoke(bContext *C, wmOperator *op, const wmEvent *event
if (CTX_wm_space_seq(C) != nullptr && region->regiontype == RGN_TYPE_PREVIEW) {
return OPERATOR_CANCELLED;
}
if (sequencer_skip_for_handle_tweak(C, event)) {
return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
}
/* Change to frame that mouse is over before adding modal handler,
* as user could click on a single frame (jump to frame) as well as

View File

@ -14,11 +14,19 @@ struct Scene;
struct Sequence;
struct SpaceSeq;
struct bContext;
struct View2D;
enum eSeqHandle {
SEQ_HANDLE_NONE,
SEQ_HANDLE_LEFT,
SEQ_HANDLE_RIGHT,
iss marked this conversation as resolved Outdated

You should not be mixing semantic of some of elements of enum. Doing so might seem that it saves some code, but in practice it leads to confusions situations when some flag might be directly used for something completely unrelated, but not the other flags (potentially even causing actual bugs later on, after some of the area got expanded).

You should not be mixing semantic of some of elements of enum. Doing so might seem that it saves some code, but in practice it leads to confusions situations when some flag might be directly used for something completely unrelated, but not the other flags (potentially even causing actual bugs later on, after some of the area got expanded).
SEQ_HANDLE_BOTH,
};
struct StripSelection {
Sequence *seq1 = nullptr;
iss marked this conversation as resolved Outdated

Since this is C++, you can benefit from the implicit default constructor:

struct StripSelection {
  Sequence *seq1 = nullptr;
  Sequence *seq2 = nullptr;
  eSeqHandle handle = SEQ_HANDLE_NONE;
};

This will allow making some code more reslitient and clear towards possible refactors in the future.

Since this is C++, you can benefit from the implicit default constructor: ``` struct StripSelection { Sequence *seq1 = nullptr; Sequence *seq2 = nullptr; eSeqHandle handle = SEQ_HANDLE_NONE; }; ``` This will allow making some code more reslitient and clear towards possible refactors in the future.
Sequence *seq2 = nullptr;
eSeqHandle handle = SEQ_HANDLE_NONE;
};
void ED_sequencer_select_sequence_single(Scene *scene, Sequence *seq, bool deselect_all);
@ -54,7 +62,6 @@ Sequence *ED_sequencer_special_preview_get();
void ED_sequencer_special_preview_set(bContext *C, const int mval[2]);
void ED_sequencer_special_preview_clear();
bool sequencer_retiming_mode_is_active(const bContext *C);
/**
* Returns collection with selected strips presented to user. If operation is done in preview,
* collection is limited to selected presented strips, that can produce image output at current
@ -64,5 +71,8 @@ bool sequencer_retiming_mode_is_active(const bContext *C);
* \return collection of strips (`Sequence`)
*/
blender::VectorSet<Sequence *> ED_sequencer_selected_strips_from_context(bContext *C);
bool ED_sequencer_can_select_handle(const Sequence *seq);
StripSelection ED_sequencer_pick_strip_and_handle(const struct Scene *scene,
const View2D *v2d,
float mouse_co[2]);
bool ED_sequencer_can_select_handle(const Scene *scene, const Sequence *seq, const View2D *v2d);
bool ED_sequencer_handle_is_selected(const Sequence *seq, eSeqHandle handle);

View File

@ -233,6 +233,7 @@ void SEQUENCER_OT_scene_frame_range_update(wmOperatorType *ot);
void SEQUENCER_OT_select_all(wmOperatorType *ot);
void SEQUENCER_OT_select(wmOperatorType *ot);
void SEQUENCER_OT_select_handle(wmOperatorType *ot);
void SEQUENCER_OT_select_side_of_frame(wmOperatorType *ot);
void SEQUENCER_OT_select_more(wmOperatorType *ot);
void SEQUENCER_OT_select_less(wmOperatorType *ot);
@ -329,6 +330,7 @@ SeqRetimingKey *retiming_mousover_key_get(const bContext *C, const int mval[2],
int left_fake_key_frame_get(const bContext *C, const Sequence *seq);
int right_fake_key_frame_get(const bContext *C, const Sequence *seq);
bool retiming_keys_are_visible(const SpaceSeq *sseq);
rctf seq_retiming_keys_box_get(const Scene *scene, const View2D *v2d, const Sequence *seq);
/* `sequencer_timeline_draw.cc` */
blender::Vector<Sequence *> sequencer_visible_strips_get(const bContext *C);

View File

@ -80,6 +80,7 @@ void sequencer_operatortypes()
/* `sequencer_select.cc` */
WM_operatortype_append(SEQUENCER_OT_select_all);
WM_operatortype_append(SEQUENCER_OT_select);
WM_operatortype_append(SEQUENCER_OT_select_handle);
WM_operatortype_append(SEQUENCER_OT_select_more);
WM_operatortype_append(SEQUENCER_OT_select_less);
WM_operatortype_append(SEQUENCER_OT_select_linked_pick);

View File

@ -100,7 +100,7 @@ static rctf strip_box_get(const Scene *scene, const View2D *v2d, const Sequence
/** Size in pixels. */
#define RETIME_KEY_MOUSEOVER_THRESHOLD (16.0f * UI_SCALE_FAC)
static rctf retiming_keys_box_get(const Scene *scene, const View2D *v2d, const Sequence *seq)
rctf seq_retiming_keys_box_get(const Scene *scene, const View2D *v2d, const Sequence *seq)
iss marked this conversation as resolved Outdated

Public functions should either be in the corresponding namespace. such as blender::ed::seq (which is more preferred way nowadays), or have an old-style prefix in the name. such as ED_seq_retiming_keys_box_get or sequencer_retiming_keys_box_get.

Even though the function is "intern" to the space_sequencer, it is public from the linkage perspective.

Public functions should either be in the corresponding namespace. such as `blender::ed::seq` (which is more preferred way nowadays), or have an old-style prefix in the name. such as `ED_seq_retiming_keys_box_get` or `sequencer_retiming_keys_box_get`. Even though the function is "intern" to the `space_sequencer`, it is public from the linkage perspective.

I guess I could use seq_ prefix I use in core intern functions. Will have to look up more details on namespaces. I have used them, but not sure how to declare these such that they won't end up creating mess.

To me it seems, that it would be better to add these functions to namespace all at once if this is preferred, so better to do that in separate patch.

I guess I could use `seq_` prefix I use in core intern functions. Will have to look up more details on namespaces. I have used them, but not sure how to declare these such that they won't end up creating mess. To me it seems, that it would be better to add these functions to namespace all at once if this is preferred, so better to do that in separate patch.
{
rctf rect = strip_box_get(scene, v2d, seq);
rect.ymax = KEY_CENTER + KEY_SIZE / 2;
@ -129,7 +129,7 @@ static bool retiming_fake_key_is_clicked(const bContext *C,
{
const View2D *v2d = UI_view2d_fromcontext(C);
rctf box = retiming_keys_box_get(CTX_data_scene(C), v2d, seq);
rctf box = seq_retiming_keys_box_get(CTX_data_scene(C), v2d, seq);
if (!BLI_rctf_isect_pt(&box, mval[0], mval[1])) {
return false;
}
@ -206,7 +206,7 @@ SeqRetimingKey *retiming_mousover_key_get(const bContext *C, const int mval[2],
const Scene *scene = CTX_data_scene(C);
const View2D *v2d = UI_view2d_fromcontext(C);
for (Sequence *seq : sequencer_visible_strips_get(C)) {
rctf box = retiming_keys_box_get(scene, v2d, seq);
rctf box = seq_retiming_keys_box_get(scene, v2d, seq);
if (!BLI_rctf_isect_pt(&box, mval[0], mval[1])) {
continue;
}

View File

@ -16,9 +16,11 @@
#include "BLI_ghash.h"
#include "BLI_math_geom.h"
#include "BLI_math_vector.h"
#include "BLI_math_vector_types.hh"
#include "BLI_utildefines.h"
#include "DNA_scene_types.h"
#include "DNA_space_types.h"
#include "BKE_context.hh"
#include "BKE_report.hh"
@ -54,6 +56,19 @@
/** \name Selection Utilities
* \{ */
iss marked this conversation as resolved Outdated

For now it is probably fine to have such utility, but in a longer term I don't think we should have something like this. Instead, it should become

const float2 region(RNA_int_get(op->ptr, "mouse_x"), RNA_int_get(op->ptr, "mouse_y"));
const float2 view = UI_view2d_region_to_view(v2d, region);
For now it is probably fine to have such utility, but in a longer term I don't think we should have something like this. Instead, it should become ``` const float2 region(RNA_int_get(op->ptr, "mouse_x"), RNA_int_get(op->ptr, "mouse_y")); const float2 view = UI_view2d_region_to_view(v2d, region); ```

That would be better, I agree. Will use this utility for this patch.

That would be better, I agree. Will use this utility for this patch.
class MouseCoords {
iss marked this conversation as resolved
Review

new code should really be using vectorized types like int2 and float2.

new code should really be using vectorized types like `int2` and `float2`.
public:
blender::int2 region;
blender::float2 view;
MouseCoords(const View2D *v2d, int x, int y)
{
region[0] = x;
region[1] = y;
UI_view2d_region_to_view(v2d, x, y, &view[0], &view[1]);
}
};
blender::VectorSet<Sequence *> all_strips_from_context(bContext *C)
{
Scene *scene = CTX_data_scene(C);
@ -331,7 +346,7 @@ Sequence *find_nearest_seq(const Scene *scene,
if (SEQ_transform_sequence_can_be_translated(seq)) {
/* Clamp handles to defined size in pixel space. */
handsize = 2.0f * sequence_handle_size_get_clamped(scene, seq, pixelx);
handsize = 4.0f * sequence_handle_size_get_clamped(scene, seq, pixelx);
displen = float(abs(SEQ_time_left_handle_frame_get(scene, seq) -
SEQ_time_right_handle_frame_get(scene, seq)));
@ -833,10 +848,25 @@ bool ED_sequencer_handle_is_selected(const Sequence *seq, eSeqHandle handle)
((handle == SEQ_HANDLE_RIGHT) && (seq->flag & SEQ_RIGHTSEL));
}
static bool element_already_selected(const Sequence *seq, const eSeqHandle handle_clicked)
static bool element_already_selected(const StripSelection selection)
{
return ((seq->flag & SELECT) && handle_clicked == SEQ_HANDLE_NONE) ||
ED_sequencer_handle_is_selected(seq, handle_clicked);
if (selection.seq1 == nullptr) {
return false;
}
const bool seq1_already_selected = ((selection.seq1->flag & SELECT) != 0);
if (selection.seq2 == nullptr) {
const bool handle_already_selected = ED_sequencer_handle_is_selected(selection.seq1,
selection.handle) ||
selection.handle == SEQ_HANDLE_NONE;
return seq1_already_selected && handle_already_selected;
}
const bool seq2_already_selected = ((selection.seq2->flag & SELECT) != 0);
const int seq1_handle = selection.seq1->flag & (SEQ_RIGHTSEL | SEQ_LEFTSEL);
const int seq2_handle = selection.seq2->flag & (SEQ_RIGHTSEL | SEQ_LEFTSEL);
/* Handles must be selected in XOR fashion, with `seq1` matching `handle_clicked`. */
const bool both_handles_selected = seq1_handle == selection.handle && seq2_handle != 0 &&
seq1_handle != seq2_handle;
return seq1_already_selected && seq2_already_selected && both_handles_selected;
}
static void sequencer_select_strip_impl(const Editing *ed,
@ -891,14 +921,188 @@ static void sequencer_select_strip_impl(const Editing *ed,
}
}
bool ED_sequencer_can_select_handle(const Sequence *seq)
/* Similar to `sequence_handle_size_get_clamped()` but allows for larger clickable area. */
static float clickable_handle_size_get(const Scene *scene, const Sequence *seq, const View2D *v2d)
{
if ((seq->type & SEQ_TYPE_EFFECT) && SEQ_effect_get_num_inputs(seq->type) > 0) {
const float pixelx = 1 / UI_view2d_scale_get_x(v2d);
const float strip_len = SEQ_time_right_handle_frame_get(scene, seq) -
SEQ_time_left_handle_frame_get(scene, seq);
return min_ff(15.0f * pixelx * U.pixelsize, strip_len / 4);
}
bool ED_sequencer_can_select_handle(const Scene *scene, const Sequence *seq, const View2D *v2d)
{
if (SEQ_effect_get_num_inputs(seq->type) > 0) {
return false;
}
int min_len = 25 * U.pixelsize;
if ((U.sequencer_editor_flag & USER_SEQ_ED_SIMPLE_TWEAKING) == 0) {
min_len = 15 * U.pixelsize;
}
const float pixelx = 1 / UI_view2d_scale_get_x(v2d);
const int strip_len = SEQ_time_right_handle_frame_get(scene, seq) -
SEQ_time_left_handle_frame_get(scene, seq);
if (strip_len / pixelx < min_len) {
iss marked this conversation as resolved Outdated
* r_left_handle = *r_body;
* r_right_handle = *r_body;
``` * r_left_handle = *r_body; * r_right_handle = *r_body; ```
return false;
}
return true;
}
static void strip_clickable_areas_get(const Scene *scene,
const Sequence *seq,
const View2D *v2d,
rctf *r_body,
rctf *r_left_handle,
rctf *r_right_handle)
{
seq_rectf(scene, seq, r_body);
*r_left_handle = *r_body;
*r_right_handle = *r_body;
const float handsize = clickable_handle_size_get(scene, seq, v2d);
BLI_rctf_pad(r_left_handle, handsize / 3, 0.0f);
BLI_rctf_pad(r_right_handle, handsize / 3, 0.0f);
r_left_handle->xmax = r_body->xmin + handsize;
r_right_handle->xmin = r_body->xmax - handsize;
BLI_rctf_pad(r_body, -handsize, 0.0f);
}
static rctf strip_clickable_area_get(const Scene *scene, const View2D *v2d, const Sequence *seq)
{
rctf body, left, right;
strip_clickable_areas_get(scene, seq, v2d, &body, &left, &right);
BLI_rctf_union(&body, &left);
BLI_rctf_union(&body, &right);
return body;
}
static float strip_to_frame_distance(const Scene *scene,
const View2D *v2d,
const Sequence *seq,
float timeline_frame)
{
iss marked this conversation as resolved Outdated

What is the problem with a small strip? Why is the sorting only done for the first 2 elements? And what is the issue with using std::sort ?

What is the problem with a small strip? Why is the sorting only done for the first 2 elements? And what is the issue with using `std::sort` ?

This should have been marked as TODO, will need to solve this.

The problem with small strips is, that Francesco requested to be able to select handle from outside of the strip. This means, that strip with < ~2px screenspace size could be between 2 larger strips. And this is not filtered out. But seems, that this would be easy to solve by actually checking its size.

This should have been marked as TODO, will need to solve this. The problem with small strips is, that Francesco requested to be able to select handle from outside of the strip. This means, that strip with < ~2px screenspace size could be between 2 larger strips. And this is not filtered out. But seems, that this would be easy to solve by actually checking its size.
rctf body, left, right;
strip_clickable_areas_get(scene, seq, v2d, &body, &left, &right);
return BLI_rctf_length_x(&body, timeline_frame);
}
/* Get strips that can be selected by click. */
static blender::Vector<Sequence *> mouseover_strips_sorted_get(const Scene *scene,
const View2D *v2d,
float mouse_co[2])
{
iss marked this conversation as resolved Outdated

Do we need to worry about the SEQ_time_start_frame_get(seq) / SEQ_time_content_end_frame_get(scene, seq) or can we limit this to just SEQ_time_left_handle_frame_get(scene, seq) / SEQ_time_right_handle_frame_get(scene, seq) ?

Do we need to worry about the `SEQ_time_start_frame_get(seq)` / `SEQ_time_content_end_frame_get(scene, seq)` or can we limit this to just `SEQ_time_left_handle_frame_get(scene, seq)` / `SEQ_time_right_handle_frame_get(scene, seq)` ?

Right, I copy-pasted this from timeline drawing. Strip start is used there for offset drawing... Don't need to do that here.

Right, I copy-pasted this from timeline drawing. Strip start is used there for offset drawing... Don't need to do that here.
Editing *ed = SEQ_editing_get(scene);
blender::Vector<Sequence *> strips;
LISTBASE_FOREACH (Sequence *, seq, ed->seqbasep) {
if (seq->machine != int(mouse_co[1])) {
continue;
}
if (SEQ_time_left_handle_frame_get(scene, seq) > v2d->cur.xmax) {
continue;
}
if (SEQ_time_right_handle_frame_get(scene, seq) < v2d->cur.xmin) {
continue;
}
if (!ED_sequencer_can_select_handle(scene, seq, v2d)) {
continue;
}
const rctf body = strip_clickable_area_get(scene, v2d, seq);
if (!BLI_rctf_isect_pt_v(&body, mouse_co)) {
continue;
}
strips.append(seq);
}
BLI_assert(strips.size() <= 2);
/* Ensure, that `seq1` is left strip and `seq2` right strip. */
iss marked this conversation as resolved Outdated

std::swap.

`std::swap`.
if (strips.size() == 2 && strip_to_frame_distance(scene, v2d, strips[0], mouse_co[0]) <
strip_to_frame_distance(scene, v2d, strips[1], mouse_co[0]))
{
std::swap(strips[0], strips[1]);
}
return strips;
}
iss marked this conversation as resolved Outdated

What exactly this function does?

What exactly this function does?

It checks if 2 handles next to each other are selected at conce.

It checks if 2 handles next to each other are selected at conce.
static bool strips_are_adjacent(const Scene *scene, const Sequence *seq1, const Sequence *seq2)
{
const int s1_left = SEQ_time_left_handle_frame_get(scene, seq1);
const int s1_right = SEQ_time_right_handle_frame_get(scene, seq1);
const int s2_left = SEQ_time_left_handle_frame_get(scene, seq2);
const int s2_right = SEQ_time_right_handle_frame_get(scene, seq2);
iss marked this conversation as resolved Outdated

get_strip_handle_under_cursor() ?

`get_strip_handle_under_cursor()` ?
return s1_right == s2_left || s1_left == s2_right;
}
static eSeqHandle get_strip_handle_under_cursor(const Scene *scene,
const Sequence *seq,
const View2D *v2d,
float mouse_co[2])
{
rctf body, left, right;
strip_clickable_areas_get(scene, seq, v2d, &body, &left, &right);
if (BLI_rctf_isect_pt_v(&left, mouse_co)) {
return SEQ_HANDLE_LEFT;
}
if (BLI_rctf_isect_pt_v(&right, mouse_co)) {
return SEQ_HANDLE_RIGHT;
}
iss marked this conversation as resolved Outdated

is_mouse_over_both_handles_of_adjacent_strips. While it might seen lengthy, it actually shows clear intent of what is going on: makes it unambiguous what are the handles which are checked, and also clarifies that check is based on coordinates, and not on selection flags.

`is_mouse_over_both_handles_of_adjacent_strips`. While it might seen lengthy, it actually shows clear intent of what is going on: makes it unambiguous what are the handles which are checked, and also clarifies that check is based on coordinates, and not on selection flags.
return SEQ_HANDLE_NONE;
}
iss marked this conversation as resolved Outdated

seq1_handle. But is also wierd to require passing one of the handles. Might as well just re-compute it here and simplify the function signature without introducing much penalty to hot code paths?

`seq1_handle`. But is also wierd to require passing one of the handles. Might as well just re-compute it here and simplify the function signature without introducing much penalty to hot code paths?
static bool is_mouse_over_both_handles_of_adjacent_strips(const Scene *scene,
blender::Vector<Sequence *> strips,
const View2D *v2d,
float mouse_co[2])
iss marked this conversation as resolved Outdated

This seems to be quite low-level function to perform such a check. Such branching in behavior should be done on a higher level.

You can easily see it from trying to summarize what function does: check whether adjacent handles are selected, but only if the user preference is not the simple tweaking.

This seems to be quite low-level function to perform such a check. Such branching in behavior should be done on a higher level. You can easily see it from trying to summarize what function does: check whether adjacent handles are selected, but only if the user preference is not the simple tweaking.

This seems to be quite low-level function to perform such a check. Such branching in behavior should be done on a higher level.

You can easily see it from trying to summarize what function does: check whether adjacent handles are selected, but only if the user preference is not the simple tweaking.

This would mean, that flags could never be used as preconditions, which can result in unreadable code. As example you can look at draw_seq_strip() before and after in 4d668e6825
Originally this function was meant to return whether both handles should be selected, so to me it did make sense to make the check here.

> This seems to be quite low-level function to perform such a check. Such branching in behavior should be done on a higher level. > > You can easily see it from trying to summarize what function does: check whether adjacent handles are selected, but only if the user preference is not the simple tweaking. This would mean, that flags could never be used as preconditions, which can result in unreadable code. As example you can look at `draw_seq_strip()` before and after in 4d668e6825ac35b189b1bc4699aa1a43b42fcc03 Originally this function was meant to return whether both handles should be selected, so to me it did make sense to make the check here.

I am not sure what you mean by preconditions. Having check deep inside a code path is exactly opposite of precondition.

It is also quite confusing when all inlined comments are immediately marked as resolved, without stating a resolution in them. From the comment it is unclear whether something got changed in the code inspired by the comment, or is it still open discussion.

I am not sure what you mean by preconditions. Having check deep inside a code path is exactly opposite of precondition. It is also quite confusing when all inlined comments are immediately marked as resolved, without stating a resolution in them. From the comment it is unclear whether something got changed in the code inspired by the comment, or is it still open discussion.

I am not sure what you mean by preconditions. Having check deep inside a code path is exactly opposite of precondition.

Perhaps early return would be better term here, but I have moved the check where it is necessary to be.

It is also quite confusing when all inlined comments are immediately marked as resolved, without stating a resolution in them. From the comment it is unclear whether something got changed in the code inspired by the comment, or is it still open discussion.

What do you suggest? To me marking inline as resolved means agreement with proposed solution. If no solution is proposed I would probably mention what I have done.

When I do review, I do go over resolved inlines, and look at the code anyway. I must say, that this was much easier with phabricator, where you could jump between multiple "revisions".

> I am not sure what you mean by preconditions. Having check deep inside a code path is exactly opposite of precondition. Perhaps early return would be better term here, but I have moved the check where it is necessary to be. > It is also quite confusing when all inlined comments are immediately marked as resolved, without stating a resolution in them. From the comment it is unclear whether something got changed in the code inspired by the comment, or is it still open discussion. What do you suggest? To me marking inline as resolved means agreement with proposed solution. If no solution is proposed I would probably mention what I have done. When I do review, I do go over resolved inlines, and look at the code anyway. I must say, that this was much easier with phabricator, where you could jump between multiple "revisions".

If you simply address feedback without providing further information it is fine to mark conversation as resolved.
If you write new information in the comment, leave the conversation open, so it is easier to go over all open conversations.

If you simply address feedback without providing further information it is fine to mark conversation as resolved. If you write new information in the comment, leave the conversation open, so it is easier to go over all open conversations.
{
const eSeqHandle seq1_handle = get_strip_handle_under_cursor(scene, strips[0], v2d, mouse_co);
if (seq1_handle == SEQ_HANDLE_NONE) {
return false;
}
if (!strips_are_adjacent(scene, strips[0], strips[1])) {
return false;
}
iss marked this conversation as resolved Outdated

eSeqHandle seq2_handle.

`eSeqHandle seq2_handle`.
const eSeqHandle seq2_handle = get_strip_handle_under_cursor(scene, strips[1], v2d, mouse_co);
if (seq1_handle == SEQ_HANDLE_RIGHT && seq2_handle != SEQ_HANDLE_LEFT) {
return false;
}
else if (seq1_handle == SEQ_HANDLE_LEFT && seq2_handle != SEQ_HANDLE_RIGHT) {
return false;
}
return true;
}
StripSelection ED_sequencer_pick_strip_and_handle(const Scene *scene,
const View2D *v2d,
float mouse_co[2])
{
blender::Vector<Sequence *> strips = mouseover_strips_sorted_get(scene, v2d, mouse_co);
StripSelection selection;
if (strips.size() == 0) {
return selection;
}
selection.seq1 = strips[0];
selection.handle = get_strip_handle_under_cursor(scene, selection.seq1, v2d, mouse_co);
if (strips.size() == 2 && (U.sequencer_editor_flag & USER_SEQ_ED_SIMPLE_TWEAKING) != 0 &&
is_mouse_over_both_handles_of_adjacent_strips(scene, strips, v2d, mouse_co))
{
selection.seq2 = strips[1];
}
return selection;
}
static bool use_retiming_mode(const bContext *C, const Sequence *seq_key_test)
{
return seq_key_test && SEQ_retiming_data_is_editable(seq_key_test) &&
@ -907,7 +1111,7 @@ static bool use_retiming_mode(const bContext *C, const Sequence *seq_key_test)
int sequencer_select_exec(bContext *C, wmOperator *op)
{
View2D *v2d = UI_view2d_fromcontext(C);
const View2D *v2d = UI_view2d_fromcontext(C);
Scene *scene = CTX_data_scene(C);
Editing *ed = SEQ_editing_get(scene);
ARegion *region = CTX_wm_region(C);
@ -936,25 +1140,24 @@ int sequencer_select_exec(bContext *C, wmOperator *op)
bool toggle = RNA_boolean_get(op->ptr, "toggle");
bool center = RNA_boolean_get(op->ptr, "center");
iss marked this conversation as resolved Outdated

Use constructor directly:

MouseCoords mouse_co(v2d, RNA_int_get(op->ptr, "mouse_x"), RNA_int_get(op->ptr, "mouse_y"));

Should also use const to make things even better.

Use constructor directly: ``` MouseCoords mouse_co(v2d, RNA_int_get(op->ptr, "mouse_x"), RNA_int_get(op->ptr, "mouse_y")); ``` Should also use `const` to make things even better.
int mval[2];
mval[0] = RNA_int_get(op->ptr, "mouse_x");
mval[1] = RNA_int_get(op->ptr, "mouse_y");
MouseCoords mouse_co(v2d, RNA_int_get(op->ptr, "mouse_x"), RNA_int_get(op->ptr, "mouse_y"));
eSeqHandle handle_clicked = SEQ_HANDLE_NONE;
Sequence *seq = nullptr;
StripSelection selection;
if (region->regiontype == RGN_TYPE_PREVIEW) {
seq = seq_select_seq_from_preview(C, mval, toggle, extend, center);
selection.seq1 = seq_select_seq_from_preview(C, mouse_co.region, toggle, extend, center);
}
else {
seq = find_nearest_seq(scene, v2d, mval, &handle_clicked);
selection = ED_sequencer_pick_strip_and_handle(scene, v2d, mouse_co.view);
iss marked this conversation as resolved Outdated

With the current code you're leaving seq2 and handle uninitialized. The proposal around StripSelection solves this.

With the current code you're leaving `seq2` and `handle` uninitialized. The proposal around `StripSelection` solves this.
}
/* NOTE: `side_of_frame` and `linked_time` functionality is designed to be shared on one
* keymap, therefore both properties can be true at the same time. */
Sequence *seq_key_test = nullptr;
SeqRetimingKey *key = retiming_mousover_key_get(C, mval, &seq_key_test);
SeqRetimingKey *key = retiming_mousover_key_get(C, mouse_co.region, &seq_key_test);
/* NOTE: `side_of_frame` and `linked_time` functionality is designed to be shared on one keymap,
* therefore both properties can be true at the same time. */
if (seq && RNA_boolean_get(op->ptr, "linked_time")) {
/* NOTE: `side_of_frame` and `linked_time` functionality is designed to be shared on one
* keymap, therefore both properties can be true at the same time. */
if (selection.seq1 && RNA_boolean_get(op->ptr, "linked_time")) {
if (use_retiming_mode(C, seq_key_test)) {
return sequencer_retiming_select_linked_time(C, op);
}
@ -962,10 +1165,10 @@ int sequencer_select_exec(bContext *C, wmOperator *op)
if (!extend && !toggle) {
ED_sequencer_deselect_all(scene);
}
sequencer_select_strip_impl(ed, seq, handle_clicked, extend, deselect, toggle);
select_linked_time(scene, ed->seqbasep, seq);
sequencer_select_strip_impl(ed, selection.seq1, selection.handle, extend, deselect, toggle);
select_linked_time(scene, ed->seqbasep, selection.seq1);
sequencer_select_do_updates(C, scene);
sequencer_select_set_active(scene, seq);
sequencer_select_set_active(scene, selection.seq1);
return OPERATOR_FINISHED;
}
}
@ -975,27 +1178,36 @@ int sequencer_select_exec(bContext *C, wmOperator *op)
if (!extend && !toggle) {
ED_sequencer_deselect_all(scene);
}
sequencer_select_side_of_frame(C, v2d, mval, scene);
sequencer_select_side_of_frame(C, v2d, mouse_co.region, scene);
sequencer_select_do_updates(C, scene);
return OPERATOR_FINISHED;
}
/* On Alt selection, select the strip and bordering handles. */
if (seq && RNA_boolean_get(op->ptr, "linked_handle")) {
if (selection.seq1 && RNA_boolean_get(op->ptr, "linked_handle")) {
if (!extend && !toggle) {
ED_sequencer_deselect_all(scene);
}
sequencer_select_linked_handle(C, seq, handle_clicked);
sequencer_select_linked_handle(C, selection.seq1, selection.handle);
sequencer_select_do_updates(C, scene);
sequencer_select_set_active(scene, seq);
sequencer_select_set_active(scene, selection.seq1);
return OPERATOR_FINISHED;
}
const bool wait_to_deselect_others = RNA_boolean_get(op->ptr, "wait_to_deselect_others");
const bool already_selected = element_already_selected(selection);
SpaceSeq *sseq = CTX_wm_space_seq(C);
if (selection.handle != SEQ_HANDLE_NONE && already_selected) {
sseq->flag &= ~SPACE_SEQ_DESELECT_STRIP_HANDLE;
}
else {
sseq->flag |= SPACE_SEQ_DESELECT_STRIP_HANDLE;
}
/* Clicking on already selected element falls on modal operation.
* All strips are deselected on mouse button release unless extend mode is used. */
if (seq && element_already_selected(seq, handle_clicked) && wait_to_deselect_others && !toggle) {
if (already_selected && wait_to_deselect_others && !toggle) {
return OPERATOR_RUNNING_MODAL;
}
@ -1003,7 +1215,7 @@ int sequencer_select_exec(bContext *C, wmOperator *op)
/* Realize "fake" key, if it is clicked on. */
if (key == nullptr && seq_key_test != nullptr) {
key = try_to_realize_virtual_keys(C, seq_key_test, mval);
key = try_to_realize_virtual_keys(C, seq_key_test, mouse_co.region);
}
bool retiming_key_clicked = (key != nullptr);
@ -1020,12 +1232,15 @@ int sequencer_select_exec(bContext *C, wmOperator *op)
bool changed = false;
/* Deselect everything */
if (deselect_all || (seq && (extend == false && deselect == false && toggle == false))) {
if (deselect_all ||
(selection.seq1 && (extend == false && deselect == false && toggle == false) &&
!already_selected))
{
changed |= ED_sequencer_deselect_all(scene);
}
/* Nothing to select, but strips could be deselected. */
if (!seq) {
if (!selection.seq1) {
if (changed) {
sequencer_select_do_updates(C, scene);
}
@ -1033,10 +1248,16 @@ int sequencer_select_exec(bContext *C, wmOperator *op)
}
/* Do actual selection. */
sequencer_select_strip_impl(ed, seq, handle_clicked, extend, deselect, toggle);
sequencer_select_strip_impl(ed, selection.seq1, selection.handle, extend, deselect, toggle);
if (selection.seq2 != nullptr) {
/* Invert handle selection for second strip */
eSeqHandle seq2_handle_clicked = (selection.handle == SEQ_HANDLE_LEFT) ? SEQ_HANDLE_RIGHT :
SEQ_HANDLE_LEFT;
sequencer_select_strip_impl(ed, selection.seq2, seq2_handle_clicked, extend, deselect, toggle);
}
sequencer_select_do_updates(C, scene);
sequencer_select_set_active(scene, seq);
sequencer_select_set_active(scene, selection.seq1);
return OPERATOR_FINISHED;
}
@ -1104,6 +1325,101 @@ void SEQUENCER_OT_select(wmOperatorType *ot)
/** \} */
/* -------------------------------------------------------------------- */
/** \name Select Handle Operator
* \{ */
static int sequencer_select_handle_exec(bContext *C, wmOperator *op)
{
const View2D *v2d = UI_view2d_fromcontext(C);
Scene *scene = CTX_data_scene(C);
Editing *ed = SEQ_editing_get(scene);
iss marked this conversation as resolved Outdated

Same as above.

Same as above.
if (ed == nullptr) {
return OPERATOR_CANCELLED;
}
if ((U.sequencer_editor_flag & USER_SEQ_ED_SIMPLE_TWEAKING) == 0) {
return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
}
if (sequencer_retiming_mode_is_active(C) && retiming_keys_are_visible(CTX_wm_space_seq(C))) {
return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
}
MouseCoords mouse_co(v2d, RNA_int_get(op->ptr, "mouse_x"), RNA_int_get(op->ptr, "mouse_y"));
StripSelection selection = ED_sequencer_pick_strip_and_handle(scene, v2d, mouse_co.view);
iss marked this conversation as resolved Outdated

The variable is not used (it is only assigned to, never read back).

The variable is not used (it is only assigned to, never read back).
if (selection.seq1 == nullptr || selection.handle == SEQ_HANDLE_NONE) {
return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
}
/* Ignore clicks on retiming keys. */
Sequence *seq_key_test = nullptr;
retiming_mousover_key_get(C, mouse_co.region, &seq_key_test);
if (use_retiming_mode(C, seq_key_test) && seq_key_test != nullptr) {
return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
}
SpaceSeq *sseq = CTX_wm_space_seq(C);
if (element_already_selected(selection)) {
sseq->flag &= ~SPACE_SEQ_DESELECT_STRIP_HANDLE;
return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
}
else {
sseq->flag |= SPACE_SEQ_DESELECT_STRIP_HANDLE;
ED_sequencer_deselect_all(scene);
}
/* Do actual selection. */
sequencer_select_strip_impl(ed, selection.seq1, selection.handle, false, false, false);
if (selection.seq2 != nullptr) {
/* Invert handle selection for second strip */
eSeqHandle seq2_handle_clicked = (selection.handle == SEQ_HANDLE_LEFT) ? SEQ_HANDLE_RIGHT :
SEQ_HANDLE_LEFT;
sequencer_select_strip_impl(ed, selection.seq2, seq2_handle_clicked, false, false, false);
}
sequencer_select_do_updates(C, scene);
sequencer_select_set_active(scene, selection.seq1);
return OPERATOR_FINISHED | OPERATOR_PASS_THROUGH;
}
static int sequencer_select_handle_invoke(bContext *C, wmOperator *op, const wmEvent *event)
{
ARegion *region = CTX_wm_region(C);
int mval[2];
WM_event_drag_start_mval(event, region, mval);
RNA_int_set(op->ptr, "mouse_x", mval[0]);
RNA_int_set(op->ptr, "mouse_y", mval[1]);
return sequencer_select_handle_exec(C, op);
}
void SEQUENCER_OT_select_handle(wmOperatorType *ot)
{
/* Identifiers. */
ot->name = "Select Handle";
ot->idname = "SEQUENCER_OT_select_handle";
ot->description = "Select strip handle";
/* Api callbacks. */
ot->exec = sequencer_select_handle_exec;
ot->invoke = sequencer_select_handle_invoke;
ot->poll = ED_operator_sequencer_active;
/* Flags. */
ot->flag = OPTYPE_UNDO;
/* Properties. */
WM_operator_properties_generic_select(ot);
}
/** \} */
/* -------------------------------------------------------------------- */
/** \name Select More Operator
* \{ */
@ -1715,7 +2031,7 @@ static int sequencer_box_select_exec(bContext *C, wmOperator *op)
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(scene, seq, pixelx);
float handsize = sequence_handle_size_get_clamped(scene, seq, pixelx) * 4;
/* Right handle. */
if (rectf.xmax > (SEQ_time_right_handle_frame_get(scene, seq) - handsize)) {
@ -1770,7 +2086,7 @@ static int sequencer_box_select_exec(bContext *C, wmOperator *op)
static int sequencer_box_select_invoke(bContext *C, wmOperator *op, const wmEvent *event)
{
Scene *scene = CTX_data_scene(C);
View2D *v2d = &CTX_wm_region(C)->v2d;
const View2D *v2d = UI_view2d_fromcontext(C);
ARegion *region = CTX_wm_region(C);
if (region->regiontype == RGN_TYPE_PREVIEW && !sequencer_view_preview_only_poll(C)) {
@ -1780,11 +2096,14 @@ static int sequencer_box_select_invoke(bContext *C, wmOperator *op, const wmEven
const bool tweak = RNA_boolean_get(op->ptr, "tweak");
if (tweak) {
eSeqHandle hand_dummy;
int mval[2];
float mouse_co[2];
WM_event_drag_start_mval(event, region, mval);
Sequence *seq = find_nearest_seq(scene, v2d, mval, &hand_dummy);
if (seq != nullptr) {
UI_view2d_region_to_view(v2d, mval[0], mval[1], &mouse_co[0], &mouse_co[1]);
StripSelection selection = ED_sequencer_pick_strip_and_handle(scene, v2d, mouse_co);
if (selection.seq1 != nullptr) {
return OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH;
}
}

View File

@ -64,7 +64,6 @@
#include "sequencer_intern.hh"
#include "sequencer_quads_batch.hh"
#define SEQ_HANDLE_SIZE 8.0f
#define MUTE_ALPHA 120
constexpr float MISSING_ICON_SIZE = 16.0f;
@ -758,7 +757,9 @@ static void draw_handle_transform_text(const TimelineDrawContext *timeline_ctx,
float sequence_handle_size_get_clamped(const Scene *scene, Sequence *seq, const float pixelx)
{
const float maxhandle = (pixelx * SEQ_HANDLE_SIZE) * U.pixelsize;
const bool use_thin_handle = (U.sequencer_editor_flag & USER_SEQ_ED_SIMPLE_TWEAKING) != 0;
const float handle_size = use_thin_handle ? 5.0f : 8.0f;
const float maxhandle = (pixelx * handle_size) * U.pixelsize;
/* Ensure that handle is not wider, than quarter of strip. */
return min_ff(maxhandle,
@ -767,21 +768,28 @@ float sequence_handle_size_get_clamped(const Scene *scene, Sequence *seq, const
4.0f));
}
/* Draw a handle, on left or right side of strip. */
static void draw_seq_handle(TimelineDrawContext *timeline_ctx,
static void draw_seq_handle(const TimelineDrawContext *timeline_ctx,
const StripDrawContext *strip_ctx,
eSeqHandle handle)
{
const Sequence *seq = strip_ctx->seq;
const bool show_handles = (U.sequencer_editor_flag & USER_SEQ_ED_SIMPLE_TWEAKING) == 0;
const bool strip_selected = (seq->flag & SELECT) != 0;
const bool handle_selected = ED_sequencer_handle_is_selected(seq, handle);
if ((!strip_selected || !handle_selected) && !show_handles) {
return;
}
if (SEQ_transform_is_locked(timeline_ctx->channels, seq)) {
return;
}
if (!ED_sequencer_can_select_handle(seq)) {
if ((seq->type & SEQ_TYPE_EFFECT) && SEQ_effect_get_num_inputs(seq->type) > 0) {
return;
}
if (!ED_sequencer_can_select_handle(timeline_ctx->scene, seq, timeline_ctx->v2d)) {
return;
}
uchar col[4];
if (strip_selected && handle_selected && seq == SEQ_select_active_get(timeline_ctx->scene)) {
UI_GetThemeColor4ubv(TH_SEQ_ACTIVE, col);

View File

@ -30,6 +30,7 @@
#include "ED_markers.hh"
#include "ED_screen.hh"
#include "ED_sequencer.hh"
#include "ED_space_api.hh"
#include "ED_time_scrub_ui.hh"
#include "ED_transform.hh"
@ -39,6 +40,7 @@
#include "WM_api.hh"
#include "WM_message.hh"
#include "SEQ_retiming.hh"
#include "SEQ_sequencer.hh"
#include "SEQ_time.hh"
#include "SEQ_transform.hh"
@ -645,6 +647,92 @@ static void sequencer_main_region_message_subscribe(const wmRegionMessageSubscri
}
}
static bool is_mouse_over_retiming_key(const Scene *scene,
iss marked this conversation as resolved Outdated

is_mouse_over_retiming_key ?

`is_mouse_over_retiming_key ` ?
const Sequence *seq,
const View2D *v2d,
const ScrArea *area,
float mouse_co_region[2])
{
const SpaceSeq *sseq = static_cast<SpaceSeq *>(area->spacedata.first);
if (!SEQ_retiming_data_is_editable(seq) || !retiming_keys_are_visible(sseq)) {
return false;
}
rctf retiming_keys_box = seq_retiming_keys_box_get(scene, v2d, seq);
return BLI_rctf_isect_pt_v(&retiming_keys_box, mouse_co_region);
}
static void sequencer_main_cursor(wmWindow *win, ScrArea *area, ARegion *region)
{
int wmcursor = WM_CURSOR_DEFAULT;
const bToolRef *tref = area->runtime.tool;
if (!STREQ(tref->idname, "builtin.select")) {
WM_cursor_set(win, wmcursor);
return;
}
rcti scrub_rect = region->winrct;
scrub_rect.ymin = scrub_rect.ymax - UI_TIME_SCRUB_MARGIN_Y;
if (BLI_rcti_isect_pt_v(&scrub_rect, win->eventstate->xy)) {
WM_cursor_set(win, wmcursor);
return;
}
if ((U.sequencer_editor_flag & USER_SEQ_ED_SIMPLE_TWEAKING) == 0) {
WM_cursor_set(win, wmcursor);
return;
}
float mouse_co_region[2] = {float(win->eventstate->xy[0] - region->winrct.xmin),
float(win->eventstate->xy[1] - region->winrct.ymin)};
float mouse_co_view[2];
UI_view2d_region_to_view(
&region->v2d, mouse_co_region[0], mouse_co_region[1], &mouse_co_view[0], &mouse_co_view[1]);
const Scene *scene = win->scene;
const Editing *ed = SEQ_editing_get(scene);
if (ed == NULL) {
WM_cursor_set(win, wmcursor);
return;
}
StripSelection selection = ED_sequencer_pick_strip_and_handle(
scene, &region->v2d, mouse_co_view);
iss marked this conversation as resolved Outdated

The way how currently this function works, it will create a temporary array of all visible strips (which could be 100s) on every mouse move. It is not something we should be accepting easily.

There are easy ways of solving this, having minimal code duplication:

static blender::Vector<Sequence *> mouseover_strips_sorted_get(const Scene *scene,
                                                               const View2D *v2d,
                                                               float mouse_co[2])
{
  blender::Vector<Sequence *> strips;
  LISTBASE_FOREACH (Sequence *, seq, ed->seqbasep) {
    if (!is_strip_visible(v2d, seq)) {
      continue;
    }
    const rctf body = strip_clickable_area_get(scene, v2d, seq);
    if (!BLI_rctf_isect_pt_v(&body, mouse_co)) {
      continue;
    }
    strips.append(seq);
  }
  // sort. or even do sorting while a
  return strips;
}

Can even optimize it further by pre-calcualting channel from the mouse, doing an early output for strips outside of that channel.

The way how currently this function works, it will create a temporary array of all visible strips (which could be 100s) on every mouse move. It is not something we should be accepting easily. There are easy ways of solving this, having minimal code duplication: ```Cpp static blender::Vector<Sequence *> mouseover_strips_sorted_get(const Scene *scene, const View2D *v2d, float mouse_co[2]) { blender::Vector<Sequence *> strips; LISTBASE_FOREACH (Sequence *, seq, ed->seqbasep) { if (!is_strip_visible(v2d, seq)) { continue; } const rctf body = strip_clickable_area_get(scene, v2d, seq); if (!BLI_rctf_isect_pt_v(&body, mouse_co)) { continue; } strips.append(seq); } // sort. or even do sorting while a return strips; } ``` Can even optimize it further by pre-calcualting channel from the mouse, doing an early output for strips outside of that channel.
if (selection.seq1 == nullptr) {
WM_cursor_set(win, wmcursor);
return;
}
if (is_mouse_over_retiming_key(scene, selection.seq1, &region->v2d, area, mouse_co_region)) {
WM_cursor_set(win, wmcursor);
return;
}
const View2D *v2d = &region->v2d;
const float scale_y = UI_view2d_scale_get_y(v2d);
if (!ED_sequencer_can_select_handle(scene, selection.seq1, v2d) || scale_y < 16 * U.pixelsize) {
WM_cursor_set(win, wmcursor);
return;
}
if (selection.seq1 != nullptr && selection.seq2 != nullptr) {
wmcursor = WM_CURSOR_BOTH_HANDLES;
}
else if (selection.handle == SEQ_HANDLE_LEFT) {
wmcursor = WM_CURSOR_LEFT_HANDLE;
}
else if (selection.handle == SEQ_HANDLE_RIGHT) {
wmcursor = WM_CURSOR_RIGHT_HANDLE;
}
WM_cursor_set(win, wmcursor);
}
/* *********************** header region ************************ */
/* Add handlers, stuff you only do once or on area/region changes. */
static void sequencer_header_region_init(wmWindowManager * /*wm*/, ARegion *region)
@ -1014,6 +1102,9 @@ void ED_spacetype_sequencer()
art->message_subscribe = sequencer_main_region_message_subscribe;
art->keymapflag = ED_KEYMAP_TOOL | ED_KEYMAP_GIZMO | ED_KEYMAP_VIEW2D | ED_KEYMAP_FRAMES |
ED_KEYMAP_ANIMATION;
art->cursor = sequencer_main_cursor;
art->event_cursor = true;
art->clip_gizmo_events_by_ui = true;
BLI_addhead(&st->regiontypes, art);
/* Preview. */

View File

@ -31,6 +31,7 @@
#include "transform.hh"
#include "transform_convert.hh"
#include "transform_mode.hh"
#define SEQ_EDGE_PAN_INSIDE_PAD 3.5
#define SEQ_EDGE_PAN_OUTSIDE_PAD 0 /* Disable clamping for panning, use whole screen. */