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

Outliner rework of Material items: #2775

Merged
merged 5 commits into from
Dec 14, 2022
Merged

Conversation

JGamache-autodesk
Copy link
Collaborator

Right-click menu on prim when material is assigned.

  • Unbind Material should say Unassign Material

Right-click menu on any scope that matches the default scope name

  • Bind Material menu should be removed
  • Assign New Material should be changed to Add New Material

Right-click menu on material node when nothing else is selected

  • Assign Material to Selection should be hidden, not just disabled, if there's no selection
  • Bind Material menu should be removed
  • Assign New Material: should you be able to assign a material to a material? This should be removed

Right-click menu on material node when there is a selection

  • Assign New Material: should be removed

Letter-casing in Assign New Material sub-menus is incorrect

  • USD instead of Usd
  • glTF PBR instead of Gltf Pbr

Added unit test for the new "Add New Material" command.

Right-click menu on prim when material is assigned.
- Unbind Material should say Unassign Material

Right-click menu on any scope that matches the default scope name
- Bind Material menu should be removed
- Assign New Material should be changed to Add New Material

Right-click menu on material node when nothing else is selected
- Assign Material to Selection should be hidden, not just disabled, if there's no selection
- Bind Material menu should be removed
- Assign New Material: should you be able to assign a material to a material? This should be removed

Right-click menu on material node when there is a selection
- Assign New Material: should be removed

Letter-casing in Assign New Material sub-menus is incorrect
- USD instead of Usd
- glTF PBR instead of Gltf Pbr

Added unit test for the new "Add New Material" command.
Copy link
Collaborator

@pierrebai-adsk pierrebai-adsk left a comment

Choose a reason for hiding this comment

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

I think checking for pointers would make the code more resiliant to future changes and subtle error in logic or potential unexpected invalid program state.

Copy link
Collaborator

@frohnej-adsk frohnej-adsk left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -752,6 +752,21 @@ bool selectionSupportsShading()
return false;
}

#ifdef UFE_V3_FEATURES_AVAILABLE
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this function is unused and came back during the conflict resolution.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see it used at line 1211. The Linux and OS/X builds do not allow an unused function to exist. I agree it should not have suddenly appeared from conflict resolution since it was added a month ago in PR #2762 and should have been in the branch I started with. This is strange.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not strange finally. The code was added last month in a branch that was merged yesterday. This is why it appeared in as a conflict to resolve in my branch.

@JGamache-autodesk JGamache-autodesk added the ready-for-merge Development process is finished, PR is ready for merge label Dec 14, 2022
@seando-adsk seando-adsk merged commit e0badff into dev Dec 14, 2022
@seando-adsk seando-adsk deleted the gamaj/material_menu_rework branch December 14, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
outliner Features affecting Maya's Outliner ready-for-merge Development process is finished, PR is ready for merge ufe-usd Related to UFE-USD plugin in Maya-Usd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants