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

Maya 105566/serialize usd edits #1152

Merged
merged 8 commits into from
Feb 10, 2021
Merged

Conversation

fowlertADSK
Copy link
Contributor

This is a continuation of an earlier start on saving Usd edits that was posted as a draft review here: #871

This PR contains the option to save edits into the Maya file, or to push edits back to the .usd files themselves. Saving back to Usd will mean choosing files for all anonymous layers other than the Session layer. This can be done automatically by Maya or through a new UI contained in this PR.

New UI Included in this PR

The UI to choose which option to use has been integrated into the File Save options dialog box. This UI is contained in this PR but additional hooks were required to be added to Maya as well, so this UI will only appear in Maya PR’s starting now.
image

Before a Maya Save/Export, if the user has not chosen “Don’t ask me again”, a dialog will appear to allow the user to make the choice.
image

If there are anonymous layers in the scene being saved the user will be given an opportunity to choose their new file names/locations. The list of file-backed layers that will also be saved will be listed as well.
image

This same batch save dialog will also now be used in the Layer Editor for saving individual stages. Previously a user would have to first save their anonymous layers individually before the save operation would work on all dirty file-backed layers. Now this same dialog will be posted, giving the user the ability to save both their anonymous and file-backed layers at the same time.

In the previous save layer workflow, the option UI to save .usd files as either binary or ascii was embedded in the file dialog itself. This has now been moved to the Layer Editor options menu.
image

Option to save back to Usd files

Overview of how it works:

  1. Iterate over all proxy shapes, and their UsdStages, looking for types that have been registered with the Layer Manager as ones that are supported. This is currently only the ASDK ProxyShape node, both the Animal Logic and Pixar proxy shape nodes will be skipped.
  2. Recursively iterate through all UsdStage layer stacks looking for any that have dirty layers.
  3. Keep a list of the layers that need to be saved as well as the parent layers that may have them as a contained sublayer.
  4. The Layer Manager can have a callback registered with it that is called at this point with the list of UsdStages. If no callback is registered, the LM will continue to save edits depending on the current save option. If it needs to save anonymous layers to disk then it will automatically choose names for them.
  5. Currently the Layer Editor will register a callback with the Layer Manager, so in step 4 the LM will end up calling the LE and this is where the batch save dialog UI is posted.

Note: The Maya callback that starts all of this is a “Check” callback, so canceling in any of the dialogs will abort the file IO operation.

Option to save edits to Maya files

The behavior to save edits into Maya files is mostly unchanged from the previous draft review.

  1. The Layer Manager node is created in a pre-save callback if there are dirty layers to write to the Maya file.
  2. In a post-read callback the Usd layers saved into the file are recreated and the LM node is deleted. A map of original and new Usd identifiers to the newly created layers is kept so the new layer can be looked up using either the old or new identifier.
  3. After creating the layers, we edit the sub layer paths of any layer whose identifier has changed. This is what allows us to save/restore all anonymous layers.
  4. The base proxy shape uses the saved map to query what session/root layers to use when first evaluating its stage.
  5. LM uses a stage loaded callback to determine when to remove layers from its map. After each proxy shape has loaded its stage the LM map should be empty and not be holding any references to the layers it created.
  6. If there are dirty layers that need to be written out, the LM will write an entry for every loaded layer whether it was dirty or not, but only the dirty layers will be exported to a string and have their data stored in the Maya file.

Step 1 of 3:
* Adding the layer manager node which is responsible for
  saving usd edits to the Maya file.
… or ASCII) in the Layer Editor

Step 2 of 3:
* Renamed "Option" menu to "Options".
* Added new submenu for save .usd file format.
* No longer force Maya native file dialog when saving layers.

MAYA-109066 - Improvements to the Bulk Save UI
MAYA-109367 - Bulk Save UI supporting Multi proxyShapes

Step 2 of 3:
* Modified the save dialog to work with single stage
  and multiple stages. Combined the anonymous and dirty
  file backed layers into single dialog.
… File - Save options

MAYA-109081 - As a user, I'd like to have modal displaying my choices on how to save USD layers

Step 3 of 3:
* New MEL file/proc for adding USD options to the Maya File|Save{As}.
* New MEL file for building the USD Save Options - both in Maya
  options and as standalone dialog.
* Added warning text when choosing 2nd option.

* Minor cleanup
** Moved import dialog images into their own resource namespace
   to avoid any potential conflicts with Maya images.
** Put MayaUsd proxy shape template strings into resource.
** Added a few missing strings in resource from maya-usd menu.
** Fixed LayerEditor what's new highlighting.
@seando-adsk seando-adsk added the enhancement New feature or request label Feb 5, 2021
namespace UsdLayerEditor {

MAYAUSD_UI_PUBLIC
bool batchSaveLayersUIDelegate(const std::vector<UsdStageRefPtr>&);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This callback is registered with the Layer Manager to provide a UI to choose file names. The core library does not depend on Qt and can't be the one to create UI.

Comment on lines 132 to 163
auto usdSaveMenu = optionMenu->addMenu(
StringResources::getAsQString(StringResources::kUsdSaveFileFormat));
auto formatGroup = new QActionGroup(usdSaveMenu);
formatGroup->setExclusive(true);
auto formatBinary
= formatGroup->addAction(StringResources::getAsQString(StringResources::kBinary));
auto formatAscii
= formatGroup->addAction(StringResources::getAsQString(StringResources::kAscii));
auto grpAnn = StringResources::getAsQString(StringResources::kSaveLayerUsdFileFormatAnn);
auto grpSbm = StringResources::getAsQString(StringResources::kSaveLayerUsdFileFormatSbm);
formatBinary->setCheckable(true);
formatBinary->setData(1);
formatBinary->setToolTip(grpAnn);
formatBinary->setStatusTip(grpSbm);
formatAscii->setCheckable(true);
formatAscii->setData(0);
formatAscii->setToolTip(grpAnn);
formatAscii->setStatusTip(grpSbm);
usdSaveMenu->addAction(formatBinary);
usdSaveMenu->addAction(formatAscii);
bool isBinary = true;

auto formatArgsOption
= UsdMayaUtil::convert(MayaUsdOptionVars->mayaUsd_SaveLayerFormatArgBinaryOption);
if (MGlobal::optionVarExists(formatArgsOption)) {
isBinary = MGlobal::optionVarIntValue(formatArgsOption) != 0;
}
isBinary ? formatBinary->setChecked(true) : formatAscii->setChecked(true);
auto onFileFormatChanged = [=](QAction* action) {
MGlobal::setOptionVarValue(formatArgsOption, action->data().toInt());
};
QObject::connect(formatGroup, &QActionGroup::triggered, in_parent, onFileFormatChanged);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is to add the binary or ascii option for .usd files to the Layer Editor menu. It was previously in the file dialog itself.

@@ -410,81 +410,31 @@ LayerTreeModel::getAllAnonymousLayers(const LayerTreeItem* item /* = nullptr*/)

void LayerTreeModel::saveStage(QWidget* in_parent)
{
QString dialogTitle = StringResources::getAsQString(StringResources::kSaveStage);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We originally only displayed an error when trying to save while there were anonymous layers. Then we changed to a dialog for anonymous layers and a message dialog when you only had file-backed to save. Now we are removing those two separate cases and instead showing the dialog that has two sections, one for anonymous layers and the other to list the file-backed.

Comment on lines -200 to +217
// this does not work:
// if OpenMaya.MFnDependencyNode(obj).typeName == PROXY_NODE_TYPE
if (obj.hasFn(MFn::kShape)) {
MDagPath dagPath;
MFnDagNode(obj).getPath(dagPath);
auto shapePath = dagPath.fullPathName();

StageEntry entry;
if (getStageEntry(&entry, shapePath)) {
QTimer::singleShot(0, [THIS, entry]() {
Q_EMIT THIS->stageRenamedSignal(entry._displayName, entry._stage);
});
}

// doing it on idle give time to the Load Stage to set a file name
QTimer::singleShot(0, [THIS, obj]() { THIS->nodeRenamedCBOnIdle(obj); });
}
}

void MayaSessionState::nodeRenamedCBOnIdle(const MObject& obj)
{
// this does not work:
// if OpenMaya.MFnDependencyNode(obj).typeName == PROXY_NODE_TYPE
if (obj.hasFn(MFn::kShape)) {
MDagPath dagPath;
MFnDagNode(obj).getPath(dagPath);
auto shapePath = dagPath.fullPathName();

StageEntry entry;
if (getStageEntry(&entry, shapePath)) {
Q_EMIT stageRenamedSignal(entry._displayName, entry._stage);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This "node renamed" callback can be called during file IO but did not have an "Idle" implementation like other callbacks here. Now nodeNamedCB just uses a QTimer to run nodeRenamedCBOnIdle at a later time.

}

return true;
return SaveLayersDialog::saveLayerFilePathUI(*out_filePath, *out_pFormat);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the new dialog instead of the MEL scripts now.

Comment on lines -18 to +34
SaveLayersDialog(
const QString& title,
const QString& message,
const std::vector<LayerTreeItem*>& layerItems,
QWidget* in_parent);
typedef std::unordered_multimap<SdfLayerRefPtr, std::string, TfHash> stageLayerMap;

// Create dialog using single stage (from session state).
SaveLayersDialog(SessionState* in_sessionState, QWidget* in_parent);

#if defined(WANT_UFE_BUILD)
// Create dialog for bulk save using all stages.
SaveLayersDialog(QWidget* in_parent, const std::vector<UsdStageRefPtr>& stages);
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously this dialog was only used from the Layer Editor itself, but it can now be displayed from a File -> Save operation. The first constructor is used from the LE, and the dialog gets most of the information it needs from the SessionState. From the File -> Save callback function we have already iterated over all of the proxies and their stages and have the list of stages we need to save, so it uses this second constructor. Saving of Usd edits does require UFE so you will see some ifdef's for that.

Comment on lines 77 to 90
#if defined(WANT_UFE_BUILD)
SdfLayerRefPtr ProxyShape::computeRootLayer(MDataBlock& dataBlock, const std::string& filePath)
{
auto rootLayerName = dataBlock.inputValue(rootLayerNameAttr).asString();
return LayerManager::findLayer(UsdMayaUtil::convert(rootLayerName));
}

SdfLayerRefPtr ProxyShape::computeSessionLayer(MDataBlock& dataBlock)
{
auto sessionLayerName = dataBlock.inputValue(sessionLayerNameAttr).asString();
return LayerManager::findLayer(UsdMayaUtil::convert(sessionLayerName));
}
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in the previous PR, this is how a supported proxy shape can respond to the compute in the base proxy with the serialized layers.

Comment on lines +325 to +327
#if defined(WANT_QT_BUILD)
MayaUsd::LayerManager::SetBatchSaveDelegate(UsdLayerEditor::batchSaveLayersUIDelegate);
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hooks up the callback to provide a UI to choose names/locations for anonymous layers during save.

Copy link
Contributor

@aloysbaillet aloysbaillet left a comment

Choose a reason for hiding this comment

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

Thanks @fowlertADSK !
The overall feature set looks great, my main comment would be to add a fourth mode which is to save file-backed USD files but keep anonymous layers anonymous. I believe that's what we would use by default.
Also it would be great to check that this all works fine with autosave, as we just noticed some events such as kBeforeSave are not sent by maya during autosave...

and Maya is running in interactive mode, then a UI dialog can be displayed to provide a choice
of file names and locations for all anonymous layers. If Maya is not running in interactive
mode, or there is no installed delegate to handle it, then Maya will automatically choose name
and locations for all anonymous layers.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's a case to be made for a new "Save back to .usd files and keep anonymous layers" case which would save all external files, but not force people to choose file locations for any anonymous layer in the scene. This is a use case we have currently where if a USD file is supposed to be saved on disk we create the file on disk, but for data that we prefer not having to choose a file location then we use anonymous layers. This early decision shouldn't have to be compromised at save time by forcing users to choose between saving their unsaved USD files on disk and giving up on anonymous layers and having to create new files for all.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a good point - both because it makes a lot of sense as default when running in batch mode, as @aloysbaillet points out, and also because I thin it "conceptually fits" with anonymous layers vs file-backed layers. That is, I think of anonymous layers as more for temporary storage, which might make sense saved out to the maya scene, whereas file-backed layers are generally for more "permanent" changes which you might want to, ie, share with others, publish downstream, etc.

However - my next thought was that, since the session layer is already treated in a special manner (ie, always saved to maya), and the session layer is the only anonymous layer we use at Luma - and likely the only anonymous layer 90%+ of the time - we would effectively already be getting the behavior @aloysbaillet describes, because of the special session-layer handling.

@aloysbaillet - I'm curious, does Animal Logic make a lot of use of non-session-layer anonymous layers? Or were you thinking more of "what if" and "just in case"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to be clear, are we talking about putting all anonymous layers into the Maya file (and saving file backed layers to disk) or are you suggesting an option where we ignore anonymous layers entirely (because you consider them kind of temporary)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aloysbaillet we talked some internally about your suggestion and I think we’d like to get this merged with the existing options for testing before moving on to additional changes. Right now the UI does implement what was requested by design so we have completed the task. I think you also had some questions about overriding some of the file naming/saving behavior, which is what we were starting to think about with the delegate function that the plugin registers to have the UI lib help, so maybe we can work to get this in and then start up a new conversation about next steps. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I confirm I would like an option to save all external files, but keep anonymous layers anonymously saved inside the maya file. And I'm happy to wait for that option!
The tension between simplicity and flexibility is quite apparent here, adding another option will make things more confusing for users, and the layer manager can be used to save all external files first, then the option of saving all unsaved layers in the maya scene would work for us.

}
}

removeManagerNode(lm);
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great to mark the maya scene as clean once the layer manager node is removed so that the "unsaved scene" dialog doesn't pop up after opening a scene that contains a layer manager node and opening a new scene (or closing maya) right after.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll look into that. Can't remember off the top of my head if there's a MGlobal/MFileIO call to easily do that or not...

@aloysbaillet
Copy link
Contributor

I also just thought that it would be great to have the dag path of the proxy shape next to each anonymous layer (and maybe the named ones as well!) in the save dialog as it would be quite tricky to know which layer comes from which shape if you have more than one in the scene...

@seando-adsk
Copy link
Collaborator

@aloysbaillet There are tooltips on both the anonymous and file backed layers that show the proxy shape(s) they are from. Does that work?

@aloysbaillet
Copy link
Contributor

Oh that's great! Thanks Sean I didn't see that. I will try to build that branch and give it a go in maya over the next few days.

// 3: ignore all Usd edits.
// mayaUsd_SerializedUsdEditsLocationPrompt: optionVar to force a prompt on every save
#define MAYA_USD_OPTIONVAR_TOKENS \
(mayaUsd_ProxyTargetsSessionLayerOnOpen)(mayaUsd_SaveLayerFormatArgBinaryOption)( \
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor, but I'd like to stick the the convention of using // clang-format off for token defintions - ie, like in jobArgs.h. I know, I was one of the big proponents of clang-format, but I think this is an exception where it looks much nicer manually formatted, and makes for much cleaner diffs if we need to add/remove tokens later...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Sean actually mentioned that to me when I complained that it kept formatting the change like this, but I forgot to go back and add that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just wanted to flag also that there was a bit of clean-up in this area from #1119 that probably created a merge conflict that needs resolving. I put the mayaUsd_ prefix in the values only:

...
    ((SaveLayerFormatArgBinaryOption, "mayaUsd_SaveLayerFormatArgBinaryOption")) \
...

Copy link
Contributor

@pmolodo pmolodo left a comment

Choose a reason for hiding this comment

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

Mostly looks good! Looking forward to getting these changes in..

@kxl-adsk kxl-adsk added the proxy Related to base proxy shape label Feb 9, 2021
…e_usd_edits

# Conflicts:
#	lib/mayaUsd/base/tokens.h
#	lib/mayaUsd/nodes/proxyShapeBase.cpp
@fowlertADSK fowlertADSK added the ready-for-merge Development process is finished, PR is ready for merge label Feb 10, 2021
@kxl-adsk kxl-adsk merged commit 6f8f193 into dev Feb 10, 2021
@kxl-adsk kxl-adsk deleted the MAYA-105566/serialize_usd_edits branch February 10, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request proxy Related to base proxy shape 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.

7 participants