Page MenuHome
Differential D6110

VSE: Speed Effect panel reworked
Needs RevisionPublic
Actions

Authored by Peter Fog (tintwotin) on Oct 21 2019, 1:45 PM.

Details

Reviewers
William Reynish (billreynish)
Richard Antalik (ISS)
Germano Cavalcante (mano-wii)
Summary

Users are complaining the the Speed Effect panel is confusing, so here's a patch to simplify it, by keeping all options visible, but disabling those invalid with the current selection.

This suggestion has been developed with @david mcsween (davidmcsween) on the basis of the work of Paulo José Oliveira Amaro: https://blender.community/c/rightclickselect/zfbbbc/.

Before(gif):

After(gif):

It may be confusing to new users that Frame Number and Scale only produce a freeze frame unless animated, so I would suggest that the default setting is a check mark in the Speed checkbox and a value of 1. I'm not able to solve this myself, neither am I able to add much needed limits on the values in the value widget:

  • When in Frame Number, then limit to number of frames of the current strip.
  • When in Speed, limit to positive values.
  • When in Scale limit to values between 0 and 1(or use a % scale of 1->100).

Diff Detail

Event Timeline

Peter Fog (tintwotin) edited the summary of this revision. (Show Details)Oct 21 2019, 4:38 PM
This revision is now accepted and ready to land.Oct 21 2019, 4:40 PM
Peter Fog (tintwotin) updated this revision to Diff 19150.Oct 21 2019, 4:49 PM

Disable Multiply Speed when Fit to Duration is selected.

noki paike (amonpaike) added a subscriber: noki paike (amonpaike).Oct 23 2019, 9:25 PM

I noticed that these "ui animations" slow down a lot in eevee when we handle heavy scenes ... so in my opinion, this type fixes are welcome ...

This revision is now accepted and ready to land.Nov 6 2019, 11:26 AM
Germano Cavalcante (mano-wii) requested changes to this revision.Mon, Nov 25, 4:50 PM

I was not even able to test the patch:

release/scripts/startup/bl_ui/space_sequencer.py
1026

Instead of this complicated set up of conditions, wouldn't it be better to do so?

if strip.use_default_fade:
    label = "Auto"
elif strip.use_as_speed:
    label = "Speed Factor"
elif strip.use_scale_to_length:
    label = "Scale"
else:
    label = "Frame Number"
1041

How can we enable this property if it is not enabled?

This revision now requires changes to proceed.Mon, Nov 25, 4:50 PM
Peter Fog (tintwotin) added a comment.Mon, Nov 25, 5:11 PM

@Germano Cavalcante (mano-wii) The patch is now that old the several hunks where rejected, did you update the patch to test it? And if you did, could you share the corrected patch, so I can take a look?

Germano Cavalcante (mano-wii) added a comment.Mon, Nov 25, 5:20 PM

Honestly I am not very satisfied with this solution.
It does not follow a convention producing an unexpected result when more than one option is enabled.

If one option discards the other, then it is better to think of a different layout and change the rna property in C to become an enum.

I'm not sure how this effect works, but it seems to need to be rethought.

Richard Antalik (ISS) added a comment.Mon, Nov 25, 5:33 PM
In D6110#146413, @Germano Cavalcante (mano-wii) wrote:

Honestly I am not very satisfied with this solution.
It does not follow a convention producing an unexpected result when more than one option is enabled.
If one option discards the other, then it is better to think of a different layout and change the rna property in C to become an enum.
I'm not sure how this effect works, but it seems to need to be rethought.

Not sure if enum would be usable here, otherwise I agree in principle. I guess I should have looked at this at greater detail. Thanks for catch @Germano Cavalcante (mano-wii)

Peter Fog (tintwotin) added a comment.Mon, Nov 25, 6:46 PM

Using an enum would simplify things, but it needs to be coded in C, and that's outside of my abilities. If anyone wants to take it in that direction, please note the missing value limits, as mentioned above.

Eitan (EitanSomething) added a subscriber: Eitan (EitanSomething).EditedTue, Dec 3, 3:11 PM

I will implement this with enums

Revision Contents
Changeset List

Diff 19150

View Options

release/scripts/startup/bl_ui/space_sequencer.py

Loading...