Skip to content

Fix layer editor path display and optimize rebuild model #4174

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

Open
adubois-spin wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Fix layer editor path display and optimize rebuild model #4174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5ce2c5
Fixing refresh perf problem and fixing layer editor path display to g…
adubois-spin Apr 1, 2025
8c117ba
Merge remote-tracking branch 'origin/dev' into fix_layer_editor_path_…
adubois-spin Apr 2, 2025
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
47 changes: 38 additions & 9 deletions lib/usd/ui/layerEditor/layerTreeModel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#include <mayaUsd/utils/utilSerialization.h>

#include <pxr/base/tf/notice.h>
#include <pxr/usd/ar/resolver.h>
#include <pxr/usd/ar/resolverContextBinder.h>

#include <maya/MGlobal.h>
#include <maya/MQtUtil.h>
Expand Down Expand Up @@ -300,13 +302,7 @@ void LayerTreeModel::rebuildModel(bool refreshLockState /*= false*/)
_lastAskedAnonLayerNameSinceRebuild = 0;

beginResetModel();

// Note: do *not* call clear() here! Unfortunately, clear() itself calls,
// beginResetModel() and endResetModel(). Qt does not detect the nested
// begin/end/ So calling clear() would make the layer manager flicker
// to be empty for a brief time.
if (rowCount() > 0)
removeRows(0, rowCount());
clear();

@pierrebai-adsk pierrebai-adsk Apr 7, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this change required? Like the comments said, clear() would cause multiple notifications of changes to be sent and cause flickers. Any reason why changing it to call clear?


if (_sessionState->isValid()) {
auto rootLayer = _sessionState->stage()->GetRootLayer();
Expand Down Expand Up @@ -349,6 +345,11 @@ void LayerTreeModel::rebuildModel(bool refreshLockState /*= false*/)
sharedStage,
&sharedLayers));
}
/*
* Here we need to create a resolver context to be able to update the target layer
*/
auto ctx = ArGetResolver().CreateDefaultContextForAsset(rootLayer->GetIdentifier());
const ArResolverContextBinder binder(ctx);

appendRow(new LayerTreeItem(
rootLayer, LayerType::RootLayer, "", &incomingLayers, sharedStage, &sharedLayers));
Expand Down Expand Up @@ -412,8 +413,36 @@ void LayerTreeModel::updateTargetLayer(InRebuildModel inRebuild)
void LayerTreeModel::usd_layerChanged(SdfNotice::LayersDidChangeSentPerLayer const& notice)
{
// experienced crashes in python prototype For now, rebuild everything
if (!_blockUsdNotices)
rebuildModelOnIdle();
if (!_blockUsdNotices) {
/*
* Here we filter the cases where we need a refresh as it takes time.
* We only refresh in the case the identifier, the resolved path,
* the content or a sublayer has been changed.
*/
bool rebuildModelRequired { false };
for (auto changePair : notice.GetChangeListVec()) {
const auto &entryList = changePair.second.GetEntryList();
for (const auto &entryPair : entryList) {

if (entryPair.second.flags.didChangeIdentifier ||
entryPair.second.flags.didChangeResolvedPath ||
entryPair.second.flags.didReplaceContent ||
entryPair.second.flags.didReloadContent ||
entryPair.second.subLayerChanges.size() > 0
) {
rebuildModelRequired = true;
break;
}
}
if (rebuildModelRequired) {
break;
}
}

if (rebuildModelRequired) {
rebuildModelOnIdle();
}
}
}

// notification from USD
Expand Down
56 changes: 45 additions & 11 deletions lib/usd/ui/layerEditor/mayaCommandHook.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ void MayaCommandHook::insertSubLayerPath(UsdLayer usdLayer, Path path, int index
cmd = "mayaUsdLayerEditor -edit -insertSubPath ";
cmd += std::to_string(index);
cmd += quote(path);
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);

@pierrebai-adsk pierrebai-adsk Apr 7, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For cases like this where the same pattern is ued multiple times, it is better to put it in a function to avoid the repetition. Maybe a function named getLayerPathForEditorCommand()

@adubois-spin adubois-spin Apr 14, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes I agree a function would be better.
We did not make one because we didn't know where in the package it would be best defined.

executeMel(cmd);
}

Expand All @@ -127,7 +130,10 @@ void MayaCommandHook::removeSubLayerPath(UsdLayer usdLayer, Path path)
cmd = "mayaUsdLayerEditor -edit -removeSubPath ";
cmd += std::to_string(index);
cmd += quote(proxyShapePath());
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
executeMel(cmd);
}

Expand All @@ -140,9 +146,15 @@ void MayaCommandHook::moveSubLayerPath(
std::string cmd;
cmd = "mayaUsdLayerEditor -edit -moveSubPath ";
cmd += quote(path);
cmd += quote(newParentUsdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string newParentUsdLayerPath{newParentUsdLayer->IsAnonymous() ? newParentUsdLayer->GetIdentifier() :
newParentUsdLayer->GetRealPath()};
cmd += quote(newParentUsdLayerPath);
cmd += std::to_string(index);
cmd += quote(oldParentUsdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string oldParentUsdLayerPath{oldParentUsdLayer->IsAnonymous() ? oldParentUsdLayer->GetIdentifier() :
oldParentUsdLayer->GetRealPath()};
cmd += quote(oldParentUsdLayerPath);
executeMel(cmd);
}

Expand All @@ -153,7 +165,10 @@ void MayaCommandHook::replaceSubLayerPath(UsdLayer usdLayer, Path oldPath, Path
cmd = "mayaUsdLayerEditor -edit -replaceSubPath ";
cmd += quote(oldPath);
cmd += quote(newPath);
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
executeMel(cmd);
}

Expand All @@ -162,7 +177,10 @@ void MayaCommandHook::discardEdits(UsdLayer usdLayer)
{
std::string cmd;
cmd = "mayaUsdLayerEditor -edit -discardEdits ";
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
executeMel(cmd);

refreshLayerSystemLock(usdLayer);
Expand All @@ -173,7 +191,10 @@ void MayaCommandHook::clearLayer(UsdLayer usdLayer)
{
std::string cmd;
cmd = "mayaUsdLayerEditor -edit -clear ";
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
executeMel(cmd);
}

Expand All @@ -183,8 +204,12 @@ UsdLayer MayaCommandHook::addAnonymousSubLayer(UsdLayer usdLayer, std::string ne
std::string cmd;
cmd = "mayaUsdLayerEditor -edit -addAnonymous ";
cmd += quote(newName);
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
std::string result = executeMel(cmd);

if (result.size() > 0)
return PXR_NS::SdfLayer::FindOrOpen(result);
else
Expand All @@ -198,7 +223,10 @@ void MayaCommandHook::muteSubLayer(UsdLayer usdLayer, bool muteIt)
cmd = "mayaUsdLayerEditor -edit -muteLayer ";
cmd += muteIt ? "1" : "0";
cmd += quote(proxyShapePath());
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
executeMel(cmd);
}

Expand All @@ -218,7 +246,10 @@ void MayaCommandHook::lockLayer(
cmd += std::to_string(lockState);
cmd += includeSubLayers ? " 1" : " 0";
cmd += quote(proxyShapePath());
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
executeMel(cmd);
}

Expand All @@ -229,7 +260,10 @@ void MayaCommandHook::refreshLayerSystemLock(UsdLayer usdLayer, bool refreshSubL
cmd += quote(proxyShapePath());
cmd += " ";
cmd += std::to_string(refreshSubLayers);
cmd += quote(usdLayer->GetIdentifier());
// Here we choose the identifier only if the layer is anonymous
std::string layerPath{usdLayer->IsAnonymous() ? usdLayer->GetIdentifier() :
usdLayer->GetRealPath()};
cmd += quote(layerPath);
executeMel(cmd);
}

Expand Down