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

Add test cases for material fanned out file nodes #3403

Conversation

AramAzhari-adsk
Copy link
Collaborator

This is a follow up to #3366 to add test cases.
There is also a minor change from MObjectArray to MObjectHandleUnorderedSet.

The test scene contains 5 cubes that cover various materials:

File Node fanning out to two other File Nodes to the same Material (Lambert):
image

File Node fanning out to two other File Nodes to the same Material (Standard Surface):
image

File Node fanning out to two other File Nodes to the same Material (USD Preview Surface):
image

File Node fanning out to two other File Nodes to different Materials (Lambert):
image

Copy link
Collaborator

@JGamache-autodesk JGamache-autodesk left a comment

Choose a reason for hiding this comment

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

This is not actually testing the fix you implemented, which was about stray file nodes.

Please add checks that:

1 - For UsdPreviewSurface exports:
file13 and file15 are present for lambert4, file14 is not
file16 and file18 are present for standardSurface2, file17 is not
....
file10 is present for lambert3, file11 and file12 are not
file12 is present for lambert2, file10 and file11 are not

2- For MaterialX exports:
file13, file14 and file15 are present for lambert4
file16, file17 and file18 are present for standardSurface2
....
file10 and file11 are present for lambert3, file12 is not
file11 and file12 are present for lambert2, file10 is not

lib/mayaUsd/fileio/shading/shadingModeUseRegistry.cpp Outdated Show resolved Hide resolved
test/lib/usd/translators/CMakeLists.txt Outdated Show resolved Hide resolved
@AramAzhari-adsk
Copy link
Collaborator Author

Updated the test logic to correctly check for nodes that must be included and the nodes that must not be included. I also locally changed the list to make sure it is functioning properly.

Note: The purpose of the fix in #3366 was to prevent adding nodes that do not exist in Maya's dependency graph for each material. The fix or its test does not cover cases where an unsupported connection is not translated onto USD, which may end up containing zombie nodes that are not connected to anything. That is a separate issue and I will create a ticket to have it fixed based on priority.

@AramAzhari-adsk AramAzhari-adsk added the unit test Related to unit tests (both python or c++) label Oct 25, 2023
@AramAzhari-adsk AramAzhari-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Oct 26, 2023
@seando-adsk seando-adsk merged commit 179a79d into dev Oct 26, 2023
11 checks passed
@seando-adsk seando-adsk deleted the azharia/EMSUSD-692/Add-Test-Cases-For-Material-FannedOut-File-Nodes branch October 26, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge unit test Related to unit tests (both python or c++)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants