Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Fusion - Resolution and frame range validators #4921

Conversation

EmberLightVFX
Copy link
Collaborator

@EmberLightVFX EmberLightVFX commented Apr 29, 2023

Changelog Description

This PR adds two validators. One for each savers resolution and one for the comps frame range.

Additional info

Both are optional as you might want/need to render out different resolutions and frame ranges.
I also made sure that when selecting problematic nodes in the publisher, the node-view now moves to that node.

Testing notes:

  1. Make a comp with wrong range range
  2. Feed a saver footage with non-project resolution
  3. The validator catches the problems.

@ynbot ynbot added size/S Denotes a PR changes 100-499 lines, ignoring general files host: Fusion type: feature Larger, user affecting changes and completely new things labels Apr 29, 2023
Copy link
Collaborator

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some notes

@EmberLightVFX
Copy link
Collaborator Author

EmberLightVFX commented Apr 30, 2023

A bunch of fixes and updates!
The biggest update is for the validate frame range. It checks the global in/out as expected. If it matches the assets range you're good to go and the render range will be fit to that range when the render starts.

But if you disable the validation Fusion will render the set render range.
One problem that comes after this is if you want to create a review file it will expect the assets full range and error out if not all files exist.
To solve this I have moved the Collect expected frames to the last step after all validations. I know it's wrong as it's not a validator but it was that or duplicate all its code inside of the Validate frame range. Any thoughts of that?

The only "bug" I have found so far is if you generate reviews with burnins the current frame isn't the correct current frame as the first frame is always the the first expected frame in the assets frame range and maybe not where your custom render range started. I don't find any real way to solve this as that part is hardcoded in the create burning plugin from what I understood of the code.

95% of all the times the artist want to render the whole comp. If they don't want they should enable "Fusion's current frame range" for the current context.

Handles are calculated in "collect comp frame range" so the logged data is correct.
@EmberLightVFX
Copy link
Collaborator Author

Now when I could add a drop-down list to the collector the validator no longer need to also work as the collector!
So the user can now pick between:

"Asset's frame range" uses the frame range set to your opened asset.
"Fusion's current frame range" will render out the frame range you manually have selected.
The handles will both be set to 0 for this one.

The validator checks if the comp currently cover the savers expected frame range.

@EmberLightVFX EmberLightVFX requested a review from BigRoy May 4, 2023 14:29
@EmberLightVFX EmberLightVFX requested a review from Minkiu May 10, 2023 17:11
Copy link
Contributor

@Minkiu Minkiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code wise, I'd say it looks good!

@mkolar
Copy link
Member

mkolar commented May 30, 2023

@BigRoy what's your take on this?

@BigRoy
Copy link
Collaborator

BigRoy commented May 31, 2023

@BigRoy what's your take on this?

With recent changes to how frames are collected for rendering and now being able to set a frame range per instance (or at least tweak where it gets the frame range from) I suspect a lot of this PR is now obsolete or in a big conflict with the behavior.

I think the resolution validator is nice. We might need to just create a new PR with only related changes that still apply to current develop? What do you think @EmberLightVFX ?

@EmberLightVFX
Copy link
Collaborator Author

Yeah I need to look over the frame-range validator. Does the new change add a frame-range validator?
I'm quite busy with a project currently but when I get some time over I'll jump back into this one

@EmberLightVFX
Copy link
Collaborator Author

As the frame range validator isn't needed any longer I made a new PR that only contains the resolution validator here: #5325

@ynbot ynbot added this to the next-patch milestone Jul 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community contribution host: Fusion size/S Denotes a PR changes 100-499 lines, ignoring general files type: feature Larger, user affecting changes and completely new things
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants