Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alembic Exports: Do not force a preroll starting at frame 0 #151

Conversation

BigRoy
Copy link
Contributor

@BigRoy BigRoy commented Oct 16, 2024

Changelog Description

Fixes Alembic Exports always defaulting to include a preroll run-up to the export from the preRollStartFrame, which defaulted to 0.

Also fixes this warning always being logged:

# Warning: Ignoring unsupported flag: -verbose

Additional info

The "pre roll" was not included in the exported data - but it did mean that for EVERY alembic export we basically ended up stepping through the frame ranges from frame 0 up to the start frame of the Alembic to export.

I'd say this is actually quite an inconvenience for scenes that happen to also be heavy to step through time before the start frame because it'd heavily influence the time taken if you actually did not care about pre roll at all.

The bug was introduced around April/May via these PRs:

I believe it all came from a misconception about what the preRoll argument on the AbcExport command was supposed to do. To explain that I'll just quote what I wrote as comment in the code for this PR:

Our logic is that preroll means:

  • True: include a preroll from preRollStartFrame to the start
    frame that is not included in the exported file. Just 'roll up'
    the export from there.
  • False: do not roll up from preRollStartFrame.

AbcExport however approaches this very differently.

A call to AbcExport allows to export multiple "jobs" of frame ranges in one go. Using -preroll argument there means: this one job in the full list of jobs SKIP writing these frames into the Alembic.

In short, marking that job as just preroll.

Then additionally, separate from -preroll the AbcExport command allows to supply preRollStartFrame which, when not None, means always RUN UP from that start frame. Since our preRollStartFrame is always an integer attribute we will convert the attributes so they behave like how we intended them initially

Testing notes:

How to debug

The easiest way to test this is to enable "Verbose" in settings: ayon+settings://maya/publish/ExtractAlembic/verbose
image

You will then end up seeing the frame numbers printed by the Alembic Exporter - e.g. this:
image

Going all the way from frame zero.

What to test

  1. Alembic Exports should not default to prerolling from frame 0.
  2. Enabling preroll on Alembic export for settings should still allow enabling the preroll.

Others that would be affected also calling extract_alembic and should now also NOT preroll from frame 0 but they did before this PR:

  • Extract Assembly
  • Extract Proxy ABC
  • Extract Unreal SkeletalMesh Abc
  • XGEN extractions with a Render (it was called here.

…mbic Extractor

This fixes the extractor ALWAYS prerolling from `preRollStartFrame` even if `preRoll` was `False` hence disallowing the `preRollStartFrame` to EVER be disabled.
@BigRoy BigRoy added the type: bug Something isn't working label Oct 16, 2024
@BigRoy BigRoy self-assigned this Oct 16, 2024
Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

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

I have made first test with latest ayon-maya addon to see if starting at zero frame for reference...yes it happened.

Once using this PR branch all works nicely and publishing start and start frame e.g. my 1001 which is desired.

Also been testing with Use Preroll enabled and set some custom start frame...again all working nicely starting at predefined frame.

All working fine and looks ok.

Copy link
Member

@moonyuet moonyuet left a comment

Choose a reason for hiding this comment

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

Code looks good for me

@iLLiCiTiT iLLiCiTiT merged commit d93d670 into ynput:develop Oct 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants