Paths

Table of Contentst

VSE: Expose existing Transform Expand as a Select Tool toggle button.
AbandonedPublic
Actions

Authored by Peter Fog (tintwotin) on Jan 11 2021, 3:05 PM.

Details

Reviewers

Richard Antalik (ISS)
Hans Goudey (HooglyBoogly)

Summary

This patch is exposing the existing Transform Expand mode:

As a select/box-select tool toggle button:

The reason for this patch is that currently this essential function is that unexposed that even seasoned VSE users are not aware of the expand/ripple mode when transforming, and it is annoying to have to add shortcut-key press for a mode, most users will want to be in most of the time. Since it is implemented as an option in the select/select-box tool, no other tools are affected or should be.

If this patch gets committed, then this patch(which includes more functionality) can be closed: D9699

EDIT:
Boolean naming: Ripple Edit
Tooltip: Adjust the position of all following strips after a transform operation.
Fix versioning.
Moved def to SequencerToolSettings.

Diff Detail

Event Timeline

Peter Fog (tintwotin) requested review of this revision.Jan 11 2021, 3:05 PM

Peter Fog (tintwotin) created this revision.

Peter Fog (tintwotin) edited the summary of this revision. (Show Details)

Peter Fog (tintwotin) added a reviewer: Hans Goudey (HooglyBoogly).Jan 11 2021, 5:26 PM

Oscar (KINjO) awarded a token.Jan 11 2021, 7:08 PM

Oscar (KINjO) added a subscriber: Oscar (KINjO).

Julian Perez (julperado) awarded a token.Jan 11 2021, 7:41 PM

David (activemotionpictures) awarded a token.Jan 11 2021, 8:22 PM

Peter Fog (tintwotin) retitled this revision from VSE: Expose existing Transform Extend as a select-tool toggle button. to VSE: Expose existing Transform Expand as a Select Tool toggle button..Jan 11 2021, 9:21 PM

Since there is plenty of space in the tool header, why not expose it like with a boolean like [ ] Ripple Edit?

@Hans Goudey (HooglyBoogly)

Expose it like a boolean: [ ] Ripple.
I called it Ripple and not Ripple Edit, since it is not affecting all edit operations - only transform.

In D10074#251608, @Peter Fog (tintwotin) wrote:

I called it Ripple and not Ripple Edit, since it is not affecting all edit operations - only transform.

Though it's only displayed during transform operations, right? So I'm not sure that's an issue. Not saying it has to be different, but just "Ripple" seems a bit terse to me.

source/blender/makesrna/intern/rna_sequencer.c
2039	This should give a brief description of what the property is supposed to do when it's on.

Jayraj Kharvadi (jayrajkharvadi) awarded a token.Jan 12 2021, 5:51 AM

@Hans Goudey (HooglyBoogly)
Boolean naming: Ripple Edit
Tooltip: Adjust the position of all following strips to make room for inserted material

Peter Fog (tintwotin) edited the summary of this revision. (Show Details)Jan 12 2021, 7:12 AM

Andy Cuccaro (AndyCuccaro) awarded a token.Jan 12 2021, 9:33 PM

Stig Oskar (Stig) awarded a token.Jan 14 2021, 2:21 PM

Is "material" the right word? Seems a bit odd considering what the term "material" usually means in Blender. Why not replace "inserted material" with "selected strips"?

Richard Antalik (ISS) requested changes to this revision.Jan 19 2021, 8:28 PM

Richard Antalik (ISS) added inline comments.

source/blender/blenloader/intern/versioning_280.c
3475	Use `versioning_290.c`
source/blender/makesdna/DNA_sequence_types.h
278–280	I think `SequencerToolSettings` are designed to have these kind of settings. defined in `DNA_scene_types.h` `transform_flags` seems to be more descriptive name It should have own enum, I would preffer to be named as well and not reuse `SEQ_EDIT_***`

This revision now requires changes to proceed.Jan 19 2021, 8:28 PM

Tooltip: Adjust the position of all following strips after a transform operation.
Fix versioning.
Moved def to SequencerToolSettings.

These comments are unclear to me. I hope you can give me a more detailed explanation in a chat pm:

transform_flags seems to be more descriptive name

How much do you want me to rename? Everything called ripple or just the "ripple_flag"?

It should have own enum, I would prefer to be named as well and not reuse SEQ_EDIT_***

It is just for a toggle button, why is it necessary to make it an enum?

In D10074#254113, @Peter Fog (tintwotin) wrote:

These comments are unclear to me. I hope you can give me a more detailed explanation in a chat pm:

Wrote im PM as well but I will respond here as well.

transform_flags seems to be more descriptive name

How much do you want me to rename? Everything called ripple or just the "ripple_flag"?

Only ripple_flag to transform_flags - these flags will affect strip transform operator.

It should have own enum, I would prefer to be named as well and not reuse SEQ_EDIT_***

It is just for a toggle button, why is it necessary to make it an enum?

By enum I meant C data type like

typedef enum eSeqTransformFlags {
  SEQ_TRANSFORM_USE_RIPPLE = 1 << 0,
} eSeqTransformFlags;

This is so you can lookup all possible flags in IDE quickly, and to if you need to store these flags in variable, it can have type eSeqTransformFlags that will make it clear what the flag is and where it is defined.
I have forgot to define value in PM (FLAG_VAL = 1 << 0), this is so we can combine various flags.

Peter Fog (tintwotin) mentioned this in D10424: VSE: Add Transform Preview.Feb 16 2021, 7:34 AM

Peter Fog (tintwotin) edited the summary of this revision. (Show Details)Feb 16 2021, 7:49 AM

This patch didn't make it into 2.93. As I do not have anymore time or motivation for a continued investment in the Blender project, I'm giving up on this patch for now.

Peter Fog (tintwotin) mentioned this in T88663: VSE UI: Operators missing "Wait for input" so they can be exposed in the menus.May 29 2021, 8:46 AM

Peter Fog (tintwotin) mentioned this in D11805: VSE: Transform overwrite mode.Jul 5 2021, 11:27 AM