Page MenuHome

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

Authored by Peter Fog (tintwotin) on Sat, May 9, 10:27 PM.

Details

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
Branch
arcpatch-D7679_1 (branched from master)
Build Status
Buildable 8135
Build 8135: arc lint + arc unit

Event Timeline

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

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

Richard Antalik (ISS) requested changes to this revision.Mon, May 11, 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.Mon, May 11, 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

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?

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.

Peter Fog (tintwotin) edited the summary of this revision. (Show Details)
  • Renamed "Center" to "Under".
  • Added comments.
Peter Fog (tintwotin) marked 3 inline comments as done.Mon, May 11, 9:46 AM
Peter Fog (tintwotin) edited the summary of this revision. (Show Details)Tue, May 12, 7:43 PM
Richard Antalik (ISS) requested changes to this revision.Thu, May 14, 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.Thu, May 14, 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.Tue, May 19, 11:15 AM
Peter Fog (tintwotin) retitled this revision from VSE: Add select at playhead center to VSE: Add select under playhead, and shortcuts for left, right, under..
Peter Fog (tintwotin) edited the summary of this revision. (Show Details)

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.

Richard Antalik (ISS) edited the summary of this revision. (Show Details)
  • Fix margin calculation and comment style
Richard Antalik (ISS) added inline comments.
source/blender/editors/space_sequencer/sequencer_select.c
425–427

Oops float under_playhead_margin = UI_view2d_view_to_region_x(v2d, 5.0f) was wrong, but I have fixed it.

This revision is now accepted and ready to land.Wed, May 20, 3:09 PM