EMSUSD-1134 export assigned materials #3786
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Modify the export command: - Add new export flag "exportAssignedMaterials" (-ama). - Default to be turned on. - Expose it in Python. - Document it. - Add a checkbox UI for it in the export options. Modify how material assignments are found: - Bundle all informations about assignments for a material in a single struct. - Add a flag to the info set if the material is used anywhere, even by meshes not being exported. - Add a flag to the info set if the material is used by something in the selection. - Also check any ancestor of the mesh for the selection inclusion since selection in the viewport selects the parent transform. - Also, by default, the outliner shows the parent transforms, not the shape/mesh, so selection there would also select an ancestor. - Modify the GetAssignments function to fill those flags. Modify how materials are filtered: - In the filter function for materials, add the logic for assigned-materials. - Use the flags about assignments and assignments-in-selection to determine if a material should be exported. - When exporting assigned materials, if the material is not used at all, then the material is not exported. - When exporting the selection, if a material was used by something in the selection, export it even if the user was something not exported, like when the meshes are off. Add unit tests for when assigned materials are off. (The case when it is on was already tested by existing tests.)
pierrebai-adsk
commented
May 27, 2024
| for (MDagPath walkUpPath(dagPath); walkUpPath.length() > 0; walkUpPath.pop()) | ||
| if (GetExportArgs().fullObjectList.hasItem(walkUpPath)) | ||
| ret.hasAnyAssignmentInSelection = true; | ||
|
|
There was a problem hiding this comment.
To support the logic of determining if a material should be exported, we need to know if it is used in the scene and if it used by something that was selected. This is what the new code above does.
barbalt
reviewed
May 29, 2024
| // contains the transform above the mesh or a higher-up ancestor, | ||
| // so also check the ancestors of the mesh. | ||
| for (MDagPath walkUpPath(dagPath); walkUpPath.length() > 0; walkUpPath.pop()) | ||
| if (GetExportArgs().fullObjectList.hasItem(walkUpPath)) |
There was a problem hiding this comment.
Is there a policy in code guidelines for one line if cases like if they should have {} or not? I couldn't find one. This is minor, anyways
There was a problem hiding this comment.
There is no policy for this (no {} is allowed), other than the formatting done by clang-format.
barbalt
approved these changes
May 29, 2024
pierrebai-adsk
added
the
ready-for-merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Modify the export command:
Modify how material assignments are found:
Modify how materials are filtered:
Add unit tests for when assigned materials are off. (The case when it is on was already tested by existing tests.)