VSE: Add slip tool
Needs RevisionPublic
Actions

Authored by Richard Antalik (ISS) on Mar 8 2020, 9:05 PM.

Details

Reviewers

Campbell Barton (campbellbarton)
William Reynish (billreynish)

Summary

Add tool for moving content inside strips

Only passthrough check has been added to operator in this patch

Diff Detail

Repository

rB Blender

Branch

tool-slip (branched from master)

Build Status

Buildable 7034
Build 7034: arc lint + arc unit

Event Timeline

Richard Antalik (ISS) created this revision.Mar 8 2020, 9:05 PM

Harbormaster completed remote builds in B7030: Diff 22522.Mar 8 2020, 9:05 PM

William Reynish (billreynish) awarded a token.Mar 8 2020, 9:46 PM

Richard Antalik (ISS) added a project: Video Sequencer.Mar 8 2020, 10:16 PM

This change raises a design issue with sequencer tools.

Since this only operates on one strip at a time - for an action where you might reasonably want to operate on multiple items at once.

If this convention is uses across all other tools, it means anyone who wants to make multiple edits will have to perform them on each strip - or give up on using tools and use direct key shortcuts.

Design Task: T74561: Proposal for sequencer tool interactions

source/blender/editors/space_sequencer/sequencer_edit.c
1519–1530	Should be moved into a utility function, similar to `find_nearest_seq` but checking for cursor inside the sequence instead of nearest.

This revision now requires changes to proceed.Mar 9 2020, 5:46 AM

Campbell Barton (campbellbarton) mentioned this in T74561: Proposal for sequencer tool interactions .Mar 9 2020, 6:05 AM

In D7063#168369, @Campbell Barton (campbellbarton) wrote:

This change raises a design issue with sequencer tools.

Since this only operates on one strip at a time - for an action where you might reasonably want to operate on multiple items at once.

Even with this implementation you can select more strips and operate on selection.
I will change behavior to check if tweak is done on selected strip to start action, otherwise passthrough to selection.

Even better would be to draw and use gizmo I guess, I could try to do it so I understand the system better. It's still a bit foggy for me. But I still should be focused on triaging, so can't promise, that I will do it.

Make tool passthrough if it doesn't operate on selection
Add seq_get_sequence_mouse_over() function to get strip under cursor

Harbormaster completed remote builds in B7034: Diff 22527.Mar 9 2020, 9:08 AM

Tested and this seems to work.

This makes the slip tool work the same as tweak, so you can select, border select as well as slipping multiple strips.

P1286 D7063 alternative

1	diff --git a/release/scripts/presets/keyconfig/keymap_data/blender_default.py b/release/scripts/presets/keyconfig/keymap_data/blender_default.py
2	index 5aff2a75e03..856f7fe1f46 100644
3	--- a/release/scripts/presets/keyconfig/keymap_data/blender_default.py
4	+++ b/release/scripts/presets/keyconfig/keymap_data/blender_default.py
5	@@ -6137,6 +6137,19 @@ def km_sequencer_editor_tool_blade(_params):
6	)
7
8
9	+def km_sequencer_editor_tool_slip(params):
10	+ return (
11	+ "Sequencer Tool: Slip",
12	+ {"space_type": 'SEQUENCE_EDITOR', "region_type": 'WINDOW'},
13	+
14	+ {"items": [
15	+ ("sequencer.select", {"type": params.select_mouse, "value": 'PRESS'},
16	+ {"properties": [("extend", False), ("deselect_all", not params.legacy)]}),
17	+ ("sequencer.slip", {"type": 'EVT_TWEAK_L', "value": 'ANY'}, None),
18	+ ]},
19	+ )
20	+
21	+
22	# ------------------------------------------------------------------------------
23	# Full Configuration
24
25	@@ -6353,6 +6366,7 @@ def generate_keymaps(params=None):
26	km_sequencer_editor_tool_select(params),
27	km_sequencer_editor_tool_select_box(params),
28	km_sequencer_editor_tool_blade(params),
29	+ km_sequencer_editor_tool_slip(params)
30	]
31
32	# ------------------------------------------------------------------------------
33	diff --git a/release/scripts/startup/bl_ui/space_toolsystem_toolbar.py b/release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
34	index 63fb72a71bd..d54d1142615 100644
35	--- a/release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
36	+++ b/release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
37	@@ -1740,6 +1740,16 @@ class _defs_sequencer_generic:
38	keymap="Sequencer Tool: Blade",
39	draw_settings=draw_settings,
40	)
41	+ @ToolDef.from_fn
42	+ def slip():
43	+ return dict(
44	+ idname="builtin.slip",
45	+ label="Blade",
46	+ icon=None,
47	+ widget=None,
48	+ keymap="Sequencer Tool: Slip",
49	+ draw_settings=None,
50	+ )
51
52
53	class _defs_sequencer_select:
54	@@ -2272,11 +2282,13 @@ class SEQUENCER_PT_tools_active(ToolSelectPanelHelper, Panel):
55	'SEQUENCER': [
56	*_tools_select,
57	_defs_sequencer_generic.blade,
58	+ _defs_sequencer_generic.slip,
59	],
60	'SEQUENCER_PREVIEW': [
61	*_tools_select,
62	*_tools_annotate,
63	_defs_sequencer_generic.blade,
64	+ _defs_sequencer_generic.slip,
65	],
66	}
67
68	diff --git a/source/blender/editors/space_sequencer/sequencer_edit.c b/source/blender/editors/space_sequencer/sequencer_edit.c
69	index ca2a5eaa54f..1bd5c7134fe 100644
70	--- a/source/blender/editors/space_sequencer/sequencer_edit.c
71	+++ b/source/blender/editors/space_sequencer/sequencer_edit.c
72	@@ -1487,7 +1487,12 @@ static int sequencer_slip_invoke(bContext C, wmOperator op, const wmEvent *eve
73	num_seq = slip_count_sequences_rec(ed->seqbasep, true);
74
75	if (num_seq == 0) {
76	- return OPERATOR_CANCELLED;
77	+ if (ISTWEAK(event->type)) {
78	+ return OPERATOR_PASS_THROUGH;
79	+ }
80	+ else {
81	+ return OPERATOR_CANCELLED;
82	+ }
83	}
84
85	data = op->customdata = MEM_mallocN(sizeof(SlipData), "trimdata");

Reposting my comment here, so there aren't 10 threads

Personally I would preffer to use check if tweak is done on selected sequence, just because there is not much of visual indication of what's going on...
But yeah I would be OK to do it like this.

Do you mean it's not clear that slip has been activated?

In this case it could:

Show the offsets during slip.
Use a grab mouse cursor while the operator runs (or even before the operator runs).
Set G.moving while the operator runs (draws outlines brighter - also used for transforming).

Slip tool based on P1286 + enabled visual representation of ongoing action by setting sseq->draw_flag |= SEQ_DRAW_OFFSET_EXT and G.moving |= G_TRANSFORM_SEQ

Harbormaster completed remote builds in B7119: Diff 22725.Mar 12 2020, 2:18 PM

Richard Antalik (ISS) marked an inline comment as done.Mar 12 2020, 2:19 PM

Right click select keymap option currently causes this tool not to do nothing.

Otherwise it seems OK.

Needed to updated the patch to resolve conflicts: P1329

This revision now requires changes to proceed.Apr 6 2020, 9:00 AM

jorge (jorsh) awarded a token.Apr 18 2020, 8:28 PM

Peter Fog (tintwotin) added a project: VFX & Video.Sep 22 2020, 8:21 AM