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

Tremblp/maya 112824/pull push 01 to dev #1789

Merged
merged 12 commits into from
Nov 5, 2021

Conversation

ppt-adsk
Copy link
Collaborator

@ppt-adsk ppt-adsk commented Oct 28, 2021

This pull request introduces the edit as Maya data / merge back to USD workflow (a.k.a pull and push, respectively). This workflow is used to convert USD data to Maya data for editing, then back to USD once editing is done. It leverages existing USD import support for edit as Maya (pull), and USD export support for merge back to USD (push). In a similar way to the import and export prim readers and writers, the edit as Maya / merge back to USD uses prim updaters, created on a per type basis, to perform the conversions.

Once a USD hierarchy has been edited as Maya, it appears in the Outliner hierarchy as parented under its original USD parent. However, in the Maya data model a separate, hidden Maya hierarchy is created to contain the pulled objects in their Maya representation. Because the paths to these pulled objects are pure Maya paths, all existing Maya editors and tools can operate on pulled data, and they will appear in the Maya selection as normal Maya objects.

ppt-adsk and others added 2 commits October 27, 2021 11:14
* Second demo

* Added marking menu support for USD and DAG objects

* Only show USD marking menu for pulled DAG objects

* Added push animation support and push to USD marking menu

* Bake animation only when pulled object is animated

* Import MayaReference under common parent. Fix pull of MayaReference.

* Move pull/pushBack workflow to updater. Python script is still available, but soon we should migrate the rest of the code

* Added missing export symbols.

* WIP - Shot based animation

* Extend registry to support look-up by maya type

* WIP - Shot based animation

* Support for pulled object directly under proxy shape
* Don't show pull menu when no USD prims are selected

* WIP - Shot based animation

* Added new PullObjectHierarchy to handle pickwalking.

* MayaReference updater called from manager

* Fix baking

* Clang

* WIP - Shot based animation

* Hide Maya pulled object.
* Remove complex Maya path stuff.
* New UsdPathMappingHandler

* Implement copy spec in mayareference updater

* Implemented maya reference push back to payload in cache variant

* WIP - Shot based animation

* Register for new callback "addItemsToOutlinerNodePopupMenu" to add
  context menu items to Maya objects.
* Moved "USD Layer Editor" context menu (on proxy) to plugin (from Maya).

* AutoPull for updaters that declare a need for it

* CopyBetween data models. Old script is now pretty much replaced.

* WIP - Shot based animation

* Register/unregister a Maya path mapping handler with
  the core library.
* Refactor some common code in {Usd/ProxyShape}Hierarchy.
* Optimize the path mapping handler with a cache and
  check ancestors of input host path.

* Invisibile prims don't get notifications on _PropagateDirtyBits or Sync, so when a prim turns invisible make sure all reprs can draw correctly.

* Clang format.

* MAYA-111521: Fix error messages showing up when opening the "Applied Schema" category.

* Basis Curves have the same issue meshes do. Invisible prims get no chances to update, so make sure all reprs will draw correctly when the prim turns invisible.

* Update SchemaRegistry callsite for a USD function name change

* Updating the build doc with the latest supported version of USD dev

* Disable the new point snapping support for the time being.

* Implement setMatrixCmd() for common transform API.

* MAYA-112058 - AE Missing Information on Materials and texture
              file prims USD v21.02 in Maya 2022.1

* In Maya 2022.1 we need to hold onto the Ufe SceneItem to make
  sure it doesn't go stale. This is not needed in latest Maya.

* MAYA-112126: make sure parented object is added to selection list after it is moved.

* Address feedbacks.

* MAYA-111835: Fix Grouping a prim twice that crashed Maya.

* check for return status when calling MGlobal::executeCommand.

* Simplify selection logic.

* Revert "MAYA-112126: make sure parented object is added to selection list after it is moved."

* Update ae_template.py

Python 2 strings are ascii by default, whereas Python 2 strings as are unicode by default. This string had a unicode char (for apostrophe) that I replaced with the ascii one.

* Working Dag pull / push with pull root and pull parent.

* Parent transform onto pull parent, remove pull root on last child, and hide it in Outliner.

* Update to latest changes from Krystian.

* Node lock / unlock on pull / push, and copy operation support.

* WIP - Shot based animation

* Fixed crash from UFE test.

* Respect edit target

* Use new export roots implementation.

* Ufe PathMappingHandler is now in UFE v3

* clang format

* Small fix for Outliner callback

* Fixing bad merge of test.

* Two-step pull, pull clear, design wording and icons, and tests. (#251)

* Two-step pull, pull clear, design wording and icons, and tests.

* Addressed code review comments.

* Addressed test code review feedback.

* Edit as Maya tests conditional to UFE path mapper.

* Edit as Maya depends on UFE v3 path mapping.

* More edit as Maya dependence on UFE v3 path mapping.

* More conditional compilation of edit as Maya code.

* Yet more conditional compilation of edit as Maya code.

* Conditional compilation of edit as Maya code part 6.

* Conditional compilation of edit as Maya code part 7.

* Conditional compilation of edit as Maya code part 8.

* Conditional compilation of edit as Maya code part 9.

* WIP: layer traversal on push copy specs.

* Prim updater push copy spec code cleanup.

* Production-quality changes for push traversal.

* WIP changes for push traversal.

* Split out DiscardEdits and PushEnd.

* Layer traversal error reporting and discard edits fix.

* Prim updater cleanup.

* Debugging output cleanup.

Co-authored-by: Krystian Ligenza <[email protected]>
Co-authored-by: Sean Donnelly <[email protected]>
Co-authored-by: krickw <[email protected]>
Co-authored-by: Hamed Sabri <[email protected]>
Co-authored-by: Dan McGarry <[email protected]>
@@ -319,8 +319,7 @@ MStatus MayaUSDExportCommand::doIt(const MArgList& args)
std::string rootPath = tmpArgList.asString(0).asChar();

if (!rootPath.empty()) {
MDagPath rootDagPath;
UsdMayaUtil::GetDagPathByName(rootPath, rootDagPath);
MDagPath rootDagPath = UsdMayaUtil::nameToDagPath(rootPath);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed duplicate name to Dag path utility and its uses.

@@ -28,6 +25,19 @@ target_sources(${PROJECT_NAME}
writeJobContext.cpp
)

# Edit as Maya requires UFE path mapping.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, edit as Maya is conditional on UFE v3, because it requires UFE path mapping for proper Outliner support. However, it would theoretically be possible to back-port the UFE path mapping interface to UFE v2, as it is a purely additive interface change to UFE. This will be for future consideration.

@@ -0,0 +1,325 @@
import maya.cmds as cmds
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file was written for an earlier prototype. Parts of it could be cleaned up, as they are no longer relevant.

Choose a reason for hiding this comment

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

We shouldn't need this file anymore. Let's remove it.

@@ -589,6 +589,11 @@ const MDagPath& UsdMaya_ReadJob::GetMayaRootDagPath() const { return mMayaRootDa

double UsdMaya_ReadJob::timeSampleMultiplier() const { return mTimeSampleMultiplier; }

const UsdMayaPrimReaderContext::ObjectRegistry& UsdMaya_ReadJob::GetNewNodeRegistry() const
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Provide access to the import new Maya node registry, so that the edit as Maya code can be run on them.

Choose a reason for hiding this comment

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

Should it be available in python?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds potentially useful, but I could not find a Python wrapper wrapper for a UsdMaya_ReadJob.

@@ -819,4 +827,9 @@ bool UsdMaya_WriteJob::_CheckNameClashes(const SdfPath& path, const MDagPath& da
return true;
}

const UsdMayaUtil::MDagPathMap<SdfPath>& UsdMaya_WriteJob::GetDagPathToUsdPathMap() const
{
return mDagPathToUsdPathMap;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Provide access to the export Dag path to USD path map, so the merge to USD code can access it.


const MDagPath& UsdMayaPrimUpdater::GetDagPath() const { return _dagPath; }
return SdfCopySpec(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Plain SdfCopySpec overload recurses to USD children, which is not useful for us as we want to control the primSpec traversal and provide virtual function callbacks. The new overload allows for single primSpec control, but requires complex functors to obtain this behavior.

return true;
}

bool PrimUpdaterManager::Pull(const Ufe::Path& path)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Core of the review: edit as Maya (pull). The pull is done is two steps, first import, then a pull customization step is done on the pulled Maya data.

UsdMayaPrimUpdaterContext context(srcProxyShape->getTime(), proxyStage, userArgs);

// FIXME Re-implement with proper traversal. PPT, 22-Oct-2021.
#if 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copying was part of an earlier prototype, must be re-implemented with the newer architecture in this pull request.

@@ -215,19 +215,6 @@ MStatus UsdMayaUtil::GetMObjectByName(const std::string& nodeName, MObject& mObj
return status;
}

MStatus UsdMayaUtil::GetDagPathByName(const std::string& nodeName, MDagPath& dagPath)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed duplicated functionality.

@@ -185,10 +185,6 @@ MStatus GetMObjectByName(const std::string& nodeName, MObject& mObj);
MAYAUSD_CORE_PUBLIC
UsdStageRefPtr GetStageByProxyName(const std::string& nodeName);

/// Gets the Maya MDagPath for the node named \p nodeName.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Duplicated functionality, removed.

@@ -0,0 +1,325 @@
import maya.cmds as cmds

Choose a reason for hiding this comment

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

We shouldn't need this file anymore. Let's remove it.

@@ -589,6 +589,11 @@ const MDagPath& UsdMaya_ReadJob::GetMayaRootDagPath() const { return mMayaRootDa

double UsdMaya_ReadJob::timeSampleMultiplier() const { return mTimeSampleMultiplier; }

const UsdMayaPrimReaderContext::ObjectRegistry& UsdMaya_ReadJob::GetNewNodeRegistry() const

Choose a reason for hiding this comment

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

Should it be available in python?

, _baseDagToUsdPaths(UsdMayaUtil::getDagPathMap(depNodeFn, usdPath))
namespace {
// Set name that will be used to hold all pulled objects
// MOVED

Choose a reason for hiding this comment

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

If it's moved, why do we need it here still?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Move was incomplete, readPullInformation() needed to come along with it to the PrimUpdaterManager.

// MOVED
const MString kPullDGMetadataKey("Pull_UfePath");

Ufe::Path usdToMaya(const std::string& pathStr)

Choose a reason for hiding this comment

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

Do we have a utility for it already? Looks like something useful in general.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From the call site it appears that this is just Ufe::PathString::path().

auto isDagPathAnimated = [](const MDagPath& dagPath) {
int upstreamDependencies = -1;
MString pyCommand;
pyCommand.format(

Choose a reason for hiding this comment

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

We should convert this to C++ and expose any missing APIs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Excellent idea, but I feel this is out of scope for this pull request, and would be best handled as a separate task. Entered internally to Autodesk as MAYA-114591.


//! \brief Maya run-time hierarchy handler with support for pulled Maya objects.
/*!
TO DOC

Choose a reason for hiding this comment

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

Now would be a good time to update documentation.

// start with |
MString firstPath = paths[0];
if (firstPath.substringW(0, 0) != "|")
return;

Choose a reason for hiding this comment

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

Why do we need to do this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a verbatim change from dev --- I wonder why it is showing up here. See

#1790

The reason is that as originally written, non-absolute Maya paths (e.g. just a node name, "pSphere1") are given to Ufe::PathString::path(), and since we have not (yet) registered any UFE handlers for this case (see Ufe::PathString::setCreateSingleSegmentPathFn()), PathString::path() cannot decide which run-time to choose for the path, so it throws, the exception is not caught, and we crash. A better fix would be for Maya to register a handler with UFE for this case, and the problem would be solved for all UFE clients, not just this one.

{
const MObject& parentNode = GetMayaObject();

UsdMayaTranslatorMayaReference::UnloadMayaReference(parentNode);

Choose a reason for hiding this comment

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

This doesn't discard edits on MayaReference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Quite true. If you feel this needs to be part of this pull request, I can have a look, otherwise we can consider implementing this in the next phase, which will be for Maya reference workflows.

@@ -50,6 +50,7 @@ global proc mayaUSDRegisterStrings() {
register("kMenuClear", "Clear");
register("kMenuRevertToFile", "Revert to File");
register("kMenuLayerEditor", "USD Layer Editor");
register("kContextMenuLayerEditor", "USD Layer Editor...");

Choose a reason for hiding this comment

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

kMenuLayerEditor and kContextMenuLayerEditor look very similar. Just checking that it's not a mistake to have both.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another change that I'm afraid I don't know much about. Both are used in plugin/adsk/scripts/mayaUsdMenu.mel.

@@ -155,6 +155,12 @@ def testUsdGroup(self):
groupPath = ufe.Path([mayaPathSegment, usdUtils.createUfePathSegment("/Ball_set/Props/newGroup1")])
self.assertEqual(globalSelection.front(), ufe.Hierarchy.createItem(groupPath))

# Group object (a.k.a parent) will be added to selection list. This behavior matches the native Maya group command.
globalSelection = ufe.GlobalSelection.get()

Choose a reason for hiding this comment

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

How is this change related to the rest of the PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is not. I looked at the history, and this change was done as part of the work on this feature in our internal repository, so I would need to go back and trace the origin of this change further. Since it is a test-only change, it passes, and the test ensures we meet requirements, I suggest we leave it in.

@ppt-adsk ppt-adsk added the do-not-merge-yet Development is not finished, PR not ready for merge label Nov 1, 2021
@ppt-adsk
Copy link
Collaborator Author

ppt-adsk commented Nov 1, 2021

Review comments as of 1-Nov-2021 addressed. Labeled as do not merge, because of a regression with Maya reference prim creation found internally, to be investigated.

" upstream = cmds.evaluationManager(ust='^1s')\n"
" for node in upstream:\n"
" if cmds.nodeType(node) == 'mayaUsdProxyShape':\n"
" countNonProxyShape -= 1\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see where countNonProxyShape gets initialized so 1 can be subtracted from it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Absolutely, this will generate a NameError exception. Will investigate, good catch.

@@ -49,71 +51,56 @@ class UsdMayaPrimUpdater
Push = 1 << 0,
Pull = 1 << 1,
Clear = 1 << 2,
AutoPull = 1 << 3,
All = Push | Pull | Clear
Copy link
Collaborator

Choose a reason for hiding this comment

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

IDK if AutoPull should be in All?

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 don't know either. I will assume the above is correct, until shown otherwise.

kxl-adsk
kxl-adsk previously approved these changes Nov 3, 2021
Copy link

@kxl-adsk kxl-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'm accepting this, but there is still more cleanup to do in subsequent PRs.

Comment on lines +95 to +97
.def("push", push)
.def("pull", pull)
.def("pullClear", discardEdits)
Copy link

Choose a reason for hiding this comment

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

These three looks like a good candidates for cleanup

lib/usd/translators/mayaReferenceUpdater.cpp Show resolved Hide resolved
@ppt-adsk ppt-adsk removed the do-not-merge-yet Development is not finished, PR not ready for merge label Nov 3, 2021
kxl-adsk
kxl-adsk previously approved these changes Nov 3, 2021
@kxl-adsk kxl-adsk added the workflows Related to in-context workflows label Nov 3, 2021
@ppt-adsk ppt-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge workflows Related to in-context workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants