Details

Reviewers

Commits

rB95e3356a2760: VSE: Add select under playhead, and shortcuts for left, right, under.

Summary

Since it is necessary to select strips before being able to split them(not counting the toolbar split function), a much-needed function is the ability to select all strips at the playhead quickly with a shortcut key. This patch adds that functionality, however, later, when a shortcut key is agreed on, it can be added in a separate patch.

The function is added to Select > Playhead > Under

Gif (ctrl+left mouse):

Add Equal as shortcut for select under playhead, and move Insert Gaps to backspace + ctrl.
All of the extend (+shift) functions are added to the left, right under functions.

Diff Detail

Repository: rB Blender

Event Timeline

Peter Fog (tintwotin) requested review of this revision.May 9 2020, 10:27 PM

Peter Fog (tintwotin) created this revision.

jorge (jorsh) awarded a token.May 10 2020, 2:33 PM

Add left mouse click + ctrl at playhead will select in center.

Richard Antalik (ISS) requested changes to this revision.May 11 2020, 7:59 AM

Richard Antalik (ISS) added inline comments.

source/blender/editors/space_sequencer/sequencer_select.c
425–427	What is this supposed to do? And why?
442–449	I would rather have 3 functions here and comments what particular branch does and if it is linked to any option. Separate function with descriptive names would be better perhaps but given this function is already undocumented and in quite bad shape, just comment is OK.
642	Option "center" doesn't really make sense. I guess something like "intersecting" or "under playhead" would be better.

This revision now requires changes to proceed.May 11 2020, 7:59 AM

I would rather have 3 functions here and comments what particular branch does and if it is linked to any option.

Sorry I meant 3 conditions

Peter Fog (tintwotin) added inline comments.May 11 2020, 9:00 AM

source/blender/editors/space_sequencer/sequencer_select.c
425–427	In the original function ctrl+left mouse to the left or right of playhead will select "Left" or "Right". What is added here is ctrl+left mouse close to playhead will select "Center".
642	Since the menu is called Playhead > Left/Right, I guess "Under" would be enough?

In D7679#185203, @Richard Antalik (ISS) wrote:

I would rather have 3 functions here and comments what particular branch does and if it is linked to any option.

Sorry I meant 3 conditions

I guess the reason for this way of doing it in the original code, is that it both needs to work for mouse position based selection and for the options in the enum.

Renamed "Center" to "Under".
Added comments.

Peter Fog (tintwotin) marked 3 inline comments as done.May 11 2020, 9:46 AM

Andrea Monzini (Andrea_Monzini) awarded a token.May 12 2020, 7:33 PM

Peter Fog (tintwotin) edited the summary of this revision. (Show Details)May 12 2020, 7:43 PM

Richard Antalik (ISS) requested changes to this revision.May 14 2020, 12:45 AM

Richard Antalik (ISS) added inline comments.

source/blender/editors/space_sequencer/sequencer_select.c
425–427	I guess that would be OK. It looks like it should do that, but if I just looked at code without any context, I would have no clue. So please add some comment that this is some kind of dead zone to allow 3 modes be done with mouse. My main issue then is with implementation: Use `if ()` not `if (())` `CFRA +/- 5.0f` will make dead zone 10 frames wide, which is dependant on zoom. make variable `float under_playhead_margin = UI_view2d_view_to_region_x(v2d, 5.0f)` and use that instead of `5.0f` value
642	I think it's enough for UI, though it would be better if it was called explicitly under playhead when used in code - it's more readable. {SEQ_SELECT_LR_UNDER_PLAYHEAD, "UNDER_PLAYHEAD", 0, "Under", "Select under"}, Even description for those 3 options could be updated to `Select left to playhead`, `Select right to playhead` and `Select under playhead` After all these operators could exist outside of menu providing context (custom addon) where it is nicer to hover on top of button to see what it actually does.

This revision now requires changes to proceed.May 14 2020, 12:45 AM

Updated hotzone.
Updated namings, however, the tooltips are overwritten by the main Selection operator tooltip.

Peter Fog (tintwotin) marked 2 inline comments as done.May 19 2020, 11:15 AM

Add Equal as shortcut for select under playhead, and move Insert Gaps to backspace + ctrl.
All of the extend (+shift) functions are added to the left, right under functions.