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

Maya: keep existing AOVs when creating render instance #4087

Conversation

moonyuet
Copy link
Member

@moonyuet moonyuet commented Nov 10, 2022

Brief description

Introduce an option to allow users to choose if existing AOVs should be kept when creating render instance in Openpype setting.
The setting is se to false by default
image

Description

The new options in Openpype Setting named "remove_AOVs" is avaliable in this PR. If this is set to be True, it would clean up the previous AOVs which is left by the deleted render instance. If this is set to be False, It would still store the existing AOVs as renderpasses.

Additional info

Nothing specific

Testing notes:

  1. In Openpype Settings > Maya> Render Settings > Arnold Renderer, there is a on/off options of remove_AOVs
  2. If you enable it, it would help you to clean up the existing AOVs if you create a new render instance.
  3. if you disable it, it would keep the existing AOVs while creating new AOVs without duplication of existing AOVs.

resolves #4073

@ynbot
Copy link
Contributor

ynbot commented Nov 10, 2022

@github-actions github-actions bot added this to the next-patch milestone Nov 10, 2022
@jakubjezek001 jakubjezek001 removed this from the next-patch milestone Nov 10, 2022
Copy link
Member

@m-u-r-p-h-y m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

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

Tested with M2023 and Arnold 5.2.0.1

working perfectly as expected.

I would have some UI and useability questions though

  1. I would recommend changing the label to a more descriptive Remove existing AOVs

  2. This option should be available for all renderers (it is not Arnold specific)

should the button be placed in each renderer section or in the common, upper level?
image

In case we decide to keep the option per renderer we should just swap the order and keep all AOV related options together

image

@antirotor @mkolar what do you think?

@antirotor
Copy link
Member

I don't think it should be per renderer - and I agree with changing it to Remove existing AOVs

@moonyuet
Copy link
Member Author

I don't think it should be per renderer - and I agree with changing it to Remove existing AOVs

I have renamed in the updated PR as well as swapping the position of remove existing AOVs attribute with the tiled attribute. The remove existing AOVs is now located right before the AOVs to create attribute

@m-u-r-p-h-y
Copy link
Member

so for now is it only valid for Arnold?

@m-u-r-p-h-y
Copy link
Member

I don't think it should be per renderer - and I agree with changing it to Remove existing AOVs

so the settings should be like this now

image

and the setting of removing AOV's should be valid for all renderers below.

@moonyuet
Copy link
Member Author

moonyuet commented Nov 14, 2022

I don't think it should be per renderer - and I agree with changing it to Remove existing AOVs

so the settings should be like this now

image

and the setting of removing AOV's should be valid for all renderers below.

I have moved the remove existing AOVs setting to the Render Settings

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.

tested in maya2022 and maya2023 with Arnold 5.1.33
All working as expected! No issues found...

Copy link
Member

@m-u-r-p-h-y m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

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

there is some issue with vray

# Error: KeyError: file D:\REPO\OpenPype\openpype\hosts\maya\api\lib_rendersettings.py line 179: 'remove_aovs'

and with redshift nothing is happening when running Set Render Settings or creating a Render Instance

@moonyuet
Copy link
Member Author

there is some issue with vray

# Error: KeyError: file D:\REPO\OpenPype\openpype\hosts\maya\api\lib_rendersettings.py line 179: 'remove_aovs'

and with redshift nothing is happening when running Set Render Settings or creating a Render Instance

The updated PR has fixed the error of removing aovs as well as removing the light-select aov

@mkolar mkolar changed the title Bugfix/op 4381 maya keep existing AOVs when creating render instance Maya: keep existing AOVs when creating render instance Dec 5, 2022
Copy link
Member

@m-u-r-p-h-y m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

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

there is no error popping up with vray, but it is not fully working

only image prefix and output image format is set when creating the render instance and no other options are applied (no aovs, no engine, etc.)

the same issue is with Redshift

@moonyuet
Copy link
Member Author

moonyuet commented Dec 7, 2022

This is mainly because the options of creating RedshiftAOV and Vray Render Element haven't been set up despite the existence of all the related AOV attributes. The updated PR will set them up if there is no accident
image
image

@antirotor antirotor added the type: enhancement Enhancements to existing functionality label Dec 7, 2022
@LiborBatek
Copy link
Member

Did run new tests in maya with arnold and redshift and lastly with vray

Regarding OP settings for AOVs (creation / deleting existing ones) All works fine in all three renderers.

Succesfuly adds predefined AOVs in OP settings and also reacts to "Remove Existing AOVs" switch too...

Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

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

Looks good.

Warning
Merging this needs updating v4 settings

Copy link
Member

@m-u-r-p-h-y m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

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

Vray works ok, but there is still an issue with Redshift.

tested with Maya2022.4 Redshift 3.5.09

It only breaks if one of the required AOVs is Beauty. Probably connected to Beauty -> BeautyAux name change in 3.0.67?
image

keepAOVSettings

Other render settings are not set correctly either

image

@moonyuet
Copy link
Member Author

moonyuet commented Dec 12, 2022

BeautyAux

I get the aov data by types from redshift, so the naming doesn't affect the code adding aov attributes, but rather I didn't update the selection data(i.e. cmds.ls(type="redshiftAOV"), you can notice the naming attribute of Beauty havem't changed.
See the command below:
image

Besides, the global illumination has some adjustments in the latest update. There are only two options for both primary and secondary GI Engine, Photon Map seemingly not supported in this version so I've removed it in the openpype settings. But I still keep None option to make sure GI would be switched off if the two engines are closed at the same occasion. When either one is set to None, it will set back to default options.

Copy link
Member

@m-u-r-p-h-y m-u-r-p-h-y left a comment

Choose a reason for hiding this comment

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

Redshift 3.5.12
Maya 2022.4

AOVs work as expected

@antirotor antirotor merged commit 3828d11 into ynput:develop Dec 12, 2022
@github-actions github-actions bot added this to the next-patch milestone Dec 12, 2022
@moonyuet moonyuet deleted the bugfix/OP-4381-Maya--Keep-existing-AOVs-when-creating-render-instance branch December 13, 2022 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
host: Maya type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maya: Keep existing AOVs when creating render instance
6 participants