Page MenuHome

VSE: add Adjust Last Operation panel to the video sequencer
ClosedPublic

Authored by Alessio Monti di Sopra (a.monti) on Nov 7 2019, 6:44 PM.

Details

Summary

The patch adds the adjust last operation panel to the Video Sequencer and makes some little changes to the operators.

Removed OPTYPE_REGISTER from the following operators, in order to avoid showing them in the redo panel:

  • select
  • select_all
  • select_inverse
  • select_box
  • strip_jump

Hidden some properties for the same reason:

  • modifier_move
  • modifier_remove
  • set_range_to_strips

Removed an unused transform mode property from SEQUENCER_OT_duplicate.

A couple of changes in effect_strip_add:

  • avoid throwing an error when tweaking the start and end frame of the strip (set end frame to [start frame + 1] if lower than the start frame).
  • set the soft min/max of start and end frame to 0/MAXFRAME
  • changed the color property to use an actual color socket instead of a generic array
  • hide properties depending on the effect type


There might be others, but I only noticed this as a major problem at the moment: this gets called by Move/Extend from Playhead but the redo doesn't work properly, the moment you tweak the values it always behaves as if you were moving the whole strip instead of only a handle.


I noticed similar errors in the Graph editor and the Nla with the equivalent functions.
I saw you were the last one touching this code @Germano Cavalcante (mano-wii) and found this task T68836, do you think this is something likely to be solved by your work in this area?

Diff Detail

Repository
rB Blender

Event Timeline

I noticed similar errors in the Graph editor and the Nla with the equivalent functions.
I saw you were the last one touching this code @Germano (Germano) Cavalcante (mano-wii) and found this task T68836, do you think this is something likely to be solved by your work in this area?

Some problems with transform operations are known and will be resolved after the refactor.
As soon as I have time I will update some patches and give progress to this refactor.

If the problem is not reported, it would be good to make one so we can keep track of it.

I noticed similar errors in the Graph editor and the Nla with the equivalent functions.
I saw you were the last one touching this code @Germano (Germano) Cavalcante (mano-wii) and found this task T68836, do you think this is something likely to be solved by your work in this area?

Some problems with transform operations are known and will be resolved after the refactor.
As soon as I have time I will update some patches and give progress to this refactor.

If the problem is not reported, it would be good to make one so we can keep track of it.

Thanks, I'll check the bug tracker and report the problems if they are not already there.

I just quickly skimmed over this. Seems OK, just please check when adding new region, if any versioning code is needed, so older files can use this feature.

I just quickly skimmed over this. Seems OK, just please check when adding new region, if any versioning code is needed, so older files can use this feature.

I've checked with both 2.79 and 2.80 files and it seems to work fine, the region gets enabled by default.

I just noticed that the problem exists in reverse, if you saved a file with the patch applied and the redo panel open at save time, and then open that file in 2.80, this happens:


It goes away if you change frame (I guess because it doesn't have a redo option) or if you perform an operation in another editor.

Nb. All effect strips gets a color button, but it doesn't seem to be connected to anything. Ex. the color button isn't related to the color of the text or color strips and even multicam gets a color button.

Alessio Monti di Sopra (a.monti) edited the summary of this revision. (Show Details)
  • Added a function to dynamically hide unused properties in SEQUENCER_OT_effect_strip_add.
Richard Antalik (ISS) requested changes to this revision.Nov 18 2019, 4:41 AM
Richard Antalik (ISS) added inline comments.
source/blender/editors/space_sequencer/sequencer_add.c
1089–1090

I would suggest overriding actual RNA property value here:
RNA_int_set(op->ptr, "frame_end", end_frame);

source/blender/editors/space_sequencer/sequencer_edit.c
2862

why remove undo here?

This revision now requires changes to proceed.Nov 18 2019, 4:41 AM
source/blender/editors/space_sequencer/sequencer_edit.c
2862

Right now you basically waste an undo step here, since Ctrl-z doesn't restore the previous view location. And even if it was possible to implement it, it would be quite useless imho.

Anyway I forgot to move this to D6249, where I did the same thing to other editors, will do now.

  • addressed inline comments
This revision is now accepted and ready to land.Nov 18 2019, 11:21 AM
Campbell Barton (campbellbarton) requested changes to this revision.Nov 18 2019, 11:53 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/space_sequencer/sequencer_add.c
99–100

Negative start frames are supported. Why not set this to -MAXFRAME ?

323

This should be replaced by BKE_sequence_effect_get_num_inputs(type) == 0

This revision now requires changes to proceed.Nov 18 2019, 11:53 AM
source/blender/editors/space_sequencer/sequencer_add.c
99–100

I was thinking about this as well. They are supported, but can not be played back.

It is true though, that this is unnecessary limitation.

  • addressed inline comments
Alessio Monti di Sopra (a.monti) added inline comments.
source/blender/editors/space_sequencer/sequencer_add.c
323

Thanks! Definitely more time-proof.

This revision is now accepted and ready to land.Nov 25 2019, 4:12 PM