Returning an enum from UI delegate #1259
Conversation
Have the UI delegate return an enum that describes what happened instead of a plain bool. Keep track of the parent of any anonymous layer to be saved. Should be either another layer, where the identifier would need to be remapped, or if it's the root layer then its Stage, where the owning Proxy will need to have its path attribute remapped. Initial set of test for serialization of Usd edits.
| struct LayerParent | ||
| { | ||
| // Every layer that we are saving should have either a parent layer that | ||
| // we will need to remap to point to the new path, or the stage if it is an | ||
| // anonymous root layer. | ||
| SdfLayerRefPtr _layerParent; | ||
| UsdStageRefPtr _stageParent; | ||
| }; | ||
|
|
||
| struct stageLayersToSave | ||
| { | ||
| std::vector<std::pair<SdfLayerRefPtr, LayerParent>> _anonLayers; | ||
| std::vector<SdfLayerRefPtr> _dirtyFileBackedLayers; | ||
| }; |
There was a problem hiding this comment.
Do these two structs need to have the "MAYAUSD_CORE_PUBLIC" so they will be exported as well?
|
We had two issues with the change:
To fix both of these issues I changed all the places that we were holding to a stage ptr in order to track what needs to be saved and switched to using a proxy path. A string for the full path or a MDagPath. The one reason we needed UFE was for the utility function that maps between stage and proxy shape, so moving to the proxy name removed that dependency entirely. I added a new utility function to help go from the proxy name string to the usd stage, which was always possible but I didn't see any other utility function that did exactly that. |
|
Just noticed a linter problem. Will look into that now... |
| @@ -226,6 +228,20 @@ MStatus UsdMayaUtil::GetDagPathByName(const std::string& nodeName, MDagPath& dag | |||
| return status; | |||
| } | |||
|
|
|||
| UsdStageRefPtr UsdMayaUtil::GetStageByProxyName(const std::string& proxyPath) | |||
There was a problem hiding this comment.
I get that this is to remove dependence on UFE, but in doing so this is adding a by-name lookup of a proxy shape. If it's not called a lot, this is fine.
Also, I think it would be good to steer readers of this code to lib/mayaUsd/ufe/Utils.h, which will have much better performance, if a dependence on UFE is not an issue.
There was a problem hiding this comment.
Couple thoughts:
-
Do you think lookup performance would be bad given a unique path? It might not be as fast as some of the structures in UFE, but I would hope that when looking for a single object given a unique string that it would be fairly quick. Right now this function will be called once per proxy shape when saving, so it shouldn't be too many and definitely not called interactively.
-
We are going to have to revisit the utility function that I am replacing soon anyways, since it will no longer be valid to map between proxies and stages like we have it now. The stage cache map code that can go from UsdStage ptr to proxy is the one that will have to change.
There was a problem hiding this comment.
Once per proxy shape when saving is fine, not an issue.
| @@ -180,6 +181,10 @@ MString GetUniqueNameOfDagNode(const MObject& node); | |||
| MAYAUSD_CORE_PUBLIC | |||
| MStatus GetMObjectByName(const std::string& nodeName, MObject& mObj); | |||
|
|
|||
| /// Gets the UsdStage for the proxy shape node named \p nodeName. | |||
There was a problem hiding this comment.
I would add a comment pointer here to lib/mayaUsd/ufe/Utils.h
There was a problem hiding this comment.
Sorry, you just want me to mention in the comments that there is a similar function that uses UFE over in a different utilities file?
I believe the UFE approach would be to:
Convert the MDagPath into a UFE PathSegment.
Convert the PathSegment into a UFE Path.
Use the stage map to go from a Path to a stage ptr.
The only issue with the Maya way is what the performance is when you have a unique dag path and want the depend node, which I don't think is all that bad. Once you have the node then it's just a cast to our base class and we call the function on it to return the stage ptr.
There was a problem hiding this comment.
That seems very roundabout... If you have an MDagPath, you don't need to go through UFE! You can just get the MObject from the tail of the path, and get a pointer from that.
And yes, just add a comment saying if you already have a dependence on UFE, then lib/mayaUsd/ufe/Utils.h is better.
debug cleanup
fowlertADSK
added
the
ready-for-merge
kxl-adsk
deleted the
fowlert/MAYA-109976/return-error-code-for-issues-saving
branch
Have the UI delegate return an enum that describes what happened instead of a plain bool.
Keep track of the parent of any anonymous layer to be saved. Should be either another layer, where the identifier would need to be remapped, or if it's the root layer then its Stage, where the owning Proxy will need to have its path attribute remapped. The Session layer is the only one that would have neither a parent layer or proxy/stage tracked.
Initial set of test for serialization of Usd edits.