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.

Sign up for GitHub

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

Returning an enum from UI delegate #1259

Merged
merged 4 commits into from
Mar 24, 2021
Merged

Returning an enum from UI delegate #1259

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
67a1564
Returning an enum from UI delegate
fowlertADSK Mar 2, 2021
ffd5b95
Track what to save by proxy path instead of stage ptr
fowlertADSK Mar 22, 2021
23b651c
Formatting fix
fowlertADSK Mar 22, 2021
3755270
Debug failing test
fowlertADSK Mar 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions lib/mayaUsd/nodes/layerManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <maya/MArrayDataBuilder.h>
#include <maya/MArrayDataHandle.h>
#include <maya/MDagPath.h>
#include <maya/MDagPathArray.h>
#include <maya/MDataBlock.h>
#include <maya/MDataHandle.h>
#include <maya/MFileIO.h>
Expand Down Expand Up @@ -157,13 +158,13 @@ void convertAnonymousLayersRecursive(
MayaUsd::utils::LayerParent parentPtr;
if (stage->GetRootLayer() == layer) {
parentPtr._layerParent = nullptr;
parentPtr._stageParent = stage;
parentPtr._proxyPath = basename;
} else if (stage->GetSessionLayer() == layer) {
parentPtr._layerParent = nullptr;
parentPtr._stageParent = nullptr;
parentPtr._proxyPath.clear();
} else {
parentPtr._layerParent = layer;
parentPtr._stageParent = nullptr;
parentPtr._proxyPath.clear();
}

std::vector<std::string> sublayers = layer->GetSubLayerPaths();
Expand Down Expand Up @@ -230,7 +231,7 @@ class LayerDatabase : public TfWeakBase
std::map<std::string, SdfLayerRefPtr> _idToLayer;
TfNotice::Key _onStageSetKey;
std::set<unsigned int> _supportedTypes;
std::vector<UsdStageRefPtr> _stagesToSave;
MDagPathArray _proxiesToSave;
static MCallbackId preSaveCallbackId;
static MCallbackId preExportCallbackId;
static MCallbackId postNewCallbackId;
Expand Down Expand Up @@ -363,7 +364,7 @@ bool LayerDatabase::getStagesToSave(bool isExport)
bool checkSelection = isExport && (MFileIO::kExportTypeSelected == MFileIO::exportType());
const UFE_NS::GlobalSelection::Ptr& ufeSelection = UFE_NS::GlobalSelection::get();

_stagesToSave.clear();
_proxiesToSave.clear();
for (const auto& stage : allStages) {
auto stagePath = MayaUsd::ufe::stagePath(stage);
if (!checkSelection
Expand All @@ -376,15 +377,15 @@ bool LayerDatabase::getStagesToSave(bool isExport)
SdfLayerHandleVector allLayers = stage->GetLayerStack(true);
for (auto layer : allLayers) {
if (layer->IsDirty()) {
_stagesToSave.push_back(stage);
_proxiesToSave.append(proxyDagPath);
break;
}
}
}
}
}

return !_stagesToSave.empty();
return (0 != _proxiesToSave.length());
}

bool LayerDatabase::saveUsd(bool isExport)
Expand All @@ -395,7 +396,7 @@ bool LayerDatabase::saveUsd(bool isExport)

if (MayaUsd::utils::kIgnoreUSDEdits != opt) {
if (_batchSaveDelegate) {
result = _batchSaveDelegate(_stagesToSave);
result = _batchSaveDelegate(_proxiesToSave);
}

// kAbort: we should abort and return false, which Maya will take as
Expand Down Expand Up @@ -429,7 +430,7 @@ bool LayerDatabase::saveUsd(bool isExport)
result = MayaUsd::kCompleted;
}

_stagesToSave.clear();
_proxiesToSave.clear();
return (MayaUsd::kCompleted == result);
}

Expand Down
3 changes: 2 additions & 1 deletion lib/mayaUsd/nodes/layerManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include <maya/MMessage.h>
#include <maya/MObject.h>
#include <maya/MDagPathArray.h>
#include <maya/MPxNode.h>
#include <maya/MString.h>
#include <maya/MTypeId.h>
Expand All @@ -46,7 +47,7 @@ enum BatchSaveResult
kPartiallyCompleted // Callback has handled the saving of some stages, but not all. Layer
// Manager should continue to look for unsaved stages.
};
typedef std::function<BatchSaveResult(const std::vector<UsdStageRefPtr>&)> BatchSaveDelegate;
typedef std::function<BatchSaveResult(const MDagPathArray&)> BatchSaveDelegate;

/*! \brief Maya dependency node responsible for serializing unsaved Usd edits.

Expand Down
16 changes: 16 additions & 0 deletions lib/mayaUsd/utils/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
//
#include "util.h"

#include <mayaUsd/nodes/proxyShapeBase.h>

#include <maya/MAnimControl.h>
#include <maya/MAnimUtil.h>
#include <maya/MArgDatabase.h>
Expand Down Expand Up @@ -226,6 +228,20 @@ MStatus UsdMayaUtil::GetDagPathByName(const std::string& nodeName, MDagPath& dag
return status;
}

UsdStageRefPtr UsdMayaUtil::GetStageByProxyName(const std::string& proxyPath)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couple thoughts:

  1. 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.

  2. 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Once per proxy shape when saving is fine, not an issue.

{
MObject mobj;
MStatus status = UsdMayaUtil::GetMObjectByName(proxyPath, mobj);
if (status == MStatus::kSuccess) {
MFnDependencyNode fn;
fn.setObject(mobj);
MayaUsdProxyShapeBase* pShape = static_cast<MayaUsdProxyShapeBase*>(fn.userNode());
return pShape ? pShape->getUsdStage() : nullptr;
}

return nullptr;
}

MStatus UsdMayaUtil::GetPlugByName(const std::string& attrPath, MPlug& plug)
{
std::vector<std::string> comps = TfStringSplit(attrPath, ".");
Expand Down
5 changes: 5 additions & 0 deletions lib/mayaUsd/utils/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <pxr/pxr.h>
#include <pxr/usd/sdf/path.h>
#include <pxr/usd/usd/attribute.h>
#include <pxr/usd/usd/stage.h>
#include <pxr/usd/usd/timeCode.h>

#include <maya/MArgDatabase.h>
Expand Down Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would add a comment pointer here to lib/mayaUsd/ufe/Utils.h

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

MAYAUSD_CORE_PUBLIC
UsdStageRefPtr GetStageByProxyName(const std::string& nodeName);

/// Gets the Maya MDagPath for the node named \p nodeName.
MAYAUSD_CORE_PUBLIC
MStatus GetDagPathByName(const std::string& nodeName, MDagPath& dagPath);
Expand Down
26 changes: 13 additions & 13 deletions lib/mayaUsd/utils/utilSerialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "utilSerialization.h"

#include <mayaUsd/base/tokens.h>
#include <mayaUsd/ufe/Utils.h>
#include <mayaUsd/utils/util.h>
#include <mayaUsd/utils/utilFileSystem.h>

Expand Down Expand Up @@ -103,7 +102,7 @@ void populateChildren(

if (subLayer->IsAnonymous()) {
MayaUsd::utils::LayerParent p;
p._stageParent = nullptr;
p._proxyPath.clear();
p._layerParent = layer;
anonLayersToSave.push_back(std::make_pair(subLayer, p));
} else if (subLayer->IsDirty()) {
Expand All @@ -115,23 +114,20 @@ void populateChildren(
recursionDetector->pop();
}

bool saveRootLayer(SdfLayerRefPtr layer, UsdStageRefPtr stage)
bool saveRootLayer(SdfLayerRefPtr layer, const std::string& proxy)
{
if (!layer || !stage || layer->IsAnonymous()) {
if (!layer || proxy.empty() || layer->IsAnonymous()) {
return false;
}

auto stagePath = MayaUsd::ufe::stagePath(stage);
std::string strStagePath = stagePath.popHead().string();

std::string fp = layer->GetRealPath();
#ifdef _WIN32
// Building a string that includes a file path for an executeCommand call
// can be problematic, easier to just switch the path separator.
fp = TfStringReplace(fp, "\\", "/");
#endif

MayaUsd::utils::setNewProxyPath(MString(strStagePath.c_str()), MString(fp.c_str()));
MayaUsd::utils::setNewProxyPath(MString(proxy.c_str()), MString(fp.c_str()));

return true;
}
Expand Down Expand Up @@ -254,21 +250,25 @@ SdfLayerRefPtr saveAnonymousLayer(
if (parent._layerParent) {
parent._layerParent->GetSubLayerPaths().Replace(
anonLayer->GetIdentifier(), newLayer->GetIdentifier());
} else if (parent._stageParent) {
saveRootLayer(newLayer, parent._stageParent);
} else if (!parent._proxyPath.empty()) {
saveRootLayer(newLayer, parent._proxyPath);
}
}
return newLayer;
}

void getLayersToSaveFromProxy(PXR_NS::UsdStageRefPtr stage, stageLayersToSave& layersInfo)
void getLayersToSaveFromProxy(const std::string& proxyPath, stageLayersToSave& layersInfo)
{
auto root = stage->GetRootLayer();
auto stage = UsdMayaUtil::GetStageByProxyName(proxyPath);
if (!stage) {
return;
}

auto root = stage->GetRootLayer();
populateChildren(root, nullptr, layersInfo._anonLayers, layersInfo._dirtyFileBackedLayers);
if (root->IsAnonymous()) {
LayerParent p;
p._stageParent = stage;
p._proxyPath = proxyPath;
p._layerParent = nullptr;
layersInfo._anonLayers.push_back(std::make_pair(root, p));
} else if (root->IsDirty()) {
Expand Down
4 changes: 2 additions & 2 deletions lib/mayaUsd/utils/utilSerialization.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ struct LayerParent
// 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;
std::string _proxyPath;
};

struct stageLayersToSave
Expand Down Expand Up @@ -106,7 +106,7 @@ PXR_NS::SdfLayerRefPtr saveAnonymousLayer(
layers that will need to be saved.
*/
MAYAUSD_CORE_PUBLIC
void getLayersToSaveFromProxy(PXR_NS::UsdStageRefPtr stage, stageLayersToSave& layersInfo);
void getLayersToSaveFromProxy(const std::string& proxyPath, stageLayersToSave& layersInfo);

} // namespace utils
} // namespace MAYAUSD_NS_DEF
Expand Down
5 changes: 2 additions & 3 deletions lib/usd/ui/layerEditor/batchSaveLayersUIDelegate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,12 @@ void UsdLayerEditor::initialize()
}
}

MayaUsd::BatchSaveResult
UsdLayerEditor::batchSaveLayersUIDelegate(const std::vector<UsdStageRefPtr>& stages)
MayaUsd::BatchSaveResult UsdLayerEditor::batchSaveLayersUIDelegate(const MDagPathArray& proxyShapes)
{
if (MGlobal::kInteractive == MGlobal::mayaState()) {
auto opt = MayaUsd::utils::serializeUsdEditsLocationOption();
if (MayaUsd::utils::kSaveToUSDFiles == opt) {
UsdLayerEditor::SaveLayersDialog dlg(nullptr, stages);
UsdLayerEditor::SaveLayersDialog dlg(nullptr, proxyShapes);

if (QDialog::Rejected == dlg.exec()) {
return MayaUsd::kAbort;
Expand Down
4 changes: 3 additions & 1 deletion lib/usd/ui/layerEditor/batchSaveLayersUIDelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
#include <pxr/pxr.h>
#include <pxr/usd/usd/common.h>

#include <maya/MDagPathArray.h>

#include <vector>

PXR_NAMESPACE_USING_DIRECTIVE
Expand All @@ -33,7 +35,7 @@ MAYAUSD_UI_PUBLIC
void initialize();

MAYAUSD_UI_PUBLIC
MayaUsd::BatchSaveResult batchSaveLayersUIDelegate(const std::vector<UsdStageRefPtr>&);
MayaUsd::BatchSaveResult batchSaveLayersUIDelegate(const MDagPathArray&);

} // namespace UsdLayerEditor

Expand Down
23 changes: 12 additions & 11 deletions lib/usd/ui/layerEditor/saveLayersDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include <pxr/usd/sdf/layer.h>

#include <maya/MDagPathArray.h>
#include <maya/MGlobal.h>
#include <maya/MQtUtil.h>
#include <maya/MString.h>
Expand Down Expand Up @@ -241,28 +242,28 @@ class SaveLayerPathRowArea : public QScrollArea
namespace UsdLayerEditor {

#if defined(WANT_UFE_BUILD)
SaveLayersDialog::SaveLayersDialog(QWidget* in_parent, const std::vector<UsdStageRefPtr>& stages)
SaveLayersDialog::SaveLayersDialog(QWidget* in_parent, const MDagPathArray& proxyShapes)
: QDialog(in_parent)
, _sessionState(nullptr)
{
MString msg, nbStages;

nbStages = stages.size();
nbStages = proxyShapes.length();
msg.format(StringResources::getAsMString(StringResources::kSaveXStages), nbStages);
setWindowTitle(MQtUtil::toQString(msg));

// For each stage collect the layers to save.
for (const auto& stage : stages) {
// Get the name of this stage.
auto stagePath = MayaUsd::ufe::stagePath(stage);
std::string stageName = !stagePath.empty() ? stagePath.back().string() : "Unknown";
for (const auto& shape : proxyShapes) {

getLayersToSave(stage, stageName);
getLayersToSave(shape.fullPathName().asChar(), shape.partialPathName().asChar());
}

QString msg1, msg2;
getDialogMessages(
static_cast<int>(stages.size()), static_cast<int>(_anonLayerPairs.size()), msg1, msg2);
static_cast<int>(proxyShapes.length()),
static_cast<int>(_anonLayerPairs.size()),
msg1,
msg2);
buildDialog(msg1, msg2);
}
#endif
Expand All @@ -278,7 +279,7 @@ SaveLayersDialog::SaveLayersDialog(SessionState* in_sessionState, QWidget* in_pa
std::string stageName = stageEntry._displayName;
msg.format(StringResources::getAsMString(StringResources::kSaveName), stageName.c_str());
dialogTitle = MQtUtil::toQString(msg);
getLayersToSave(stageEntry._stage, stageName);
getLayersToSave(stageEntry._proxyShapePath, stageName);
}
setWindowTitle(dialogTitle);

Expand All @@ -289,11 +290,11 @@ SaveLayersDialog::SaveLayersDialog(SessionState* in_sessionState, QWidget* in_pa

SaveLayersDialog ::~SaveLayersDialog() { QApplication::restoreOverrideCursor(); }

void SaveLayersDialog::getLayersToSave(UsdStageRefPtr stage, const std::string& stageName)
void SaveLayersDialog::getLayersToSave(const std::string& proxyPath, const std::string& stageName)
{
// Get the layers to save for this stage.
MayaUsd::utils::stageLayersToSave stageLayersToSave;
MayaUsd::utils::getLayersToSaveFromProxy(stage, stageLayersToSave);
MayaUsd::utils::getLayersToSaveFromProxy(proxyPath, stageLayersToSave);

// Keep track of all the layers for this particular stage.
for (const auto& layerPairs : stageLayersToSave._anonLayers) {
Expand Down
6 changes: 3 additions & 3 deletions lib/usd/ui/layerEditor/saveLayersDialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ class SaveLayersDialog : public QDialog
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);
// Create dialog for bulk save using all provided proxy shapes and their owned stages.
SaveLayersDialog(QWidget* in_parent, const MDagPathArray& proxyShapes);
#endif

~SaveLayersDialog();
Expand All @@ -57,7 +57,7 @@ class SaveLayersDialog : public QDialog

private:
void buildDialog(const QString& msg1, const QString& msg2);
void getLayersToSave(UsdStageRefPtr stage, const std::string& stageName);
void getLayersToSave(const std::string& proxyPath, const std::string& stageName);

private:
typedef std::vector<std::pair<SdfLayerRefPtr, MayaUsd::utils::LayerParent>> layerPairs;
Expand Down