[Bf-blender-cvs] [478bba732ba] temp-vse-speed-fx: VSE UX: Make Speed Effect strips more user friendly.

Germano Cavalcante noreply at git.blender.org
Tue Jul 6 03:32:07 CEST 2021


Commit: 478bba732ba9d92e60358dbd90dc3913d0edb3f2
Author: Germano Cavalcante
Date:   Tue Jul 6 02:37:16 2021 +0200
Branches: temp-vse-speed-fx
https://developer.blender.org/rB478bba732ba9d92e60358dbd90dc3913d0edb3f2

VSE UX: Make Speed Effect strips more user friendly.

The `Speed Effect` panel is confusing. There are many toggles and hidden values to control the speed effect.
So this patch proposes to simplify it, by adding an `enum` and making it more intuitive by drawing f-curves on the strips.

Before:
{F7843850}

After:
{F9775464,size=full}

**Enum Changes:**
- The new enums correspond to 4 modes: `Stretch`, `Multiply`, `Frame Number` and `Length`.
- Except `Stretch` the other modes now has its respective control values.

**Drawing Changes:**
- F-curve drawing for Stretch, Multiply, Length and Frame Number.
- Value drawing when no keyframes for Stretch, Length and Frame Numbers.

General view of the new drawing for each speed effect mode:
{F9796642, size=full}

Detail of the horizontal zero (blue) line in the new `Multiply` mode:
{F9798520, size=full}

Nice to have (but I don't know how):
- Auto adjusting of endframe when using Multiply or Boost.

---
**OLD STUFF FOR REFERENCE:**
The current functions are that confusing that I feel it is necessary to "spell" each one of them out:

Use Default Fade On - Was renamed to "Stretch to input strip length" in 2.8+ because the adjusts the speed so the first and last frame is the same no matter what the duration of the strip is. Default Fade makes no sense, imo. In the enum I would call it "Fit Duration" or "Stretch to Duration", but maybe it'll be too long? As the speed is relative to the duration of the clip the Multiplier has no meaning and should be disabled, imo. The tooltip could be changed to something along the lines [...]
{F8488797}

The Use as Speed does exactly the same as the Multiply Speed setting, making the latter redundant, imo. It could be called "Speed" in the enum, and the value "Factor". In the tooltip "or remapping the frame number" should be removed. How the Use Speed is working(gif):
{F8488800}

When "Scale to Length" is selected the strip duration goes from 0%(first frame) to 100%(last frame), therefore doesn't it make sense to call the value "Frame Number" but should be called "Percentage" and the Value should be set to go from 0 to 100. "Scale to Length" is not very informative, and since it only works when animated. "Multiply" has no function here either.
How it is working(gif):
{F8488801}

The fourth setting, yes, there are four, is when "Scale to Length" is unselected, and behaves like the previous setting, but in frame numbers.

Differential Revision: https://developer.blender.org/D6110

===================================================================

M	release/scripts/startup/bl_ui/space_sequencer.py
M	source/blender/blenkernel/intern/ipo.c
M	source/blender/blenloader/intern/versioning_300.c
M	source/blender/editors/space_sequencer/sequencer_draw.c
M	source/blender/makesdna/DNA_ipo_types.h
M	source/blender/makesdna/DNA_sequence_types.h
M	source/blender/makesrna/intern/rna_sequencer.c
M	source/blender/sequencer/intern/effects.c
M	source/blender/sequencer/intern/effects.h

===================================================================

diff --git a/release/scripts/startup/bl_ui/space_sequencer.py b/release/scripts/startup/bl_ui/space_sequencer.py
index ab05461f185..4786314912b 100644
--- a/release/scripts/startup/bl_ui/space_sequencer.py
+++ b/release/scripts/startup/bl_ui/space_sequencer.py
@@ -1155,14 +1155,20 @@ class SEQUENCER_PT_effect(SequencerButtonsPanel, Panel):
             flow.prop(strip, "use_only_boost")
 
         elif strip_type == 'SPEED':
-            layout.prop(strip, "use_default_fade", text="Stretch to Input Strip Length")
-            if not strip.use_default_fade:
-                layout.prop(strip, "use_as_speed")
-                if strip.use_as_speed:
-                    layout.prop(strip, "speed_factor")
-                else:
-                    layout.prop(strip, "speed_factor", text="Frame Number")
-                    layout.prop(strip, "use_scale_to_length")
+            col = layout.column(align=True)
+            col.prop(strip, "speed_control", text="Speed Control")
+            if strip.speed_control == "MULTIPLY":
+                col.prop(strip, "speed_factor", text=" ")
+            elif strip.speed_control == "LENGTH":
+                col.prop(strip, "speed_length", text=" ")
+            elif strip.speed_control == "FRAME_NUMBER":
+                col.prop(strip, "speed_frame_number", text=" ")
+
+            row = layout.row(align=True)
+            row.enabled = strip.speed_control != "STRETCH"
+            row.prop(strip, "multiply_speed", text="Boost")
+            row = layout.row(align=True, heading="Interpolation")
+            row.prop(strip, "use_frame_interpolate", text="")
 
         elif strip_type == 'TRANSFORM':
             col = layout.column()
@@ -1232,11 +1238,7 @@ class SEQUENCER_PT_effect(SequencerButtonsPanel, Panel):
             layout.prop(strip, "wrap_width", text="Wrap Width")
 
         col = layout.column(align=True)
-        if strip_type == 'SPEED':
-            col.prop(strip, "multiply_speed")
-            col.prop(strip, "use_frame_interpolate")
-
-        elif strip_type in {'CROSS', 'GAMMA_CROSS', 'WIPE', 'ALPHA_OVER', 'ALPHA_UNDER', 'OVER_DROP'}:
+        if strip_type in {'CROSS', 'GAMMA_CROSS', 'WIPE', 'ALPHA_OVER', 'ALPHA_UNDER', 'OVER_DROP'}:
             col.prop(strip, "use_default_fade", text="Default Fade")
             if not strip.use_default_fade:
                 col.prop(strip, "effect_fader", text="Effect Fader")
diff --git a/source/blender/blenkernel/intern/ipo.c b/source/blender/blenkernel/intern/ipo.c
index f365e759221..0ca490c0848 100644
--- a/source/blender/blenkernel/intern/ipo.c
+++ b/source/blender/blenkernel/intern/ipo.c
@@ -791,15 +791,15 @@ static const char *camera_adrcodes_to_paths(int adrcode, int *array_index)
   /* result depends on adrcode */
   switch (adrcode) {
     case CAM_LENS:
-#if 0  /* XXX this cannot be resolved easily... \
-        * perhaps we assume camera is perspective (works for most cases... */
+#if 0 /* XXX this cannot be resolved easily... \
+       * perhaps we assume camera is perspective (works for most cases... */
       if (ca->type == CAM_ORTHO) {
         return "ortho_scale";
       }
       else {
         return "lens";
       }
-#else  /* XXX lazy hack for now... */
+#else /* XXX lazy hack for now... */
       return "lens";
 #endif /* XXX this cannot be resolved easily */
 
@@ -808,7 +808,7 @@ static const char *camera_adrcodes_to_paths(int adrcode, int *array_index)
     case CAM_END:
       return "clip_end";
 
-#if 0  /* XXX these are not defined in RNA */
+#if 0 /* XXX these are not defined in RNA */
     case CAM_YF_APERT:
       poin = &(ca->YF_aperture);
       break;
@@ -1122,6 +1122,12 @@ static char *get_rna_access(ID *id,
         case SEQ_FAC_SPEED:
           propname = "speed_fader";
           break;
+        case SEQ_FAC_SPEED_FRAME_NUMBER:
+          propname = "speed_fader_frame_number";
+          break;
+        case SEQ_FAC_SPEED_LENGTH:
+          propname = "speed_fader_length";
+          break;
         case SEQ_FAC_OPACITY:
           propname = "blend_alpha";
           break;
@@ -2307,6 +2313,7 @@ void do_versions_ipos_to_animato(Main *bmain)
          * to different DNA variables later
          * (semi-hack (tm) )
          */
+        SpeedControlVars *v;
         switch (seq->type) {
           case SEQ_TYPE_IMAGE:
           case SEQ_TYPE_META:
@@ -2316,7 +2323,16 @@ void do_versions_ipos_to_animato(Main *bmain)
             adrcode = SEQ_FAC_OPACITY;
             break;
           case SEQ_TYPE_SPEED:
-            adrcode = SEQ_FAC_SPEED;
+            v = (SpeedControlVars *)seq->effectdata;
+            if (ELEM(v->speed_control_type, SEQ_SPEED_MULTIPLY, SEQ_SPEED_STRETCH)) {
+              adrcode = SEQ_FAC_SPEED;
+            }
+            else if (v->speed_control_type == SEQ_SPEED_FRAME_NUMBER) {
+              adrcode = SEQ_FAC_SPEED_FRAME_NUMBER;
+            }
+            else if (v->speed_control_type == SEQ_SPEED_LENGTH) {
+              adrcode = SEQ_FAC_SPEED_LENGTH;
+            }
             break;
         }
         icu->adrcode = adrcode;
diff --git a/source/blender/blenloader/intern/versioning_300.c b/source/blender/blenloader/intern/versioning_300.c
index ecee14d3d58..017c2b29874 100644
--- a/source/blender/blenloader/intern/versioning_300.c
+++ b/source/blender/blenloader/intern/versioning_300.c
@@ -199,6 +199,39 @@ static void version_node_socket_name(bNodeTree *ntree,
   }
 }
 
+static void do_versions_sequencer_speed_effect_recursive(const ListBase *seqbase)
+{
+  LISTBASE_FOREACH (Sequence *, seq, seqbase) {
+    if (seq->type == SEQ_TYPE_SPEED) {
+      SpeedControlVars *v = (SpeedControlVars *)seq->effectdata;
+      if (seq->flag & SEQ_USE_EFFECT_DEFAULT_FADE) {
+        v->speed_control_type = SEQ_SPEED_STRETCH;
+      }
+
+      /* Old SpeedControlVars->flags. */
+#define SEQ_SPEED_INTEGRATE (1 << 0)
+#define SEQ_SPEED_COMPRESS_IPO_Y (1 << 2)
+      else if (v->flags & SEQ_SPEED_INTEGRATE) {
+        v->speed_control_type = SEQ_SPEED_MULTIPLY;
+      }
+      else if (v->flags & SEQ_SPEED_COMPRESS_IPO_Y) {
+        v->speed_control_type = SEQ_SPEED_LENGTH;
+        seq->speed_fader_length = seq->speed_fader * 100;
+      }
+      else {
+        v->speed_control_type = SEQ_SPEED_FRAME_NUMBER;
+        seq->speed_fader_frame_number = (int)seq->speed_fader;
+      }
+      v->flags &= ~(SEQ_SPEED_INTEGRATE | SEQ_SPEED_COMPRESS_IPO_Y);
+#undef SEQ_SPEED_INTEGRATE
+#undef SEQ_SPEED_COMPRESS_IPO_Y
+    }
+    else if (seq->type == SEQ_TYPE_META) {
+      do_versions_sequencer_speed_effect_recursive(&seq->seqbase);
+    }
+  }
+}
+
 static bool replace_bbone_len_scale_rnapath(char **p_old_path, int *p_index)
 {
   char *old_path = *p_old_path;
@@ -478,6 +511,11 @@ void blo_do_versions_300(FileData *fd, Library *UNUSED(lib), Main *bmain)
    * \note Keep this message at the bottom of the function.
    */
   {
+    LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) {
+      if (scene->ed != NULL) {
+        do_versions_sequencer_speed_effect_recursive(&scene->ed->seqbase);
+      }
+    }
     /* Keep this block, even when empty. */
   }
 }
diff --git a/source/blender/editors/space_sequencer/sequencer_draw.c b/source/blender/editors/space_sequencer/sequencer_draw.c
index 8341dbe6014..285ecd98140 100644
--- a/source/blender/editors/space_sequencer/sequencer_draw.c
+++ b/source/blender/editors/space_sequencer/sequencer_draw.c
@@ -92,6 +92,8 @@
 
 #include "MEM_guardedalloc.h"
 
+#include "intern/effects.h"
+
 /* Own include. */
 #include "sequencer_intern.h"
 
@@ -1008,12 +1010,41 @@ static void fcurve_batch_add_verts(GPUVertBuf *vbo,
  * - Volume for sound strips.
  * - Opacity for the other types.
  */
-static void draw_seq_fcurve_overlay(
-    Scene *scene, View2D *v2d, Sequence *seq, float x1, float y1, float x2, float y2, float pixelx)
+static void draw_seq_fcurve_overlay(Scene *scene,
+                                    View2D *v2d,
+                                    Sequence *seq,
+                                    float x1,
+                                    float y1,
+                                    float x2,
+                                    float y2,
+                                    float pixelx,
+                                    float pixely)
 {
+  SpeedControlVars *v = (SpeedControlVars *)seq->effectdata;
   FCurve *fcu;
+  double curve_val;
 
-  if (seq->type == SEQ_TYPE_SOUND_RAM) {
+  if (seq->type == SEQ_TYPE_SPEED) {
+    switch (v->speed_control_type) {
+      case SEQ_SPEED_MULTIPLY: {
+        fcu = id_data_find_fcurve(&scene->id, seq, &RNA_Sequence, "speed_factor", 0, NULL);
+        break;
+      }
+      case SEQ_SPEED_FRAME_NUMBER: {
+        fcu = id_data_find_fcurve(&scene->id, seq, &RNA_Sequence, "speed_frame_number", 0, NULL);
+        break;
+      }
+      case SEQ_SPEED_LENGTH: {
+        fcu = id_data_find_fcurve(&scene->id, seq, &RNA_Sequence, "speed_length", 0, NULL);
+        break;
+      }
+      case SEQ_SPEED_STRETCH: {
+        fcu = NULL;
+        break;
+      }
+    }
+  }
+  else if (seq->type == SEQ_TYPE_SOUND_RAM) {
     fcu = id_data_find_fcurve(&scene->id, seq, &RNA_Sequence, "volume", 0, NULL);
   }
   else {
@@ -1041,13 +1072,66 @@ static void draw_seq_fcurve_overlay(
     uint vert_count = 0;
 
     const float y_height = y2 - y1;
-    float curve_val;
     float prev_val = INT_MIN;
     bool skip = false;
+    double ymax = 1.0;
+    double ymin = 0.0;
+
+    if (seq->type == SEQ_TYPE_SPEED) {
+      SEQ_effect_handle_get(seq);
+      switch (v->speed_control_type) {
+        case SEQ_SPEED_LENGTH: {
+          ymin = 0.0;
+          ymax = 100.0;
+          break;
+        }
+        case SEQ_SPEED_FRAME_NUMBER: {
+          ymin = 0;
+          ymax = seq_effect_speed_get_strip_content_length(seq->seq1);
+          break;
+        }
+        case SEQ_SPEED_MULTIPLY: {
+          ymin = 0.0;
+          ymax = 0.0;
+          /* Get range. */
+          for (int timeline_frame = eval_start; timeline_frame <= eval_end;
+               timeline_frame += eval_step) {
+            curve_val = evaluate_fcurve(fcu, timeline_frame);
+            if (curve_val > ymax) {
+              ymax = curve_val;
+            }

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list