Sequencer: Improve motion-picture workflow #82755

New Issue

Sergey Sharybin · 2020-11-16T11:03:36+01:00

Sergey Sharybin commented

2020-11-16 11:03:36 +01:00

The media transform has been improved a lot for "sequencing" type of workflow in D8393. Unfortunately, one of the usecases became way more tedious: put source at its original resolution to the sequencer. This is something what is useful for motion-picture, cinematics, and similar situations. It seems like we can easily support this within the new transform design.

Here is the updated and implemented design proposal:

The scale factor will become relative to the image source (100% means the original image pixel size - currently it was implemented relative to the render resolution).
When changing the render resolution, all media present in the canvas will not be visually transformed in any way (decreasing the render resolution could make content appear cropped, and that's ok)
3 operators will be added to the Transform menu, under "Scale":
- Fit to Frame
- Fill Frame
- Stretch to frame
A similar behavior should be added to the "Add Image/Video Strip" operator (and it should be persisted on future executions of the operator)
An operator setting dropdown should be added, to better expose this functionality (see mockup)
An operator to perform scaling using different measurement units will be added near the scale property (this is part of a different task)

The media transform has been improved a lot for "sequencing" type of workflow in [D8393](https://archive.blender.org/developer/D8393). Unfortunately, one of the usecases became way more tedious: put source at its original resolution to the sequencer. This is something what is useful for motion-picture, cinematics, and similar situations. It seems like we can easily support this within the new transform design. Here is the updated and implemented design proposal: * The scale factor will become relative to the image source (100% means the original image pixel size - currently it was implemented relative to the render resolution). * When changing the render resolution, all media present in the canvas will not be visually transformed in any way (decreasing the render resolution could make content appear cropped, and that's ok) * 3 operators will be added to the Transform menu, under "Scale": * Fit to Frame * Fill Frame * Stretch to frame * A similar behavior should be added to the "Add Image/Video Strip" operator (and it should be persisted on future executions of the operator) * An operator setting dropdown should be added, to better expose this functionality (see mockup) * An operator to perform scaling using different measurement units will be added near the scale property (this is part of a different task) ![Screenshot 2020-11-19 at 12.24.52.png](https://archive.blender.org/developer/F9324681/Screenshot_2020-11-19_at_12.24.52.png)

Sergey Sharybin self-assigned this 2020-11-16 11:03:36 +01:00

Sergey Sharybin commented

2020-11-16 11:03:36 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Sergey Sharybin commented

2020-11-16 11:03:36 +01:00

Added subscribers: @fsiddi, @Sergey, @iss

tintwotin commented

2020-11-16 12:25:42 +01:00

Added subscriber: @tintwotin

tintwotin commented

2020-11-16 12:25:42 +01:00

Related: D2506 - but for Transform strips.

Related: [D2506](https://archive.blender.org/developer/D2506) - but for Transform strips.

Richard Antalik commented

2020-11-17 07:40:10 +01:00

My proposal is:
Add Scale To Fit checkbox to transform panel. It would either scale image to fit preview(default) or render at original size. See demo here:
0001-1413.mp4
This should resolve following requirements:

Reset scale so that the strip fully fills the "canvas" (similar of default behavior of VSE in Blender 2.91)
Reset scale to make strip to have same size in the scene pixels as the source (similar to behavior of VSE in Blender 2.91 when you enable Crop option)

And add operator strip_transform_clear to menu with 4 options - Clear position, scale, rotation and all (previous 3 options combined). See demo here:
0001-3110.mp4
This should resolve reseting to default value

Since we already had Strip -> Transform menu, I created menu item Image Transform. I think this may be a bit confusing.
In 3D viewport we use Alt+R, Alt+S, Alt+G to clear object transformation, we could use these in sequencer, but Alt+R is occupied by reload operator. We can also use only one key in sequencer and then adjust operation "adjust last action" panel. I haven't assigned keyboard shortcut yet, so I am open to suggestions. I would probably use Alt+T as it is free.

If there are no objections I can post patch for review.

My proposal is: Add `Scale To Fit` checkbox to transform panel. It would either scale image to fit preview(default) or render at original size. See demo here: [0001-1413.mp4](https://archive.blender.org/developer/F9308428/0001-1413.mp4) This should resolve following requirements: > Reset scale so that the strip fully fills the "canvas" (similar of default behavior of VSE in Blender 2.91) > Reset scale to make strip to have same size in the scene pixels as the source (similar to behavior of VSE in Blender 2.91 when you enable Crop option) And add operator `strip_transform_clear` to menu with 4 options - Clear position, scale, rotation and all (previous 3 options combined). See demo here: [0001-3110.mp4](https://archive.blender.org/developer/F9308450/0001-3110.mp4) This should resolve reseting to default value Since we already had `Strip -> Transform` menu, I created menu item `Image Transform`. I think this may be a bit confusing. In 3D viewport we use Alt+R, Alt+S, Alt+G to clear object transformation, we could use these in sequencer, but Alt+R is occupied by reload operator. We can also use only one key in sequencer and then adjust operation "adjust last action" panel. I haven't assigned keyboard shortcut yet, so I am open to suggestions. I would probably use Alt+T as it is free. If there are no objections I can post patch for review.

tintwotin commented

2020-11-17 08:41:25 +01:00

@iss I think your Scale to Fit is called Scale to Fill(in the first video), whereas your Clear All is Scale to Fit.
{F9308777, size=full}
If you want to introduce presets for scaling, I think the most common options would be:
Scale to Fit
Scale to Fill
Stretch to Fill
Source - the full source size, unscaled.
Custom - enables Scale X/Y

On the Clear Transform menu, these functions could go into the panel too, but the question is if these transform options should be expanded into the Preview Toolbar too? If that is the case, these elements should go into the Preview header menu, like in the Transform Tools add-on(my fork: https://github.com/tin2tin/VSE_Transform_Tools (works in 2.8 > 2.91)):
{F9308889, size=full}

@iss I think your `Scale to Fit` is called `Scale to Fill`(in the first video), whereas your `Clear All` is `Scale to Fit`. {[F9308777](https://archive.blender.org/developer/F9308777/image.png), size=full} If you want to introduce presets for scaling, I think the most common options would be: `Scale to Fit` `Scale to Fill` `Stretch to Fill` `Source` - the full source size, unscaled. `Custom` - enables Scale X/Y On the `Clear Transform` menu, these functions could go into the panel too, but the question is if these transform options should be expanded into the Preview Toolbar too? If that is the case, these elements should go into the Preview header menu, like in the Transform Tools add-on(my fork: https://github.com/tin2tin/VSE_Transform_Tools (works in 2.8 > 2.91)): {[F9308889](https://archive.blender.org/developer/F9308889/image.png), size=full}

Richard Antalik commented

2020-11-17 08:49:20 +01:00

In #82755#1055958, @tintwotin wrote:
@iss I think your Scale to Fit is called 'Scale to Fill'(in the first video), whereas your Clear All is Scale to Fit.

No, it's scale to fit, but there is already some scale (see properties). I guess I wanted to demonstrate that this property is unchanged, but in the end it was only confusing. Sorry.

Scale to fill sounds also useful, as well as Stretch to Fill. I can implement this very easily and use enum instead of checkbox. I would like to hear feedback from @fsiddi as well though.

> In #82755#1055958, @tintwotin wrote: > @iss I think your `Scale to Fit` is called 'Scale to Fill'(in the first video), whereas your `Clear All` is `Scale to Fit`. No, it's scale to fit, but there is already some scale (see properties). I guess I wanted to demonstrate that this property is unchanged, but in the end it was only confusing. Sorry. Scale to fill sounds also useful, as well as Stretch to Fill. I can implement this very easily and use enum instead of checkbox. I would like to hear feedback from @fsiddi as well though.

Sergey Sharybin commented

2020-11-17 09:48:51 +01:00

The description of operator seems fine to me, aligned with what we've briefly talked about with Francesco here.

The option in the transform panel I don't think it should exist. It introduces secondary setting which defines resolution of strip. For the ease of code and mental model, I'd say the strip scale is the source of truth. For the convenience purposes we might add a button/easy-to-access-menu next to the scale slider, and make that to run an operator.

The description of operator seems fine to me, aligned with what we've briefly talked about with Francesco here. The option in the transform panel I don't think it should exist. It introduces secondary setting which defines resolution of strip. For the ease of code and mental model, I'd say the strip scale is the source of truth. For the convenience purposes we might add a button/easy-to-access-menu next to the scale slider, and make that to run an operator.

Richard Antalik commented

2020-11-17 10:00:34 +01:00

In #82755#1055983, @Sergey wrote:
The description of operator seems fine to me, aligned with what we've briefly talked about with Francesco here.

The option in the transform panel I don't think it should exist. It introduces secondary setting which defines resolution of strip. For the ease of code and mental model, I'd say the strip scale is the source of truth. For the convenience purposes we might add a button/easy-to-access-menu next to the scale slider, and make that to run an operator.

So it should be operator that would change scale instead? That would violate "Be fully flexible in terms of replacing source: artist can replace media source to higher/lower resolution and have composition intact!"

What I am doing now doesn't look that terrible actually:

diff --git a/source/blender/sequencer/intern/render.c b/source/blender/sequencer/intern/render.c
index 008ea1cd3a0..5b28584c816 100644
--- a/source/blender/sequencer/intern/render.c
+++ b/source/blender/sequencer/intern/render.c
@@ -625,9 +625,12 @@ static ImBuf *input_preprocess(const SeqRenderData *context,
     IMB_filtery(preprocessed_ibuf);
   }

-  /* Calculate scale factor, so image fits in preview area with original aspect ratio. */
-  const float scale_to_fit_factor = MIN2((float)context->rectx / (float)ibuf->x,
-                                         (float)context->recty / (float)ibuf->y);
+  float scale_to_fit_factor = 1.0f;
+  if ((seq->strip->transform->flag & SEQ_TRANSFORM_SCALE_TO_FIT) != 0) {
+    /* Calculate scale factor, so image fits in preview area with original aspect ratio. */
+    scale_to_fit_factor = MIN2((float)context->rectx / (float)ibuf->x,
+                               (float)context->recty / (float)ibuf->y);
+  }

> In #82755#1055983, @Sergey wrote: > The description of operator seems fine to me, aligned with what we've briefly talked about with Francesco here. > > The option in the transform panel I don't think it should exist. It introduces secondary setting which defines resolution of strip. For the ease of code and mental model, I'd say the strip scale is the source of truth. For the convenience purposes we might add a button/easy-to-access-menu next to the scale slider, and make that to run an operator. So it should be operator that would change scale instead? That would violate "Be fully flexible in terms of replacing source: artist can replace media source to higher/lower resolution and have composition intact!" What I am doing now doesn't look that terrible actually: ``` diff --git a/source/blender/sequencer/intern/render.c b/source/blender/sequencer/intern/render.c index 008ea1cd3a0..5b28584c816 100644 --- a/source/blender/sequencer/intern/render.c +++ b/source/blender/sequencer/intern/render.c @@ -625,9 +625,12 @@ static ImBuf *input_preprocess(const SeqRenderData *context, IMB_filtery(preprocessed_ibuf); } - /* Calculate scale factor, so image fits in preview area with original aspect ratio. */ - const float scale_to_fit_factor = MIN2((float)context->rectx / (float)ibuf->x, - (float)context->recty / (float)ibuf->y); + float scale_to_fit_factor = 1.0f; + if ((seq->strip->transform->flag & SEQ_TRANSFORM_SCALE_TO_FIT) != 0) { + /* Calculate scale factor, so image fits in preview area with original aspect ratio. */ + scale_to_fit_factor = MIN2((float)context->rectx / (float)ibuf->x, + (float)context->recty / (float)ibuf->y); + } ```

Sergey Sharybin commented

2020-11-17 10:18:57 +01:00

That would violate "Be fully flexible in terms of replacing source: artist can replace media source to higher/lower resolution and have composition intact!"

I am not sure how? This isn't something what I expect from scale to behave, and here is a test I did:

Added 512 x 512 image as an image strip
Set scale to 0.64 by 0.32.
Replaced the image with 4k x 4k.
The composition stayed the same.

This is exactly behavior I would expect :)

> That would violate "Be fully flexible in terms of replacing source: artist can replace media source to higher/lower resolution and have composition intact!" I am not sure how? This isn't something what I expect from scale to behave, and here is a test I did: * Added 512 x 512 image as an image strip * Set scale to 0.64 by 0.32. * Replaced the image with 4k x 4k. * The composition stayed the same. This is exactly behavior I would expect :)

Richard Antalik commented

2020-11-17 10:40:07 +01:00

Ok I think I misunderstood this, will update patch with new operator and add more scale to X features proposed by @tintwotin while I am at it.

Ok I think I misunderstood this, will update patch with new operator and add more `scale to X` features proposed by @tintwotin while I am at it.

tintwotin commented

2020-11-17 11:28:40 +01:00

As for the Clear Transform operators, reset buttons in the panels are not common, but in the keymap preferences they look like this:

As for the `Clear Transform` operators, reset buttons in the panels are not common, but in the keymap preferences they look like this: ![image.png](https://archive.blender.org/developer/F9310132/image.png)

Francesco Siddi commented

2020-11-17 12:58:17 +01:00

Thank you @Sergey for getting this started. I'm evaluating different possible scenarios/workflows currently (provide an operator to alter scale as it's been suggested here, expose the original image dimensions, etc.) and will come with a proposal soon.

Francesco Siddi commented

2020-11-19 12:25:24 +01:00

After a conversation with a few artists and developers, there is an agreement to revise the implementation of scale as follows.

The scale factor will become relative to the image source (100% means the original image pixel size - currently it was implemented relative to the render resolution).
When changing the render resolution, all media present in the canvas will not be visually transformed in any way (decreasing the render resolution could make content appear cropped, and that's ok)
3 operators will be added to the Transform menu, under "Scale":
- Fit to Frame
- Fill Frame
- Stretch to frame
A similar behavior should be added to the "Add Image/Video Strip" operator (and it should be persisted on future executions of the operator)
An operator setting dropdown should be added, to better expose this functionality (see mockup)
An operator to perform scaling using different measurement units will be added near the scale property (this is part of a different task)

Thanks to everyone for the feedback!

After a conversation with a few artists and developers, there is an agreement to revise the implementation of scale as follows. * The scale factor will become relative to the image source (100% means the original image pixel size - currently it was implemented relative to the render resolution). * When changing the render resolution, all media present in the canvas will not be visually transformed in any way (decreasing the render resolution could make content appear cropped, and that's ok) * 3 operators will be added to the Transform menu, under "Scale": * Fit to Frame * Fill Frame * Stretch to frame * A similar behavior should be added to the "Add Image/Video Strip" operator (and it should be persisted on future executions of the operator) * An operator setting dropdown should be added, to better expose this functionality (see mockup) * An operator to perform scaling using different measurement units will be added near the scale property (this is part of a different task) ![Screenshot 2020-11-19 at 12.24.52.png](https://archive.blender.org/developer/F9324681/Screenshot_2020-11-19_at_12.24.52.png) Thanks to everyone for the feedback!

Sergey Sharybin commented

2020-12-03 16:47:55 +01:00

@fsiddi, thanks for the updated design. Can you please make sure the final design is moved to the task description, so it's always easy to find?