From d72529bb75d4cf924f30b8240c151a486ee4b1f4 Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Mon, 12 Dec 2022 15:06:21 -0500 Subject: [PATCH 1/4] Outliner rework of Material items: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- lib/mayaUsd/ufe/UsdContextOps.cpp | 84 ++++--- lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp | 241 ++++++++++++++++---- lib/mayaUsd/ufe/UsdUndoMaterialCommands.h | 46 +++- test/lib/ufe/testContextOps.py | 79 ++++++- 4 files changed, 379 insertions(+), 71 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdContextOps.cpp b/lib/mayaUsd/ufe/UsdContextOps.cpp index 00f20a95e1..0d39222493 100644 --- a/lib/mayaUsd/ufe/UsdContextOps.cpp +++ b/lib/mayaUsd/ufe/UsdContextOps.cpp @@ -139,6 +139,8 @@ static constexpr char kBindMaterialToSelectionLabel[] = "Assign Material to Sele #if UFE_PREVIEW_VERSION_NUM >= 4010 static constexpr char kAssignNewMaterialItem[] = "Assign New Material"; static constexpr char kAssignNewMaterialLabel[] = "Assign New Material"; +static constexpr char kAddNewMaterialItem[] = "Add New Material"; +static constexpr char kAddNewMaterialLabel[] = "Add New Material"; static constexpr char kAssignNewUsdMaterialItem[] = "USD Material"; static constexpr char kAssignNewUsdMaterialLabel[] = "USD"; static constexpr char kAssignNewMaterialXMaterialItem[] = "MaterialX Material"; @@ -146,7 +148,7 @@ static constexpr char kAssignNewMaterialXMaterialLabel[] = "MaterialX"; static constexpr char kAssignNewArnoldMaterialItem[] = "Arnold Material"; static constexpr char kAssignNewArnoldMaterialLabel[] = "Arnold"; static constexpr char kAssignNewUsdPreviewSurfaceMaterialItem[] = "UsdPreviewSurface"; -static constexpr char kAssignNewUsdPreviewSurfaceMaterialLabel[] = "Usd Preview Surface"; +static constexpr char kAssignNewUsdPreviewSurfaceMaterialLabel[] = "USD Preview Surface"; static constexpr char kAssignNewAIStandardSurfaceMaterialItem[] = "arnold:standard_surface"; static constexpr char kAssignNewAIStandardSurfaceMaterialLabel[] = "AI Standard Surface"; #endif @@ -236,17 +238,16 @@ const MxShaderMenuEntryVec& getMaterialXSurfaceShaders() // - utility nodes like ND_add_surfaceshader // - basic building blocks like ND_thin_surface // - shaders that exist only as pure definitions like ND_disney_bsdf_2015_surface - static const std::vector vettedSurfaces - = { "ND_standard_surface_surfaceshader", - "ND_gltf_pbr_surfaceshader", - "ND_UsdPreviewSurface_surfaceshader" }; - for (auto&& identifier : vettedSurfaces) { - auto shaderDef = sdrRegistry.GetShaderNodeByIdentifier(TfToken(identifier)); + static const std::vector> vettedSurfaces + = { { "ND_standard_surface_surfaceshader", "Standard Surface" }, + { "ND_gltf_pbr_surfaceshader", "glTF PBR" }, + { "ND_UsdPreviewSurface_surfaceshader", "USD Preview Surface" } }; + for (auto&& info : vettedSurfaces) { + auto shaderDef = sdrRegistry.GetShaderNodeByIdentifier(TfToken(info.first)); if (!shaderDef) { continue; } - const std::string label = UsdMayaUtil::prettifyName(shaderDef->GetFamily().GetString()); - mxSurfaceShaders.emplace_back(label, shaderDef->GetIdentifier().GetString()); + mxSurfaceShaders.emplace_back(info.second, info.first); } initialized = true; } @@ -785,6 +786,29 @@ static const std::vector getConcretePrimTypes(boo } #endif +bool selectionSupportsShading() +{ + if (auto globalSn = Ufe::GlobalSelection::get()) { + for (auto&& selItem : *globalSn) { + if (MAYAUSD_NS_DEF::ufe::BindMaterialUndoableCommand::CompatiblePrim(selItem)) { + return true; + } + } + } + return false; +} + +bool isShadingType(const Ufe::SceneItem::Ptr& target) +{ + if (!target) { + return false; + } + const std::vector kSortedShadingTypes + = { "NodeGraph", "Material", "Scope", "Shader" }; + return std::binary_search( + kSortedShadingTypes.begin(), kSortedShadingTypes.end(), target->nodeType()); +} + } // namespace namespace MAYAUSD_NS_DEF { @@ -823,21 +847,8 @@ Ufe::ContextOps::Items UsdContextOps::getItems(const Ufe::ContextOps::ItemPath& Ufe::ContextOps::Items items; if (itemPath.empty()) { #if PXR_VERSION >= 2108 - if (fItem->prim().IsA()) { + if (fItem->prim().IsA() && selectionSupportsShading()) { items.emplace_back(kBindMaterialToSelectionItem, kBindMaterialToSelectionLabel); - bool enable = false; - if (auto globalSn = Ufe::GlobalSelection::get()) { - for (auto&& selItem : *globalSn) { - if (BindMaterialUndoableCommand::CompatiblePrim(selItem)) { - enable = true; - break; - } - } - } - // I did not see any support for disabling items in Maya Ufe outliner code, but let's - // still mark it as disabled if nothing is selected or if the selection does not contain - // imageable primitives. - items.back().enabled = enable; items.emplace_back(Ufe::ContextItem::kSeparator); } #endif @@ -911,7 +922,7 @@ Ufe::ContextOps::Items UsdContextOps::getItems(const Ufe::ContextOps::ItemPath& if (!fIsAGatewayType) { // Top level item - Bind/unbind existing materials bool materialSeparatorsAdded = false; - if (PXR_NS::UsdShadeMaterialBindingAPI::CanApply(fItem->prim())) { + if (BindMaterialUndoableCommand::CompatiblePrim(fItem).IsValid()) { // Show bind menu if there is at least one bindable material in the stage. // // TODO: Show only materials that are inside of the asset's namespace otherwise @@ -962,13 +973,19 @@ Ufe::ContextOps::Items UsdContextOps::getItems(const Ufe::ContextOps::ItemPath& Ufe::ContextItem::kHasChildren); } } + } #if UFE_PREVIEW_VERSION_NUM >= 4010 + if (BindMaterialUndoableCommand::CompatiblePrim(fItem).IsValid()) { + if (!materialSeparatorsAdded) { + items.emplace_back(Ufe::ContextItem::kSeparator); + materialSeparatorsAdded = true; + } items.emplace_back( kAssignNewMaterialItem, kAssignNewMaterialLabel, Ufe::ContextItem::kHasChildren); -#endif } +#endif if (fItem->prim().HasAPI()) { UsdShadeMaterialBindingAPI bindingAPI(fItem->prim()); // Show unbind menu item if there is a direct binding relationship: @@ -983,6 +1000,16 @@ Ufe::ContextOps::Items UsdContextOps::getItems(const Ufe::ContextOps::ItemPath& UnbindMaterialUndoableCommand::commandName); } } +#if UFE_PREVIEW_VERSION_NUM >= 4010 + if (UsdUndoAddNewMaterialCommand::CompatiblePrim(fItem)) { + if (!materialSeparatorsAdded) { + items.emplace_back(Ufe::ContextItem::kSeparator); + materialSeparatorsAdded = true; + } + items.emplace_back( + kAddNewMaterialItem, kAddNewMaterialLabel, Ufe::ContextItem::kHasChildren); + } +#endif } #endif } else { @@ -1102,7 +1129,9 @@ Ufe::ContextOps::Items UsdContextOps::getItems(const Ufe::ContextOps::ItemPath& } } #if UFE_PREVIEW_VERSION_NUM >= 4010 - } else if (itemPath.size() == 1u && itemPath[0] == kAssignNewMaterialItem) { + } else if ( + itemPath.size() == 1u + && (itemPath[0] == kAssignNewMaterialItem || itemPath[0] == kAddNewMaterialItem)) { items.emplace_back( kAssignNewUsdMaterialItem, kAssignNewUsdMaterialLabel, @@ -1270,6 +1299,9 @@ Ufe::UndoableCommand::Ptr UsdContextOps::doOpCmd(const ItemPath& itemPath) return std::make_shared( UsdUndoAssignNewMaterialCommand::create(sceneItems, itemPath[2])); } + } else if (itemPath.size() == 3u && itemPath[0] == kAddNewMaterialItem) { + return std::make_shared( + UsdUndoAddNewMaterialCommand::create(fItem, itemPath[2])); #endif } #endif diff --git a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp index 825d6717a5..3b2fb722a6 100644 --- a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp +++ b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -34,6 +35,44 @@ PXR_NAMESPACE_USING_DIRECTIVE namespace MAYAUSD_NS_DEF { namespace ufe { +namespace { +// We will not use the value of UsdUtilsGetMaterialsScopeName() for the material scope. +static const std::string kDefaultMaterialScopeName("mtl"); + +bool connectShaderToMaterial( + Ufe::SceneItem::Ptr shaderItem, + UsdPrim materialPrim, + const std::string& nodeId) +{ + auto shaderUsdItem = std::dynamic_pointer_cast(shaderItem); + auto shaderPrim = UsdShadeShader(shaderUsdItem->prim()); + UsdShadeOutput materialOutput; + PXR_NS::SdrRegistry& registry = PXR_NS::SdrRegistry::GetInstance(); + PXR_NS::SdrShaderNodeConstPtr shaderNodeDef + = registry.GetShaderNodeByIdentifier(TfToken(nodeId)); + SdrShaderPropertyConstPtr shaderOutputDef; + if (shaderNodeDef->GetSourceType() == "glslfx") { + materialOutput = UsdShadeMaterial(materialPrim).CreateSurfaceOutput(); + shaderOutputDef = shaderNodeDef->GetShaderOutput(TfToken("surface")); + } else { + if (shaderNodeDef->GetOutputNames().size() != 1) { + TF_RUNTIME_ERROR( + "Cannot resolve which output of shader %s should be connected to surface", + nodeId.c_str()); + return false; + } + materialOutput + = UsdShadeMaterial(materialPrim).CreateSurfaceOutput(shaderNodeDef->GetSourceType()); + shaderOutputDef = shaderNodeDef->GetShaderOutput(shaderNodeDef->GetOutputNames()[0]); + } + UsdShadeOutput shaderOutput = shaderPrim.CreateOutput( + shaderOutputDef->GetName(), shaderOutputDef->GetTypeAsSdfType().first); + UsdShadeConnectableAPI::ConnectToSource(materialOutput, shaderOutput); + return true; +} + +} // namespace + UsdPrim BindMaterialUndoableCommand::CompatiblePrim(const Ufe::SceneItem::Ptr& item) { auto usdItem = std::dynamic_pointer_cast(item); @@ -46,6 +85,9 @@ UsdPrim BindMaterialUndoableCommand::CompatiblePrim(const Ufe::SceneItem::Ptr& i // material or a shader. return {}; } + if (UsdGeomScope(usdPrim) && usdPrim.GetName() == kDefaultMaterialScopeName) { + return {}; + } if (PXR_NS::UsdShadeMaterialBindingAPI::CanApply(usdPrim)) { return usdPrim; } @@ -106,7 +148,7 @@ void BindMaterialUndoableCommand::redo() bindingAPI.Bind(material); } } -const std::string BindMaterialUndoableCommand::commandName("Bind Material"); +const std::string BindMaterialUndoableCommand::commandName("Assign Material"); UnbindMaterialUndoableCommand::UnbindMaterialUndoableCommand(const UsdPrim& prim) : _stage(prim.GetStage()) @@ -154,7 +196,7 @@ void UnbindMaterialUndoableCommand::redo() } } } -const std::string UnbindMaterialUndoableCommand::commandName("Unbind Material"); +const std::string UnbindMaterialUndoableCommand::commandName("Unassign Material"); #if (UFE_PREVIEW_VERSION_NUM >= 4010) UsdUndoAssignNewMaterialCommand::UsdUndoAssignNewMaterialCommand( @@ -221,12 +263,7 @@ Ufe::SceneItem::Ptr UsdUndoAssignNewMaterialCommand::insertedChild() const return {}; } -namespace { -// We will not use the value of UsdUtilsGetMaterialsScopeName() for the material scope. -static const std::string kDefaultMaterialScopeName("mtl"); -} // namespace - -void UsdUndoAssignNewMaterialCommand::execute() +std::string UsdUndoAssignNewMaterialCommand::resolvedMaterialScopeName() { std::string materialsScopeName = kDefaultMaterialScopeName; if (TfGetEnvSetting(USD_FORCE_DEFAULT_MATERIALS_SCOPE_NAME)) { @@ -238,6 +275,12 @@ void UsdUndoAssignNewMaterialCommand::execute() materialsScopeName = mayaUsdDefaultMaterialsScopeName; } } + return materialsScopeName; +} + +void UsdUndoAssignNewMaterialCommand::execute() +{ + std::string materialsScopeName = resolvedMaterialScopeName(); // Materials cannot be shared between stages. So we create a unique material per stage, // which can then be shared between any number of objects within that stage. @@ -339,10 +382,9 @@ void UsdUndoAssignNewMaterialCommand::execute() // // 4. Connect the Shader to the material: // - connectShaderToMaterial( - createShaderCmd->insertedChild(), createMaterialCmd->newPrim()); - if (!_cmds) { - // connect has failed. + if (!connectShaderToMaterial( + createShaderCmd->insertedChild(), createMaterialCmd->newPrim(), _nodeId)) { + markAsFailed(); return; } } @@ -375,43 +417,162 @@ void UsdUndoAssignNewMaterialCommand::redo() auto addMaterialCmd = std::dynamic_pointer_cast(*cmdsIt++); auto addShaderCmd = std::dynamic_pointer_cast(*cmdsIt); - connectShaderToMaterial(addShaderCmd->insertedChild(), addMaterialCmd->newPrim()); + connectShaderToMaterial(addShaderCmd->insertedChild(), addMaterialCmd->newPrim(), _nodeId); } } -void UsdUndoAssignNewMaterialCommand::connectShaderToMaterial( - Ufe::SceneItem::Ptr shaderItem, - UsdPrim materialPrim) +void UsdUndoAssignNewMaterialCommand::markAsFailed() { - auto shaderUsdItem = std::dynamic_pointer_cast(shaderItem); - auto shaderPrim = UsdShadeShader(shaderUsdItem->prim()); - UsdShadeOutput materialOutput; - PXR_NS::SdrRegistry& registry = PXR_NS::SdrRegistry::GetInstance(); + _cmds->undo(); + _cmds.reset(); +} + +UsdUndoAddNewMaterialCommand::UsdUndoAddNewMaterialCommand( + const UsdSceneItem::Ptr& parentItem, + const std::string& nodeId) + : Ufe::InsertChildCommand() + , _parentPath((parentItem && parentItem->prim().IsActive()) ? parentItem->path() : Ufe::Path()) + , _nodeId(nodeId) + , _cmds(std::make_shared()) +{ +} + +UsdUndoAddNewMaterialCommand::~UsdUndoAddNewMaterialCommand() { } + +UsdUndoAddNewMaterialCommand::Ptr +UsdUndoAddNewMaterialCommand::create(const UsdSceneItem::Ptr& parentItem, const std::string& nodeId) +{ + // Changing the hierarchy of invalid items is not allowed. + if (!parentItem || !parentItem->prim().IsActive()) + return nullptr; + + return std::make_shared(parentItem, nodeId); +} + +Ufe::SceneItem::Ptr UsdUndoAddNewMaterialCommand::insertedChild() const +{ + if (_cmds) { + auto cmdsIt = _cmds->cmdsList().begin(); + std::advance(cmdsIt, _createMaterialCmdIdx + 1); + auto addShaderCmd = std::dynamic_pointer_cast(*cmdsIt); + return addShaderCmd->insertedChild(); + } + return {}; +} + +bool UsdUndoAddNewMaterialCommand::CompatiblePrim(const Ufe::SceneItem::Ptr& target) +{ + if (!target) { + return false; + } + + // Must be a scope. + if (target->nodeType() != "Scope") { + return false; + } + + // With the magic name. + if (target->nodeName() == UsdUndoAssignNewMaterialCommand::resolvedMaterialScopeName()) { + return true; + } + + // Or with only materials inside + auto scopeHierarchy = Ufe::Hierarchy::hierarchy(target); + if (scopeHierarchy) { + for (auto&& child : scopeHierarchy->children()) { + if (child->nodeType() != "Material") { + // At least one non material + return false; + } + } + } + + return true; +} + +void UsdUndoAddNewMaterialCommand::execute() +{ + if (_parentPath.empty()) { + markAsFailed(); + return; + } + + // + // Create the Material: + // + PXR_NS::SdrRegistry& registry = PXR_NS::SdrRegistry::GetInstance(); PXR_NS::SdrShaderNodeConstPtr shaderNodeDef = registry.GetShaderNodeByIdentifier(TfToken(_nodeId)); - SdrShaderPropertyConstPtr shaderOutputDef; - if (shaderNodeDef->GetSourceType() == "glslfx") { - materialOutput = UsdShadeMaterial(materialPrim).CreateSurfaceOutput(); - shaderOutputDef = shaderNodeDef->GetShaderOutput(TfToken("surface")); - } else { - if (shaderNodeDef->GetOutputNames().size() != 1) { - TF_RUNTIME_ERROR( - "Cannot resolve which output of shader %s should be connected to surface", - _nodeId.c_str()); - markAsFailed(); - return; - } - materialOutput - = UsdShadeMaterial(materialPrim).CreateSurfaceOutput(shaderNodeDef->GetSourceType()); - shaderOutputDef = shaderNodeDef->GetShaderOutput(shaderNodeDef->GetOutputNames()[0]); + if (!shaderNodeDef) { + TF_RUNTIME_ERROR("Unknown shader identifier: %s", _nodeId.c_str()); + markAsFailed(); + return; + } + if (shaderNodeDef->GetOutputNames().empty()) { + TF_RUNTIME_ERROR("Surface shader %s does not have any outputs", _nodeId.c_str()); + markAsFailed(); + return; + } + auto scopeItem + = std::dynamic_pointer_cast(Ufe::Hierarchy::createItem(_parentPath)); + auto createMaterialCmd = UsdUndoAddNewPrimCommand::create( + scopeItem, shaderNodeDef->GetFamily().GetString(), "Material"); + createMaterialCmd->execute(); + _createMaterialCmdIdx = _cmds->cmdsList().size(); + _cmds->append(createMaterialCmd); + if (!createMaterialCmd->newPrim()) { + // The _createMaterialCmd will have emitted errors. + markAsFailed(); + return; + } + + // + // Create the Shader: + // + auto materialItem = std::dynamic_pointer_cast( + Ufe::Hierarchy::createItem(createMaterialCmd->newUfePath())); + auto createShaderCmd = UsdUndoCreateFromNodeDefCommand::create( + shaderNodeDef, materialItem, shaderNodeDef->GetFamily().GetString()); + createShaderCmd->execute(); + _cmds->append(createShaderCmd); + if (!createShaderCmd->insertedChild()) { + // The _createShaderCmd will have emitted errors. + markAsFailed(); + return; + } + + // + // Connect the Shader to the material: + // + if (!connectShaderToMaterial( + createShaderCmd->insertedChild(), createMaterialCmd->newPrim(), _nodeId)) { + markAsFailed(); + return; } - UsdShadeOutput shaderOutput = shaderPrim.CreateOutput( - shaderOutputDef->GetName(), shaderOutputDef->GetTypeAsSdfType().first); - UsdShadeConnectableAPI::ConnectToSource(materialOutput, shaderOutput); - return; } -void UsdUndoAssignNewMaterialCommand::markAsFailed() +void UsdUndoAddNewMaterialCommand::undo() +{ + if (_cmds) { + _cmds->undo(); + } +} + +void UsdUndoAddNewMaterialCommand::redo() +{ + if (_cmds) { + _cmds->redo(); + + auto cmdsIt = _cmds->cmdsList().begin(); + std::advance(cmdsIt, _createMaterialCmdIdx); + auto addMaterialCmd + = std::dynamic_pointer_cast(*cmdsIt++); + auto addShaderCmd = std::dynamic_pointer_cast(*cmdsIt); + connectShaderToMaterial(addShaderCmd->insertedChild(), addMaterialCmd->newPrim(), _nodeId); + } +} + +void UsdUndoAddNewMaterialCommand::markAsFailed() { _cmds->undo(); _cmds.reset(); diff --git a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.h b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.h index 54d5c783b1..10376b8eab 100644 --- a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.h +++ b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.h @@ -122,8 +122,10 @@ class MAYAUSD_CORE_PUBLIC UsdUndoAssignNewMaterialCommand : public Ufe::InsertCh void undo() override; void redo() override; + // Returns the name of the material scope in this Maya session. + static std::string resolvedMaterialScopeName(); + private: - void connectShaderToMaterial(Ufe::SceneItem::Ptr shaderItem, PXR_NS::UsdPrim materialPrim); void markAsFailed(); std::map> _stagesAndPaths; @@ -133,6 +135,48 @@ class MAYAUSD_CORE_PUBLIC UsdUndoAssignNewMaterialCommand : public Ufe::InsertCh std::shared_ptr _cmds; }; // UsdUndoAssignNewMaterialCommand + +//! \brief UsdUndoAddNewMaterialCommand +class MAYAUSD_CORE_PUBLIC UsdUndoAddNewMaterialCommand : public Ufe::InsertChildCommand +{ +public: + typedef std::shared_ptr Ptr; + + UsdUndoAddNewMaterialCommand( + const UsdSceneItem::Ptr& parentItem, + const std::string& sdrShaderIdentifier); + ~UsdUndoAddNewMaterialCommand() override; + + // Delete the copy/move constructors assignment operators. + UsdUndoAddNewMaterialCommand(const UsdUndoAddNewMaterialCommand&) = delete; + UsdUndoAddNewMaterialCommand& operator=(const UsdUndoAddNewMaterialCommand&) = delete; + UsdUndoAddNewMaterialCommand(UsdUndoAddNewMaterialCommand&&) = delete; + UsdUndoAddNewMaterialCommand& operator=(UsdUndoAddNewMaterialCommand&&) = delete; + + //! Create a UsdUndoAddNewMaterialCommand that creates a new material based on + //! \p sdrShaderIdentifier and adds it as child of \p parentItem + static UsdUndoAddNewMaterialCommand::Ptr + create(const UsdSceneItem::Ptr& parentItem, const std::string& sdrShaderIdentifier); + + Ufe::SceneItem::Ptr insertedChild() const override; + + void execute() override; + void undo() override; + void redo() override; + + // Can we add a material to this item. + static bool CompatiblePrim(const Ufe::SceneItem::Ptr& target); + +private: + void markAsFailed(); + + Ufe::Path _parentPath; + const std::string _nodeId; + + size_t _createMaterialCmdIdx = -1; + std::shared_ptr _cmds; + +}; // UsdUndoAddNewMaterialCommand #endif } // namespace ufe diff --git a/test/lib/ufe/testContextOps.py b/test/lib/ufe/testContextOps.py index f4890f6121..db31b259e5 100644 --- a/test/lib/ufe/testContextOps.py +++ b/test/lib/ufe/testContextOps.py @@ -555,7 +555,7 @@ def testMaterialBinding(self): self.assertFalse(capsulePrim.HasAPI(UsdShade.MaterialBindingAPI)) contextOps = ufe.ContextOps.contextOps(capsuleItem) - cmd = contextOps.doOpCmd(['Bind Material', '/Material1']) + cmd = contextOps.doOpCmd(['Assign Material', '/Material1']) self.assertTrue(cmd) ufeCmd.execute(cmd) self.assertTrue(capsulePrim.HasAPI(UsdShade.MaterialBindingAPI)) @@ -568,7 +568,7 @@ def testMaterialBinding(self): self.assertTrue(capsulePrim.HasAPI(UsdShade.MaterialBindingAPI)) self.assertEqual(capsuleBindAPI.GetDirectBinding().GetMaterialPath(), Sdf.Path("/Material1")) - cmd = contextOps.doOpCmd(['Unbind Material']) + cmd = contextOps.doOpCmd(['Unassign Material']) self.assertTrue(cmd) ufeCmd.execute(cmd) @@ -840,6 +840,77 @@ def checkItem(self, item, type, path): ufeCmd.execute(cmdSS) checkMaterial(self, rootHier, 5, 1, 1, 0, "standard_surface", "mtlx", "out", "/test_scope") + @unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '4020', 'Test only available in UFE preview version 0.4.20 and greater') + @unittest.skipUnless(Usd.GetVersion() >= (0, 21, 8), 'Requires CanApplySchema from USD') + def testAddMaterialToScope(self): + """This test adds a new material to the material scope.""" + cmds.file(new=True, force=True) + + # Create a proxy shape with empty stage to start with. + import mayaUsd_createStageWithNewLayer + proxyShape = mayaUsd_createStageWithNewLayer.createStageWithNewLayer() + + # Create a ContextOps interface for the proxy shape. + proxyPathSegment = mayaUtils.createUfePathSegment(proxyShape) + proxyShapePath = ufe.Path([proxyPathSegment]) + proxyShapeItem = ufe.Hierarchy.createItem(proxyShapePath) + contextOps = ufe.ContextOps.contextOps(proxyShapeItem) + + # Create multiple objects to test with. + cmd = contextOps.doOpCmd(['Add New Prim', 'Capsule']) + ufeCmd.execute(cmd) + + rootHier = ufe.Hierarchy.hierarchy(proxyShapeItem) + self.assertTrue(rootHier.hasChildren()) + self.assertEqual(len(rootHier.children()), 1) + + capsuleItem = rootHier.children()[0] + + capsulePrim = usdUtils.getPrimFromSceneItem(capsuleItem) + self.assertFalse(capsulePrim.HasAPI(UsdShade.MaterialBindingAPI)) + + contextOps = ufe.ContextOps.contextOps(capsuleItem) + + # Create a new material and apply it to our cube, sphere and capsule objects. + cmdPS = contextOps.doOpCmd(['Assign New Material', 'USD', 'UsdPreviewSurface']) + self.assertIsNotNone(cmdPS) + ufeCmd.execute(cmdPS) + + scopeItem = rootHier.children()[-1] + scopeHier = ufe.Hierarchy.hierarchy(scopeItem) + self.assertTrue(scopeHier.hasChildren()) + self.assertEqual(len(scopeHier.children()), 1) + + scopeOps = ufe.ContextOps.contextOps(scopeItem) + cmdAddSS = scopeOps.doOpCmd(['Add New Material', 'MaterialX', 'ND_standard_surface_surfaceshader']) + ufeCmd.execute(cmdAddSS) + + # Should now be two materials in the scope. + self.assertEqual(len(scopeHier.children()), 2) + + cmds.undo() + + self.assertEqual(len(scopeHier.children()), 1) + + cmds.redo() + + self.assertEqual(len(scopeHier.children()), 2) + + newMatItem = scopeHier.children()[-1] + + connectionHandler = ufe.RunTimeMgr.instance().connectionHandler(newMatItem.runTimeId()) + self.assertIsNotNone(connectionHandler) + connections = connectionHandler.sourceConnections(newMatItem) + self.assertIsNotNone(connectionHandler) + conns = connections.allConnections() + self.assertEqual(len(conns), 1) + + mxConn = conns[0] + self.assertEqual(ufe.PathString.string(mxConn.src.path), "|stage1|stageShape1,/mtl/standard_surface1/standard_surface1") + self.assertEqual(mxConn.src.name, "outputs:out") + self.assertEqual(ufe.PathString.string(mxConn.dst.path), "|stage1|stageShape1,/mtl/standard_surface1") + self.assertEqual(mxConn.dst.name, "outputs:mtlx:surface") + @unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '4010', 'Test only available in UFE preview version 0.4.10 and greater') @unittest.skipUnless(Usd.GetVersion() >= (0, 21, 8), 'Requires CanApplySchema from USD') def testMaterialBindingWithNodeDefHandler(self): @@ -899,7 +970,7 @@ def testMaterialBindingWithNodeDefHandler(self): self.assertFalse(capsulePrim.HasAPI(UsdShade.MaterialBindingAPI)) contextOps = ufe.ContextOps.contextOps(capsuleItem) - cmd = contextOps.doOpCmd(['Bind Material', '/Material1']) + cmd = contextOps.doOpCmd(['Assign Material', '/Material1']) self.assertTrue(cmd) ufeCmd.execute(cmd) self.assertTrue(capsulePrim.HasAPI(UsdShade.MaterialBindingAPI)) @@ -912,7 +983,7 @@ def testMaterialBindingWithNodeDefHandler(self): self.assertTrue(capsulePrim.HasAPI(UsdShade.MaterialBindingAPI)) self.assertEqual(capsuleBindAPI.GetDirectBinding().GetMaterialPath(), Sdf.Path("/Material1")) - cmd = contextOps.doOpCmd(['Unbind Material']) + cmd = contextOps.doOpCmd(['Unassign Material']) self.assertTrue(cmd) ufeCmd.execute(cmd) From 41884c3e4af4d62bd2d573c7d7f2cab6487145e1 Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Mon, 12 Dec 2022 16:32:21 -0500 Subject: [PATCH 2/4] Fix Unix builds --- lib/mayaUsd/ufe/UsdContextOps.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdContextOps.cpp b/lib/mayaUsd/ufe/UsdContextOps.cpp index 0d39222493..3f05601dfc 100644 --- a/lib/mayaUsd/ufe/UsdContextOps.cpp +++ b/lib/mayaUsd/ufe/UsdContextOps.cpp @@ -798,17 +798,6 @@ bool selectionSupportsShading() return false; } -bool isShadingType(const Ufe::SceneItem::Ptr& target) -{ - if (!target) { - return false; - } - const std::vector kSortedShadingTypes - = { "NodeGraph", "Material", "Scope", "Shader" }; - return std::binary_search( - kSortedShadingTypes.begin(), kSortedShadingTypes.end(), target->nodeType()); -} - } // namespace namespace MAYAUSD_NS_DEF { From a9d383399b6dd53e7f124cbaff692d6e0775a88b Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Mon, 12 Dec 2022 18:05:32 -0500 Subject: [PATCH 3/4] Fix build for older UFE versions. --- lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp index 3b2fb722a6..51fbf2f53c 100644 --- a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp +++ b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp @@ -39,6 +39,7 @@ namespace { // We will not use the value of UsdUtilsGetMaterialsScopeName() for the material scope. static const std::string kDefaultMaterialScopeName("mtl"); +#if (UFE_PREVIEW_VERSION_NUM >= 4010) bool connectShaderToMaterial( Ufe::SceneItem::Ptr shaderItem, UsdPrim materialPrim, @@ -70,7 +71,7 @@ bool connectShaderToMaterial( UsdShadeConnectableAPI::ConnectToSource(materialOutput, shaderOutput); return true; } - +#endif } // namespace UsdPrim BindMaterialUndoableCommand::CompatiblePrim(const Ufe::SceneItem::Ptr& item) From 8d5bd894182b5bb13f6f1406cfd28c471e10b5dd Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Tue, 13 Dec 2022 15:01:49 -0500 Subject: [PATCH 4/4] Responding to review comments. --- lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp | 141 ++++++++++++++------ lib/mayaUsd/ufe/UsdUndoMaterialCommands.h | 4 +- 2 files changed, 101 insertions(+), 44 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp index 51fbf2f53c..25dbda87c8 100644 --- a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp +++ b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp @@ -36,7 +36,9 @@ namespace MAYAUSD_NS_DEF { namespace ufe { namespace { -// We will not use the value of UsdUtilsGetMaterialsScopeName() for the material scope. + +// Pixar uses "Looks" to name the materials scope. The USD asset workgroup recommendations is to +// use "mtl" instead. So we will go with the WG recommendation when creating new material scopes. static const std::string kDefaultMaterialScopeName("mtl"); #if (UFE_PREVIEW_VERSION_NUM >= 4010) @@ -45,12 +47,18 @@ bool connectShaderToMaterial( UsdPrim materialPrim, const std::string& nodeId) { - auto shaderUsdItem = std::dynamic_pointer_cast(shaderItem); - auto shaderPrim = UsdShadeShader(shaderUsdItem->prim()); - UsdShadeOutput materialOutput; - PXR_NS::SdrRegistry& registry = PXR_NS::SdrRegistry::GetInstance(); + auto shaderUsdItem = std::dynamic_pointer_cast(shaderItem); + if (!shaderUsdItem) { + return false; + } + auto shaderPrim = UsdShadeShader(shaderUsdItem->prim()); + UsdShadeOutput materialOutput; + PXR_NS::SdrRegistry& registry = PXR_NS::SdrRegistry::GetInstance(); PXR_NS::SdrShaderNodeConstPtr shaderNodeDef = registry.GetShaderNodeByIdentifier(TfToken(nodeId)); + if (!shaderNodeDef) { + return false; + } SdrShaderPropertyConstPtr shaderOutputDef; if (shaderNodeDef->GetSourceType() == "glslfx") { materialOutput = UsdShadeMaterial(materialPrim).CreateSurfaceOutput(); @@ -66,8 +74,14 @@ bool connectShaderToMaterial( = UsdShadeMaterial(materialPrim).CreateSurfaceOutput(shaderNodeDef->GetSourceType()); shaderOutputDef = shaderNodeDef->GetShaderOutput(shaderNodeDef->GetOutputNames()[0]); } + if (!shaderOutputDef) { + return false; + } UsdShadeOutput shaderOutput = shaderPrim.CreateOutput( shaderOutputDef->GetName(), shaderOutputDef->GetTypeAsSdfType().first); + if (!shaderOutput) { + return false; + } UsdShadeConnectableAPI::ConnectToSource(materialOutput, shaderOutput); return true; } @@ -255,6 +269,12 @@ UsdUndoAssignNewMaterialCommand::Ptr UsdUndoAssignNewMaterialCommand::create( Ufe::SceneItem::Ptr UsdUndoAssignNewMaterialCommand::insertedChild() const { + // This is broken. Since PR 2641 we now loop over the selection to handle multiple stages. + // This command returns a single inserted child, while this new implementation can now create + // multiple shaders. This will have to be fixed at a higher level. + // There is still a shader creation command directly after the command at _createMaterialCmdIdx, + // but it will be the last created shader. Still better than nothing, and works correctly in + // the most common workflow where selection covers a single stage. if (_cmds) { auto cmdsIt = _cmds->cmdsList().begin(); std::advance(cmdsIt, _createMaterialCmdIdx + 1); @@ -293,6 +313,10 @@ void UsdUndoAssignNewMaterialCommand::execute() // auto parentItem = std::dynamic_pointer_cast(Ufe::Hierarchy::createItem(parentPath)); + if (!parentItem) { + markAsFailed(); + return; + } Ufe::Path scopePath; PXR_NS::UsdStageWeakPtr stage = getStage(parentItem->path()); if (stage) { @@ -315,6 +339,10 @@ void UsdUndoAssignNewMaterialCommand::execute() MayaUsd::ufe::stagePath(stage), stage->GetPseudoRoot()), materialsScopeName, "Scope"); + if (!createScopeCmd) { + markAsFailed(); + return; + } createScopeCmd->execute(); _cmds->append(createScopeCmd); scopePath = createScopeCmd->newUfePath(); @@ -322,6 +350,10 @@ void UsdUndoAssignNewMaterialCommand::execute() auto itemOps = Ufe::SceneItemOps::sceneItemOps(Ufe::Hierarchy::createItem(scopePath)); auto rename = itemOps->renameItemCmd(Ufe::PathComponent(materialsScopeName)); + if (!rename.undoableCommand) { + markAsFailed(); + return; + } _cmds->append(rename.undoableCommand); scopePath = rename.item->path(); } @@ -356,6 +388,10 @@ void UsdUndoAssignNewMaterialCommand::execute() Ufe::Hierarchy::createItem(scopePath)); auto createMaterialCmd = UsdUndoAddNewPrimCommand::create( scopeItem, shaderNodeDef->GetFamily().GetString(), "Material"); + if (!createMaterialCmd) { + markAsFailed(); + return; + } createMaterialCmd->execute(); _createMaterialCmdIdx = _cmds->cmdsList().size(); _cmds->append(createMaterialCmd); @@ -372,6 +408,10 @@ void UsdUndoAssignNewMaterialCommand::execute() Ufe::Hierarchy::createItem(createMaterialCmd->newUfePath())); auto createShaderCmd = UsdUndoCreateFromNodeDefCommand::create( shaderNodeDef, materialItem, shaderNodeDef->GetFamily().GetString()); + if (!createShaderCmd) { + markAsFailed(); + return; + } createShaderCmd->execute(); _cmds->append(createShaderCmd); if (!createShaderCmd->insertedChild()) { @@ -395,6 +435,10 @@ void UsdUndoAssignNewMaterialCommand::execute() // auto bindCmd = std::make_shared( parentItem->prim(), materialItem->prim().GetPath()); + if (!bindCmd) { + markAsFailed(); + return; + } bindCmd->execute(); _cmds->append(bindCmd); } @@ -414,11 +458,22 @@ void UsdUndoAssignNewMaterialCommand::redo() _cmds->redo(); auto cmdsIt = _cmds->cmdsList().begin(); - std::advance(cmdsIt, _createMaterialCmdIdx); - auto addMaterialCmd - = std::dynamic_pointer_cast(*cmdsIt++); - auto addShaderCmd = std::dynamic_pointer_cast(*cmdsIt); - connectShaderToMaterial(addShaderCmd->insertedChild(), addMaterialCmd->newPrim(), _nodeId); + while (cmdsIt != _cmds->cmdsList().end()) { + // Find out all Material creation followed by a shader creation and reconnect the + // shader to the material. Don't assume any ordering. + auto addMaterialCmd + = std::dynamic_pointer_cast(*cmdsIt++); + if (addMaterialCmd && addMaterialCmd->newPrim() + && UsdShadeMaterial(addMaterialCmd->newPrim()) + && cmdsIt != _cmds->cmdsList().end()) { + auto addShaderCmd + = std::dynamic_pointer_cast(*cmdsIt++); + if (addShaderCmd) { + connectShaderToMaterial( + addShaderCmd->insertedChild(), addMaterialCmd->newPrim(), _nodeId); + } + } + } } } @@ -434,7 +489,6 @@ UsdUndoAddNewMaterialCommand::UsdUndoAddNewMaterialCommand( : Ufe::InsertChildCommand() , _parentPath((parentItem && parentItem->prim().IsActive()) ? parentItem->path() : Ufe::Path()) , _nodeId(nodeId) - , _cmds(std::make_shared()) { } @@ -452,11 +506,8 @@ UsdUndoAddNewMaterialCommand::create(const UsdSceneItem::Ptr& parentItem, const Ufe::SceneItem::Ptr UsdUndoAddNewMaterialCommand::insertedChild() const { - if (_cmds) { - auto cmdsIt = _cmds->cmdsList().begin(); - std::advance(cmdsIt, _createMaterialCmdIdx + 1); - auto addShaderCmd = std::dynamic_pointer_cast(*cmdsIt); - return addShaderCmd->insertedChild(); + if (_createShaderCmd) { + return _createShaderCmd->insertedChild(); } return {}; } @@ -494,7 +545,6 @@ bool UsdUndoAddNewMaterialCommand::CompatiblePrim(const Ufe::SceneItem::Ptr& tar void UsdUndoAddNewMaterialCommand::execute() { if (_parentPath.empty()) { - markAsFailed(); return; } @@ -506,22 +556,22 @@ void UsdUndoAddNewMaterialCommand::execute() = registry.GetShaderNodeByIdentifier(TfToken(_nodeId)); if (!shaderNodeDef) { TF_RUNTIME_ERROR("Unknown shader identifier: %s", _nodeId.c_str()); - markAsFailed(); return; } if (shaderNodeDef->GetOutputNames().empty()) { TF_RUNTIME_ERROR("Surface shader %s does not have any outputs", _nodeId.c_str()); - markAsFailed(); return; } + auto scopeItem = std::dynamic_pointer_cast(Ufe::Hierarchy::createItem(_parentPath)); - auto createMaterialCmd = UsdUndoAddNewPrimCommand::create( + _createMaterialCmd = UsdUndoAddNewPrimCommand::create( scopeItem, shaderNodeDef->GetFamily().GetString(), "Material"); - createMaterialCmd->execute(); - _createMaterialCmdIdx = _cmds->cmdsList().size(); - _cmds->append(createMaterialCmd); - if (!createMaterialCmd->newPrim()) { + if (!_createMaterialCmd) { + return; + } + _createMaterialCmd->execute(); + if (!_createMaterialCmd->newPrim()) { // The _createMaterialCmd will have emitted errors. markAsFailed(); return; @@ -531,12 +581,15 @@ void UsdUndoAddNewMaterialCommand::execute() // Create the Shader: // auto materialItem = std::dynamic_pointer_cast( - Ufe::Hierarchy::createItem(createMaterialCmd->newUfePath())); - auto createShaderCmd = UsdUndoCreateFromNodeDefCommand::create( + Ufe::Hierarchy::createItem(_createMaterialCmd->newUfePath())); + _createShaderCmd = UsdUndoCreateFromNodeDefCommand::create( shaderNodeDef, materialItem, shaderNodeDef->GetFamily().GetString()); - createShaderCmd->execute(); - _cmds->append(createShaderCmd); - if (!createShaderCmd->insertedChild()) { + if (!_createShaderCmd) { + markAsFailed(); + return; + } + _createShaderCmd->execute(); + if (!_createShaderCmd->insertedChild()) { // The _createShaderCmd will have emitted errors. markAsFailed(); return; @@ -546,7 +599,7 @@ void UsdUndoAddNewMaterialCommand::execute() // Connect the Shader to the material: // if (!connectShaderToMaterial( - createShaderCmd->insertedChild(), createMaterialCmd->newPrim(), _nodeId)) { + _createShaderCmd->insertedChild(), _createMaterialCmd->newPrim(), _nodeId)) { markAsFailed(); return; } @@ -554,29 +607,33 @@ void UsdUndoAddNewMaterialCommand::execute() void UsdUndoAddNewMaterialCommand::undo() { - if (_cmds) { - _cmds->undo(); + if (_createMaterialCmd) { + _createShaderCmd->undo(); + _createMaterialCmd->undo(); } } void UsdUndoAddNewMaterialCommand::redo() { - if (_cmds) { - _cmds->redo(); + if (_createMaterialCmd) { + _createMaterialCmd->redo(); + _createShaderCmd->redo(); - auto cmdsIt = _cmds->cmdsList().begin(); - std::advance(cmdsIt, _createMaterialCmdIdx); - auto addMaterialCmd - = std::dynamic_pointer_cast(*cmdsIt++); - auto addShaderCmd = std::dynamic_pointer_cast(*cmdsIt); - connectShaderToMaterial(addShaderCmd->insertedChild(), addMaterialCmd->newPrim(), _nodeId); + connectShaderToMaterial( + _createShaderCmd->insertedChild(), _createMaterialCmd->newPrim(), _nodeId); } } void UsdUndoAddNewMaterialCommand::markAsFailed() { - _cmds->undo(); - _cmds.reset(); + if (_createShaderCmd) { + _createShaderCmd->undo(); + _createShaderCmd.reset(); + } + if (_createMaterialCmd) { + _createMaterialCmd->undo(); + _createMaterialCmd.reset(); + } } #endif diff --git a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.h b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.h index 10376b8eab..54b86a0e67 100644 --- a/lib/mayaUsd/ufe/UsdUndoMaterialCommands.h +++ b/lib/mayaUsd/ufe/UsdUndoMaterialCommands.h @@ -173,8 +173,8 @@ class MAYAUSD_CORE_PUBLIC UsdUndoAddNewMaterialCommand : public Ufe::InsertChild Ufe::Path _parentPath; const std::string _nodeId; - size_t _createMaterialCmdIdx = -1; - std::shared_ptr _cmds; + UsdUndoAddNewPrimCommand::Ptr _createMaterialCmd; + UsdUndoCreateFromNodeDefCommand::Ptr _createShaderCmd; }; // UsdUndoAddNewMaterialCommand #endif