Page MenuHome

VSE: don't allow strip preview when clicking on the scrubbing region
AcceptedPublic

Authored by Alessio Monti di Sopra (a.monti) on Wed, Mar 25, 9:30 PM.

Details

Summary

The patch makes it so that is no longer possible to trigger strip preview when clicking on the scrubbing area and a strip is drawn behind it.

Diff Detail

Repository
rB Blender

Event Timeline

This revision is now accepted and ready to land.Wed, Mar 25, 11:31 PM

This effectively disables the preview feature entirely for the LCS keymap. If you guys say that's fine, sure go ahead, but it should be explicitly mentioned in the commit log.

On the code side - shouldn't the caller handle this logic? This function should simply and only set the preview to whatever strip is closest, after all it's called _set(). The caller should only execute that when it makes sense. You can also use ED_time_scrub_event_in_region() there then.

This effectively disables the preview feature entirely for the LCS keymap. If you guys say that's fine, sure go ahead, but it should be explicitly mentioned in the commit log.
On the code side - shouldn't the caller handle this logic? This function should simply and only set the preview to whatever strip is closest, after all it's called _set(). The caller should only execute that when it makes sense. You can also use ED_time_scrub_event_in_region() there then.

Ok, I'll look into moving the condition outside.
This doesn't block preview for LCS though, it only prevents it from happening from the scrubbing area, making sure that you always scrub the actual timeline from there. For preview one can use Shift (if I remeber correctly) + right click

This effectively disables the preview feature entirely for the LCS keymap. If you guys say that's fine, sure go ahead, but it should be explicitly mentioned in the commit log.

This is not true unless I am missing something.
Ideally you should only use Shift + RMB for "special preview" (in LCS keymap). Or rather do not use "special preview" when scrubbing in scrubbing area - that should cover both LCS and RCS keymaps consistently.

On the code side - shouldn't the caller handle this logic? This function should simply and only set the preview to whatever strip is closest, after all it's called _set(). The caller should only execute that when it makes sense. You can also use ED_time_scrub_event_in_region() there then.

I can agree with this point. I did not pay attention to what was modified and what caller code is...

  • moved condition to change_frame_seq_preview_begin() using ED_time_scrub_event_in_region()