Page MenuHome

VSE: Use snapping settings for scrubbing
ClosedPublic

Authored by Richard Antalik (ISS) on Jun 29 2021, 7:42 PM.

Details

Summary

Use "Snap Playhead to Strips" option to enable playhead snapping.
Change behavior of CTRL key to invert snapping similar to transform operator.

Currently this option is disabled by default. It makes editing quite unpleasant
for me personally, but ideally I should gather feedback from more users.

Diff Detail

Repository
rB Blender
Branch
arcpatch-D11745 (branched from master)
Build Status
Buildable 15534
Build 15534: arc lint + arc unit

Event Timeline

Richard Antalik (ISS) requested review of this revision.Jun 29 2021, 7:42 PM
Richard Antalik (ISS) created this revision.

I haven't tested this yet, but I can imagine a playhead constantly snapping will make editing very difficult, and I'm not sure what purpose it would serve to snap the playhead to strip ends as a mode? An alternative approach would be to map the goto next/prev function to the mouse wheel, which will make it fast to move the playhead forward and backwards at strip starts/ends.

This is what I wrote about this in the VSE chat: https://blender.chat/channel/vse?msg=zwvwp2GJNfoRuhJCo

As it is quite annoying the multi-tapping on J, K & L for playback speeds are missing from the VSE, an alternative approach could be to look at movement in the 3d space, where +shift alters the speed and +ctrl snaps, that could be translated into the following vse keys/functions: alt + mouse wheel = scrub 1/-1 frame, shift + mouse wheel = scrub 100/-100 frames and ctrl + mouse wheel = next/prev. strip end. Unfortunately + alt can't be overwritten and is locked to +-1, so the +shift can't lower the speed like in the 3d view(but in walk/fly mode +shift adds speed too). In a gif it looks like this:

Download patch: https://blender.chat/file-upload/nP3hixfwjBpqHeEBL/scub_keys_installation_file.zip

Btw. Ctrl + transform is industry standard for snapping in NLEs.

Campbell Barton (campbellbarton) requested changes to this revision.EditedJul 1 2021, 2:42 AM

There are a few changes in this patch:

  • Snapping sequencer strips has a threshold (if users prefer this it seems fine).
  • Snapping the current frame has a threshold too (was this intended?)
  • Use snapping by default (this is a bigger change).

Breaking away from Blender in terms of consistency isn't something to do lightly, and it seems disputable that this is even preferred by users.

I don't see why snapping needs to be enabled by default, if the button is available in the interface users who prefer this can always turn it on very easily.


Suggest to split changing defaults out of this patch since it's more of a design decision and isn't a significant part of this patch which otherwise looks fine for the most part.

source/blender/makesdna/DNA_scene_types.h
2059–2060

Use "CURRENT_FRAME" term in keeping with the rest of Blender.

source/blender/makesrna/intern/rna_scene.c
3527

Use "Current Frame" in keeping with the rest of Blender.

This revision now requires changes to proceed.Jul 1 2021, 2:42 AM
  • Use current frame instead of playhead
Richard Antalik (ISS) marked 2 inline comments as done.Jul 1 2021, 10:43 AM

There are a few changes in this patch:

  • Snapping sequencer strips has a threshold (if users prefer this it seems fine).
  • Snapping the current frame has a threshold too (was this intended?)
  • Use snapping by default (this is a bigger change).

Breaking away from Blender in terms of consistency isn't something to do lightly, and it seems disputable that this is even preferred by users.

I don't see why snapping needs to be enabled by default, if the button is available in the interface users who prefer this can always turn it on very easily.


I don't disagree, though I agreed for snapping to be enabled because it's not very intrusive currently.

Suggest to split changing defaults out of this patch since it's more of a design decision and isn't a significant part of this patch which otherwise looks fine for the most part.

By changing defaults, do you mean snapping being enabled? That is not scope of this patch, you can suggest this in D11759. This patch should be only for using snapping settings for current frame snapping changes.

I wanted to gather feedback on devtalk for snapping after this feature is implemented, because it will be possibly most controversial.

source/blender/makesdna/DNA_scene_types.h
2059–2060

There has been some feedback, for general snapping feature addressed in D11759

Richard Antalik (ISS) marked an inline comment as done.Jul 1 2021, 10:43 AM
  • rebase patch on master - It used WIP branch
  • Include missing changes
  • Rebase again, since changes are in master

Sorry for noise, I have messed up patch update and haven't noticed right away

Accepting with some minor changes requested.

source/blender/editors/animation/anim_ops.c
94

Generally cmp functions compare two values and return equality.

Suggest seq_frame_snap_update_best.

source/blender/makesrna/intern/rna_scene.c
3524

Should be use_snap_current_frame_to_strips, (other's should use use_ prefix here too).

This revision is now accepted and ready to land.Jul 5 2021, 1:30 PM
This revision was automatically updated to reflect the committed changes.
Richard Antalik (ISS) marked 2 inline comments as done.