Add contextual menu to bind materials #2287
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
- Adds a bind/unbind contextual menu to the outliner. The menu lists available materials to bind. - Extends the create reference menu to list all asset types that can be referenced. Includes MaterialX and Alembic files.
| // | ||
| // TODO: Show only materials that are inside of the asset's namespace otherwise there | ||
| // will be "refers to a path outside the scope" errors. See | ||
| // https://groups.google.com/g/usd-interest/c/dmjV5bQBKIo/m/LeozZ3k6BAAJ |
There was a problem hiding this comment.
Any idea how to do this?
| @@ -48,6 +48,9 @@ Ufe::ConstAttributeDefs getAttrs(const PXR_NS::SdrShaderNodeConstPtr& shaderNode | |||
| for (const PXR_NS::TfToken& name : names) { | |||
| PXR_NS::SdrShaderPropertyConstPtr property | |||
| = input ? shaderNodeDef->GetShaderInput(name) : shaderNodeDef->GetShaderOutput(name); | |||
| if (!property) { | |||
| continue; | |||
There was a problem hiding this comment.
I have third-party shaders crashing here. They registered the input names, but did not register a SdrShaderProperty.
There was a problem hiding this comment.
That would be a great code comment.
ppt-adsk
previously approved these changes
Apr 14, 2022
There was a problem hiding this comment.
Looks good to me! Just minor comments. Material discovery traversals are a bit worrisome, I will say, especially if this is used in a context where UI must be displayed synchronously, but I will assume that you feel the typical scene hierarchy where this will be used will not cause performance problems.
| // | ||
| // TODO: Show only materials that are inside of the asset's namespace otherwise there | ||
| // will be "refers to a path outside the scope" errors. See | ||
| // https://groups.google.com/g/usd-interest/c/dmjV5bQBKIo/m/LeozZ3k6BAAJ |
lib/mayaUsd/ufe/UsdContextOps.cpp
Outdated
| // TODO: Introduce material binding via collections API | ||
| // | ||
| auto stage = fItem->prim().GetStage(); | ||
| for (UsdPrim currentPrim : stage->TraverseAll()) { |
There was a problem hiding this comment.
How expensive can this traversal end up being?
| @@ -48,6 +48,9 @@ Ufe::ConstAttributeDefs getAttrs(const PXR_NS::SdrShaderNodeConstPtr& shaderNode | |||
| for (const PXR_NS::TfToken& name : names) { | |||
| PXR_NS::SdrShaderPropertyConstPtr property | |||
| = input ? shaderNodeDef->GetShaderInput(name) : shaderNodeDef->GetShaderOutput(name); | |||
| if (!property) { | |||
| continue; | |||
There was a problem hiding this comment.
That would be a great code comment.
- Traversing large scenes is not an option. Instead we traverse the selection to find materials either directly selected, or under a selected parent (Looks section). The workflow is now: - Select the materials or the looks section - Navigate in the outliner to the geometry you want to bind, and activate the context menu (this does not change selection). - If materials are found in the selection, they will be offered in a Bind Material context menu item. In UX terms, this forces you to pre-select the "object" before right clicking on the "subject" to get the Bind "verb" as an option. This is far from perfect, so I am open to any other solution that does not process the whole stage to find materials.
ppt-adsk
previously approved these changes
Apr 20, 2022
feldstj
previously approved these changes
Apr 20, 2022
d764d99
ppt-adsk
approved these changes
Apr 20, 2022
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.
available materials to bind.
be referenced. Includes MaterialX and Alembic files.