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

EMSUSD-623 edit as Maya multiple variants #3350

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

pierrebai-adsk
Copy link
Collaborator

This change adds support to edit-as-Maya a prim that is under a variant, with each variation having a different Maya reference. That is, a single prim can now be edited-as-Maya multiple times in parallel. This required some changes on how those edited prim are tracked.

Edit-as-Maya authors data in the session layer related to the edited prim. Previously, when edited-as-Maya were orphaned or ceased to be, the order in which they were processed was random. This caused two prims with the same name but in different variants to potentially clobber each other's data. Now we process new orphans first, then new non-orphans second. This allows new non-orphans to write their data without it being wiped out by an orphan.

Additional variant-related helper functions:

  • Add the getEditTargetForVariants function helper function to create the USD edit target targeting all the variants that affect a given prim.
  • Ensure getEditTargetForVariants works in more complex scenarios spanning payloads and references.
  • Add the getVariantPath helper function to create a USD path containing a variant selection.

Additional pull information helper functions:

  • Warn when we fail to remove pull information.

Change to the orphan manager:

  • Support having multiple entries for a given UFE path.
  • To differentiate between entries, the corresponding root Maya DAG path must be given in all functions to figure out which version of the prim we are dealing with.
  • Fortunately, all callers always have on hand the DAG path of the root Maya node.
  • Change all implementation functions to iterate over these variations.
  • Change the orphaned nodes serialization to support the extra data.
  • Make the serialization format backward compatible by special-casing the usual case of having a single variation at a given edited UFE path.
  • Add a log to tell the user when an edited prim is orphaned.
  • Add a has helper function in the orphaned node manager to check if a prim is already edited.
  • Use it to avoid editing a prim variant twice.
  • Fix how the orphaned manager handles notifications to determine orphans.
  • In particular, handle deactivated prims correctly: when switching variants, the order of the consequences is not deterministic, so the handling must not assume that edited prims are active.
  • Make the orphan manager handle orphans in two passes: first the new orphans, second the ones that are no longer orphans.
  • This way, non-orphans can write their metadata and it won't be deleted by orphans of the same name.

Changes to the prim updater manager:

  • Ignore edited prim that are orphaned when determining if a prim has edited descendants.
  • Adapt to changes to the orphaned manager by passing the Maya DAG path to the root of the edited nodes.
  • When merging to USD, when removing the pull information, use the helper function.

Changes to Maya reference translator (importer from USD to Maya):

  • Make many functions be purely in the implementation instead of declared private in the class.
  • This allows better hiding of the implementation.
  • This was also necessary to make the function callable from internal implementation functions.
  • Renamed LoadMayaReference to CreateMayaReference to better reflect what the function does.
  • Split the long update function into multiple function for clarity.
  • When trying to reuse Maya reference nodes, verify that the referenced file is the one that was expected.
  • This allows two prim with the same USD path but different references to work alongside each other.
  • This happens when one prim with two variants each contain a Maya reference.
  • Add more comments in the code to explain what is going on.
  • Add a log to tell the user when a reference does not have the expected file path.

@pierrebai-adsk pierrebai-adsk added bug Something isn't working adsk Related to Autodesk plugin labels Sep 26, 2023
MayaUsd::ProgressBarScope progressBar(1);
removePulledPrimMetadata(stage, pulledPrim);
removePulledPrimMetadata(ufePulledPath);
progressBar.advance();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: all removed code has moved into removePulledPrimMetadata()

This change adds support to edit-as-Maya a prim that is under a variant, with
each variation having a different Maya reference. That is, a single prim can
now be edited-as-Maya multiple times in parallel. This required some changes
on how those edited prim are tracked.

Edit-as-Maya authors data in the session layer related to the edited prim.
Previously, when edited-as-Maya were orphaned or ceased to be, the order in
which they were processed was random. This caused two prims with the same
name but in different variants to potentially clobber each other's data.
Now we process new orphans first, then new non-orphans second. This allows
new non-orphans to write their data without it being wiped out by an orphan.

Additional variant-related helper functions:
- Add the getEditTargetForVariants function helper function to create the USD edit target targeting all the variants that affect a given prim.
- Ensure getEditTargetForVariants works in more complex scenarios spanning payloads and references.
- Add the getVariantPath helper function to create a USD path containing a variant selection.

Additional pull information helper functions:
- Warn when we fail to remove pull information.

Change to the orphan manager:
- Support having multiple entries for a given UFE path.
- To differentiate between entries, the corresponding root Maya DAG path must be given in all functions to figure out which version of the prim we are dealing with.
- Fortunately, all callers always have on hand the DAG path of the root Maya node.
- Change all implementation functions to iterate over these variations.
- Change the orphaned nodes serialization to support the extra data.
- Make the serialization format backward compatible by special-casing the usual case of having a single variation at a given edited UFE path.
- Add a log to tell the user when an edited prim is orphaned.
- Add a has helper function in the orphaned node manager to check if a prim is already edited.
- Use it to avoid editing a prim variant twice.
- Fix how the orphaned manager handles notifications to determine orphans.
- In particular, handle deactvated prims correctly: when switching variants, the order of the consequences is not deterministic, so the handling must not assume that edited prims are active.
- Make the orphan manager handle orphans in two passes: first the new orphans, secon d the ones that are no longer orphans.
- This way, non-orphans can write their metadata and it won't be deleted by orphans of the same name.

Changes to the prim updater manager:
- Ignore edited prim that are orphaned when determining if a prim has edited descendants.
- Adapt to changes to the orphaned manager by passing the Maya DAG path to the root of the edited nodes.
- When merging to USD, when removing the pull information, use the helper function.

Changes to Maya reference translator (importer from USD to Maya):
- Make many functions be purely in the implementation instead of declared private in the class.
- This allows better hiding of the implementation.
- This was also necessary to make the function callable from internal implementation functions.
- Renamed LoadMayaReference to CreateMayaReference to better reflect what the function does.
- Split the long update function into multiple function for clarity.
- When trying to reuse Maya reference nodes, verify that the referenced file is the one that was expected.
- This allows two prim with the same USD path but different references to work alongside each other.
- This happens when one prim with two variants each contain a Maya reference.
- Add more comments in the code to explain what is going on.
- Add a log to tell the user when a reference does not have the expected file path.
@pierrebai-adsk pierrebai-adsk force-pushed the bailp/EMSUSD-623/multi-maya-refs-in-variants branch from b48d731 to cbfadf1 Compare September 27, 2023 16:03
for (const PullVariantInfo& info : trieNode->data()) {
const MDagPath& mayaPath = info.editedAsMayaRoot;
TF_VERIFY(writePullInformation(pulledPath, mayaPath));
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are a lot of changes like this in the orphan manager where what use to be a single value is now a vector because we support a single prim path being under different variants.

@@ -310,7 +354,8 @@ void OrphanedNodesManager::handleOp(const Ufe::SceneCompositeNotification::Op& o
// point. It may be an internal node, without data.
auto ancestorNode = _pulledPrims.node(op.path);
TF_VERIFY(ancestorNode);
recursiveSwitch(ancestorNode, op.path);
recursiveSwitch(ancestorNode, op.path, true);
recursiveSwitch(ancestorNode, op.path, false);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to process newly orphaned nodes first because that deletes information in various places while the ones that are no longer orphans add information. If we let the change be done in any order, we would sometimes lose information that got deleted by orphans.

@@ -345,63 +390,54 @@ void OrphanedNodesManager::handleOp(const Ufe::SceneCompositeNotification::Op& o
return;
}

auto parentHier = Ufe::Hierarchy::hierarchy(parentItem);
if (!parentHier->hasChildren()) {
Copy link
Collaborator Author

@pierrebai-adsk pierrebai-adsk Sep 27, 2023

Choose a reason for hiding this comment

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

This line was wrong because it ignored deactivated prims. Prims get deactivated when edited-as-Maya, so we would not process notifications correctly in all cases.

The code below has not changes much but all its indentation changed, compared by ignoring whitespace for an easier view of what changed.

@@ -626,8 +699,10 @@ OrphanedNodesManager::variantSetDescriptors(const Ufe::Path& p)
auto ancestor = Ufe::Hierarchy::createItem(path);
auto usdAncestor = std::static_pointer_cast<UsdUfe::UsdSceneItem>(ancestor);
auto variantSets = usdAncestor->prim().GetVariantSets();
auto setNames = variantSets.GetNames();
std::sort(setNames.begin(), setNames.end());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sort the variant set names to get a guaranteed consistent order for comparison purpose.

variantSet.SetVariantSelection('Rig')
editTarget = variantSet.GetVariantEditTarget(editTarget.GetLayer())
if variantSetName:
cacheParent.GetVariantSet(variantSetName).SetVariantSelection('Rig')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The code was doing unnecessary things. (I think this block was copy/paste from earlier in the same test.) I had to go through this unit test in early prototypes and fixed it back then, so I left the change in, even though it is not strictly necessary for this PR.

@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Sep 28, 2023
try:
os.remove(self.getCacheFileName())
except:
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you fix this so there is no Chorus warning? I think you just need to do except Exception: instead. I also found 2-3 other places identical to this (old code) that would be great if you could clean those up as well.

Copy link
Collaborator

@seando-adsk seando-adsk left a comment

Choose a reason for hiding this comment

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

Thanks for doing this.

@pierrebai-adsk
Copy link
Collaborator Author

THE PF failed because the OSX 2023, and 2022 build failed to launch. Unrelated to the latest changes,

@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Oct 2, 2023
@seando-adsk seando-adsk merged commit 2779e5c into dev Oct 3, 2023
12 of 14 checks passed
@seando-adsk seando-adsk deleted the bailp/EMSUSD-623/multi-maya-refs-in-variants branch October 3, 2023 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adsk Related to Autodesk plugin bug Something isn't working ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants