Assign to component tags #2963

JGamache-autodesk · 2023-03-22T20:42:35Z

Allows assigning materials to component tags
Will set family to "materialBind" if required
That is part 1 of 2. The second part requires being able to roundtrip these assignments thru import/export without gaining or losing any component tags.

- Automatically adjust the family name when assigning. - Use a modern undo/redo framework to fix bugs.

JGamache-autodesk · 2023-03-22T20:45:48Z

lib/mayaUsd/ufe/UsdContextOps.cpp

@@ -735,7 +735,7 @@ static const std::vector<MayaUsd::ufe::SchemaTypeGroup> getConcretePrimTypes(boo
 bool sceneItemSupportsShading(const Ufe::SceneItem::Ptr& sceneItem)
 {
 #if PXR_VERSION >= 2108
-    if (MayaUsd::ufe::BindMaterialUndoableCommand::CompatiblePrim(sceneItem).IsValid()) {
+    if (MayaUsd::ufe::BindMaterialUndoableCommand::CompatiblePrim(sceneItem)) {


Some signatures were changed. This one now only returns a boolean.

JGamache-autodesk · 2023-03-22T20:46:07Z

lib/mayaUsd/ufe/UsdContextOps.cpp

@@ -1239,24 +1239,23 @@ Ufe::UndoableCommand::Ptr UsdContextOps::doOpCmd(const ItemPath& itemPath)
 #endif
 #if PXR_VERSION >= 2108
    else if (itemPath[0] == BindMaterialUndoableCommand::commandName) {
-        return std::make_shared<BindMaterialUndoableCommand>(fItem->prim(), SdfPath(itemPath[1]));
+        return std::make_shared<BindMaterialUndoableCommand>(fItem->path(), SdfPath(itemPath[1]));


And the constructor now takes a Ufe path.

JGamache-autodesk · 2023-03-22T20:49:23Z

lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp

 }

 BindMaterialUndoableCommand::BindMaterialUndoableCommand(
-    const UsdPrim& prim,
+    Ufe::Path      primPath,


I was about to say that keeping a Ufe::Path was better than keeping a Stage/SdfPath for undo/redo, but then noticed that since I modernized to use a UsdUndoBlock, this is not relevant anymore.

JGamache-autodesk · 2023-03-22T20:50:27Z

lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp

-        }
-        bindingAPI.Bind(material);
+    if (auto subset = UsdGeomSubset(prim)) {
+        subset.GetFamilyNameAttr().Set(UsdShadeTokens->materialBind);


That was the only required change to make assignment work and have the material display in the viewport.

OK, but as I asked, IDk what are the implication of changing the family name on an existing subset.

pierrebai-adsk · 2023-03-22T21:27:45Z

test/lib/ufe/testContextOps.py

@@ -312,7 +313,6 @@ def testAddNewPrim(self):
        cmds.file(new=True, force=True)

        # Create a proxy shape with empty stage to start with.
-        import mayaUsd_createStageWithNewLayer
        proxyShape = mayaUsd_createStageWithNewLayer.createStageWithNewLayer()


TY, it annoyed me too in past ;)

pierrebai-adsk · 2023-03-22T21:31:05Z

test/lib/ufe/testContextOps.py

+        ufeCmd.execute(cmd)
+
+        self.assertEqual(topSubset.GetFamilyNameAttr().Get(), "materialBind")
+        self.assertTrue(topSubset.GetPrim().HasAPI(UsdShade.MaterialBindingAPI))


I guess it was discussed internally, but i'm not sure what is better: change the name of the set or create a new set? The family name, according to USD, is used to group tags that are related, so changing it changes the group-of-subset composed of with members with the same family name.

In this case we're the one generating the componentTag, but it feels bad to now have 5 side with componentTag family name and one with the material binding one.

The end result of the discussions is that we try to keep the number of subsets to a minimum (hence the in-place family renaming), and a subsequent PR will also make sure that the number stays as stable as possible across import/export.

Maya: create poly sphere, select half the faces and apply material (no component tags on the mesh gets created)

Export to USD: creates a geom subset with same name as material and some extra roundtripping info

Import to Maya: the poly mesh does not get a new component tag but the face material assignment is preserved

Export again to USD: number of geom subsets stays stable.

or

Maya: create poly cube (automatically gets component tags)

Export to USD: tags exported as subsets. Extra info added to specify source.

In MayaUSD: assign material to one geom subset (this PR). Rename family and add material relationship.

Import from USD: We create both a mesh component tag (since we remember the origin) and a face-assigned material (since we want to see material in viewport). Need to add info to mark both as coming from the same geom subset.

Export again to USD: number of geom subsets stays stable because we recognize that the material subset is the same as the mesh subset (unless editing in Maya caused them to diverge).

This means the geom subset import/export will no longer be hardcoded to only support "componentTag" family name.

All families are supported. Default if family is unknown is still "componentTag"

There will be a way to assign a family name using USD dynamic tags on the Mesh (TBD). The import/export code will use that.

pierrebai-adsk · 2023-03-22T21:35:10Z

lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp

 {
-    if (!_stage || _primPath.IsEmpty() || _materialPath.IsEmpty()) {
+    if (_primPath.empty() || _materialPath.IsEmpty()) {
        return;
    }


IDk if it warrants a warning or some other user feedback when the arguments are not valid?

If the error handling scheme is still CTOR throws if command cannot execute then I agree this test should be moved to the constructor and throw accordingly. The execute() function should assume these are valid. Will fix that this afternoon.

pierrebai-adsk · 2023-03-22T21:38:34Z

lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp

+
+    auto prim = ufePathToPrim(_primPath);
+    if (!prim.IsValid()) {
+        return;


Same comment about all these early returns vs giving user feedback.

pierrebai-adsk

I would only want to know the rationale behind the decision to modify family names before approving.

stefanminning-autodesk

Nice improvements, LGTM.

JGamache-autodesk · 2023-03-23T13:17:10Z

I would only want to know the rationale behind the decision to modify family names before approving.

That is required by USD:

/// \note Material bindings authored on GeomSubsets are honored by renderers
/// only if their familyName is UsdShadeTokens->materialBind. This
/// allows robust interchange of subset bindings between multiple DCC apps.

There is a separate story logged as MAYA-128503 to reconcile the two known Maya geom subsets sources on import/export/rountrip.

pierrebai-adsk · 2023-03-23T13:38:00Z

My question was not about the name itself, but the decision to modify the subset family name instead of creating a new subset with a different family name.

JGamache-autodesk · 2023-03-23T21:54:10Z

lib/mayaUsd/ufe/UsdUndoMaterialCommands.cpp

+            if (!BindMaterialUndoableCommand::CompatiblePrim(parentItem)) {
+                const std::string error = TfStringPrintf(
+                    "Assign new material: Skipping incompatible prim [%s] found in selection.",
+                    Ufe::PathString::string(parentItem->path()).c_str());


Found in the unit tests where the first new material ended up in the selection list for the second assign new material call.

JGamache-autodesk added 3 commits March 21, 2023 09:12

Modernize assign/unassign commands and handle component tags

d0fcd83

- Automatically adjust the family name when assigning. - Use a modern undo/redo framework to fix bugs.

Merge branch 'dev' into gamaj/assign_to_component_tags

292dae3

Added unit test.

fb516d2

JGamache-autodesk requested review from pierrebai-adsk and stefanminning-autodesk March 22, 2023 20:44

JGamache-autodesk assigned ecp-maya-devops-adsk Mar 22, 2023

JGamache-autodesk added the ufe Related to UFE component in Maya label Mar 22, 2023

JGamache-autodesk commented Mar 22, 2023

View reviewed changes

pierrebai-adsk reviewed Mar 22, 2023

View reviewed changes

stefanminning-autodesk previously approved these changes Mar 23, 2023

View reviewed changes

pierrebai-adsk previously approved these changes Mar 23, 2023

View reviewed changes

Added validation.

2e2d9bb

JGamache-autodesk requested review from pierrebai-adsk and stefanminning-autodesk March 23, 2023 21:49

JGamache-autodesk dismissed stale reviews from pierrebai-adsk and stefanminning-autodesk via 2e2d9bb March 23, 2023 21:51

JGamache-autodesk commented Mar 23, 2023

View reviewed changes

JGamache-autodesk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Mar 23, 2023

stefanminning-autodesk approved these changes Mar 24, 2023

View reviewed changes

pierrebai-adsk approved these changes Mar 24, 2023

View reviewed changes

JGamache-autodesk added the ready-for-merge Development process is finished, PR is ready for merge label Mar 24, 2023

seando-adsk added ufe-usd Related to UFE-USD plugin in Maya-Usd and removed ufe Related to UFE component in Maya labels Mar 24, 2023

seando-adsk merged commit 5863f62 into dev Mar 24, 2023

seando-adsk deleted the gamaj/assign_to_component_tags branch March 24, 2023 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assign to component tags #2963

Assign to component tags #2963

JGamache-autodesk commented Mar 22, 2023 •

edited

Loading

JGamache-autodesk Mar 22, 2023

JGamache-autodesk Mar 22, 2023

JGamache-autodesk Mar 22, 2023

JGamache-autodesk Mar 22, 2023

pierrebai-adsk Mar 22, 2023

pierrebai-adsk Mar 22, 2023

pierrebai-adsk Mar 22, 2023

JGamache-autodesk Mar 23, 2023

pierrebai-adsk Mar 22, 2023

JGamache-autodesk Mar 23, 2023

pierrebai-adsk Mar 22, 2023

pierrebai-adsk left a comment

stefanminning-autodesk left a comment

JGamache-autodesk commented Mar 23, 2023

pierrebai-adsk commented Mar 23, 2023

JGamache-autodesk Mar 23, 2023

Assign to component tags #2963

Assign to component tags #2963

Conversation

JGamache-autodesk commented Mar 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierrebai-adsk left a comment

Choose a reason for hiding this comment

stefanminning-autodesk left a comment

Choose a reason for hiding this comment

JGamache-autodesk commented Mar 23, 2023

pierrebai-adsk commented Mar 23, 2023

Choose a reason for hiding this comment

JGamache-autodesk commented Mar 22, 2023 •

edited

Loading