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

Support merged AOVs in arnold render #72

Merged
merged 4 commits into from
Sep 5, 2024

Conversation

MustafaJafar
Copy link
Contributor

Changelog Description

Fixes #69

Test Runs

Here's a test run on my side.
The setup is shown in the ROP node parameters and the created files are shown in the removed files log.
image

Testing notes:

  1. Create AYON Arnold ROP publish instance.
  2. Configure the ROP node to have multiple AOVs and set them to separated files
  3. Publish. AOVs with separated files should be a separated publish instance.

@MustafaJafar MustafaJafar added type: bug Something isn't working sponsored This is directly sponsored by a client or community member labels Aug 20, 2024
@MustafaJafar MustafaJafar self-assigned this Aug 20, 2024
@kartikgupta
Copy link

Thannks @MustafaJafar and @BigRoy for looking into it.

There are parts of the pull request that I don't understand completely.
Is it trying to support separated AOVs, or combined AOVs?
Looking at the previous comments, the discussion seems to be revolving around how to resolve the aov file paths.

But what we're looking to get is that it shouldn't look for the AOV file paths at all and just integrate the overall file path mentioned in ar_picture attribute of the rop, instead of integrating each AOV.
Not sure if there's something I'm missing here.

@BigRoy
Copy link
Contributor

BigRoy commented Sep 4, 2024

But what we're looking to get is that it shouldn't look for the AOV file paths at all and just integrate the overall file path mentioned in ar_picture attribute of the rop, instead of integrating each AOV.
Not sure if there's something I'm missing here.

It should do that with the current changes as it would now ignore active AOVs that are not explicitly set to be a separate file.

Is it trying to support separated AOVs, or combined AOVs?

In the comments we're trying to make sure it does both correctly - but the part that allows combined AOVs should already have been solved looking at the code.

Anyway, I'll do some testing.

@kartikgupta
Copy link

Thanks @BigRoy for clearing up the misunderstanding!

Copy link
Contributor

@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.

This fixes at least being able render MERGED aovs.

We should follow up with a new issue and PR after this to solve the remaining TODO and the notes given in this comment.

Note that I have not tested whether Cryptomattes break the current "merged" AOV publishing so that remains unconfirmed.

@BigRoy BigRoy changed the title Support separated AOVs in arnold render Support non-separated AOVs in arnold render Sep 5, 2024
@BigRoy BigRoy changed the title Support non-separated AOVs in arnold render Support merged AOVs in arnold render Sep 5, 2024
@BigRoy BigRoy merged commit 7010326 into develop Sep 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sponsored This is directly sponsored by a client or community member type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arnold ROP AOV multichannel EXR
4 participants