From d5a3815d48b886018d7b8443178a20339677433f Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Mon, 18 Mar 2024 14:40:02 -0400 Subject: [PATCH 1/3] EMSUSD-1098 Fixed saving vs locked layers Also EMSUSD-1097, EMSUSD-1099 and EMSUSD-1100 EMSUSD-1097 Refactor anonymous layer saving. There was code that was nearly identical between the Layer Editor item saveAnonymousLayer function, the saveSubLayer function and the generic saveAnonymousLayer function in utilSerialization.h. Refactor the layer editor item to use the generic one instead, avoiding having to fix problems in two places. - Remove the saveSubLayer function that is now entirely redundant. - Refactor the LayerTreeItem function to use the generic saveAnonymousLayer function. - All the code left in the LayerTreeItem is now specific the the layer editor or its session state. - Add a FileBackup helper class in the utilFileSystem to backup a file and possibly restore it if the new file is not commited as valid. - Add a parameter to the generic saveAnonymousLayer function to return an optional error message. - Improve error messages when saving - Prevent saving system-locked layers. - Add helper functions to transfer the muted and locked status of anonymous layers to their new layers. EMSUSD-1100 preserve the locked and muted status of layer when saved. When transitioning from anonymous to non-anonymous, update the muted and locked status of the layers. EMSUSD-1098 don't allow saving system-locked layer Prevent saving when double-clicking on a system-locked layer. EMSUSD-1099 fix parent layer when converting anonynous layers. The parent layer that was passed to saveAnonymousLayer when converting anonymous layers into saved layers was incorrect. It was passing the parent of the parent instead. This lead to the stage root layer getting replaced by its oen sub-layer! Add unit tests and Python wrappers - Add Python wrappers to lock, unlock and verify lock on layers. - Write unit tests to verify the behavior of locked layers. - Improve the tests for muted layers. --- lib/mayaUsd/nodes/layerManager.cpp | 22 +- lib/mayaUsd/python/CMakeLists.txt | 1 + lib/mayaUsd/python/module.cpp | 1 + lib/mayaUsd/python/wrapLayerLocking.cpp | 65 ++++++ lib/mayaUsd/utils/layerLocking.h | 3 +- lib/mayaUsd/utils/utilFileSystem.cpp | 53 +++++ lib/mayaUsd/utils/utilFileSystem.h | 24 +++ lib/mayaUsd/utils/utilSerialization.cpp | 126 ++++++++++-- lib/mayaUsd/utils/utilSerialization.h | 6 +- lib/usd/ui/layerEditor/layerTreeItem.cpp | 99 ++++----- lib/usd/ui/layerEditor/layerTreeView.cpp | 18 +- lib/usd/ui/layerEditor/pathChecker.cpp | 52 ----- lib/usd/ui/layerEditor/pathChecker.h | 14 -- test/lib/mayaUsd/fileio/CMakeLists.txt | 1 + .../mayaUsd/fileio/testSaveLockedAnonLayer.py | 188 ++++++++++++++++++ .../mayaUsd/fileio/testSaveMutedAnonLayer.py | 23 ++- 16 files changed, 517 insertions(+), 179 deletions(-) create mode 100644 lib/mayaUsd/python/wrapLayerLocking.cpp create mode 100644 test/lib/mayaUsd/fileio/testSaveLockedAnonLayer.py diff --git a/lib/mayaUsd/nodes/layerManager.cpp b/lib/mayaUsd/nodes/layerManager.cpp index 97dee6f2cf..9e15a93107 100644 --- a/lib/mayaUsd/nodes/layerManager.cpp +++ b/lib/mayaUsd/nodes/layerManager.cpp @@ -159,27 +159,19 @@ void convertAnonymousLayersRecursive( { auto currentTarget = stage->GetEditTarget().GetLayer(); - MayaUsd::utils::LayerParent parentPtr; - if (stage->GetRootLayer() == layer) { - parentPtr._layerParent = nullptr; - parentPtr._proxyPath = basename; - } else if (stage->GetSessionLayer() == layer) { - parentPtr._layerParent = nullptr; - parentPtr._proxyPath = basename; - } else { - parentPtr._layerParent = layer; - parentPtr._proxyPath = basename; - } - std::vector sublayers = layer->GetSubLayerPaths(); for (size_t i = 0, n = sublayers.size(); i < n; ++i) { - auto subL = layer->Find(sublayers[i]); + SdfLayerRefPtr subL = layer->Find(sublayers[i]); if (subL) { convertAnonymousLayersRecursive(subL, basename, stage); if (subL->IsAnonymous()) { - auto newLayer - = MayaUsd::utils::saveAnonymousLayer(stage, subL, parentPtr, basename); + MayaUsd::utils::LayerParent subLayerParent; + subLayerParent._layerParent = layer; + subLayerParent._proxyPath = basename; + + SdfLayerRefPtr newLayer + = MayaUsd::utils::saveAnonymousLayer(stage, subL, subLayerParent, basename); if (subL == currentTarget) { stage->SetEditTarget(newLayer); } diff --git a/lib/mayaUsd/python/CMakeLists.txt b/lib/mayaUsd/python/CMakeLists.txt index c67ad7b5a3..45b31818c8 100644 --- a/lib/mayaUsd/python/CMakeLists.txt +++ b/lib/mayaUsd/python/CMakeLists.txt @@ -33,6 +33,7 @@ target_sources(${PYTHON_TARGET_NAME} wrapConverter.cpp wrapCopyLayerPrims.cpp wrapDiagnosticDelegate.cpp + wrapLayerLocking.cpp wrapLoadRules.cpp wrapMeshWriteUtils.cpp wrapOpUndoItem.cpp diff --git a/lib/mayaUsd/python/module.cpp b/lib/mayaUsd/python/module.cpp index 59cbad6f7d..caf83785bb 100644 --- a/lib/mayaUsd/python/module.cpp +++ b/lib/mayaUsd/python/module.cpp @@ -31,6 +31,7 @@ TF_WRAP_MODULE TF_WRAP(ConverterArgs); TF_WRAP(CopyLayerPrims); TF_WRAP(DiagnosticDelegate); + TF_WRAP(LayerLocking); TF_WRAP(LoadRules); TF_WRAP(MeshWriteUtils); #ifdef UFE_V3_FEATURES_AVAILABLE diff --git a/lib/mayaUsd/python/wrapLayerLocking.cpp b/lib/mayaUsd/python/wrapLayerLocking.cpp new file mode 100644 index 0000000000..1505a37546 --- /dev/null +++ b/lib/mayaUsd/python/wrapLayerLocking.cpp @@ -0,0 +1,65 @@ +// +// Copyright 2023 Autodesk +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +#include + +#include + +#include + +using namespace boost::python; + +namespace { + +void _lockLayer(const std::string& shapeName, PXR_NS::SdfLayer& layer) +{ + PXR_NS::SdfLayerRefPtr layerPtr(&layer); + MayaUsd::lockLayer(shapeName, layerPtr, MayaUsd::LayerLock_Locked); +} + +void _systemLockLayer(const std::string& shapeName, PXR_NS::SdfLayer& layer) +{ + PXR_NS::SdfLayerRefPtr layerPtr(&layer); + MayaUsd::lockLayer(shapeName, layerPtr, MayaUsd::LayerLock_SystemLocked); +} + +void _unlockLayer(const std::string& shapeName, PXR_NS::SdfLayer& layer) +{ + PXR_NS::SdfLayerRefPtr layerPtr(&layer); + MayaUsd::lockLayer(shapeName, layerPtr, MayaUsd::LayerLock_Unlocked); +} + +bool _isLayerLocked(PXR_NS::SdfLayer& layer) +{ + PXR_NS::SdfLayerRefPtr layerPtr(&layer); + return MayaUsd::isLayerLocked(layerPtr); +} + +bool _isLayerSystemLocked(PXR_NS::SdfLayer& layer) +{ + PXR_NS::SdfLayerRefPtr layerPtr(&layer); + return MayaUsd::isLayerSystemLocked(layerPtr); +} + +} // namespace + +void wrapLayerLocking() +{ + def("lockLayer", _lockLayer); + def("systemLockLayer", _systemLockLayer); + def("unlockLayer", _unlockLayer); + def("isLayerLocked", _isLayerLocked); + def("isLayerSystemLocked", _isLayerSystemLocked); +} diff --git a/lib/mayaUsd/utils/layerLocking.h b/lib/mayaUsd/utils/layerLocking.h index 10bea2c091..debaae25cd 100644 --- a/lib/mayaUsd/utils/layerLocking.h +++ b/lib/mayaUsd/utils/layerLocking.h @@ -97,7 +97,8 @@ enum LayerLockType LayerLock_SystemLocked }; -/*! \brief Sets the lock status on a layer +/*! \brief Sets the lock status on a layer. + * Automatically calls addLockedLayer, addSystemLockedLayer or their remove counterparts. */ MAYAUSD_CORE_PUBLIC void lockLayer( diff --git a/lib/mayaUsd/utils/utilFileSystem.cpp b/lib/mayaUsd/utils/utilFileSystem.cpp index c6c1fefe61..7a0b2447f6 100644 --- a/lib/mayaUsd/utils/utilFileSystem.cpp +++ b/lib/mayaUsd/utils/utilFileSystem.cpp @@ -750,3 +750,56 @@ std::string UsdMayaUtilFileSystem::pathFindExtension(std::string& filePath) ghc::filesystem::path ext = p.extension(); return ext.string(); } + +UsdMayaUtilFileSystem::FileBackup::FileBackup(const std::string& filename) + : _filename(filename) +{ + backup(); +} + +UsdMayaUtilFileSystem::FileBackup::~FileBackup() +{ + // If commited, we don't restore the old file. + if (_commited) + return; + + try { + restore(); + } catch (...) { + // Don't allow exceptions out of a destructor. + } +} + +std::string UsdMayaUtilFileSystem::FileBackup::getBackupFilename() const +{ + return _filename + ".backup"; +} + +void UsdMayaUtilFileSystem::FileBackup::backup() +{ + if (!ghc::filesystem::exists(ghc::filesystem::path(_filename))) + return; + + const std::string backupFileName = getBackupFilename(); + remove(backupFileName.c_str()); + if (rename(_filename.c_str(), backupFileName.c_str()) != 0) + return; + + _backed = true; +} + +void UsdMayaUtilFileSystem::FileBackup::commit() +{ + // Once commited, the backup will not be put back into the original file. + _commited = true; +} + +void UsdMayaUtilFileSystem::FileBackup::restore() +{ + if (!_backed) + return; + + remove(_filename.c_str()); + const std::string backupFileName = getBackupFilename(); + rename(backupFileName.c_str(), _filename.c_str()); +} diff --git a/lib/mayaUsd/utils/utilFileSystem.h b/lib/mayaUsd/utils/utilFileSystem.h index d7efe0475a..726027aa86 100644 --- a/lib/mayaUsd/utils/utilFileSystem.h +++ b/lib/mayaUsd/utils/utilFileSystem.h @@ -308,6 +308,30 @@ void pathRemoveExtension(std::string& filePath); MAYAUSD_CORE_PUBLIC std::string pathFindExtension(std::string& filePath); +// Backup a file and restore it if not commited. +class FileBackup +{ +public: + FileBackup(const std::string& filename); + ~FileBackup(); + + // Once commited, the backup will not be put back into the original file. + void commit(); + + // Force restoration of the original file if successfully backed-up, even if commited. + void restore(); + + // Return the backup file name. + std::string getBackupFilename() const; + +public: + void backup(); + + std::string _filename; + bool _backed = false; + bool _commited = false; +}; + } // namespace UsdMayaUtilFileSystem PXR_NAMESPACE_CLOSE_SCOPE diff --git a/lib/mayaUsd/utils/utilSerialization.cpp b/lib/mayaUsd/utils/utilSerialization.cpp index 2aceae0baa..3a66274bbf 100644 --- a/lib/mayaUsd/utils/utilSerialization.cpp +++ b/lib/mayaUsd/utils/utilSerialization.cpp @@ -17,6 +17,8 @@ #include #include +#include +#include #include #include #include @@ -32,6 +34,8 @@ #include #include +#include + #include namespace { @@ -92,6 +96,41 @@ void populateChildren( recursionDetector->pop(); } +void updateMutedLayers( + const UsdStageRefPtr& stage, + const SdfLayerRefPtr& oldLayer, + const SdfLayerRefPtr& newLayer) +{ + if (!stage) + return; + if (!oldLayer) + return; + if (!newLayer) + return; + + if (stage->IsLayerMuted(oldLayer->GetIdentifier())) { + MayaUsd::addMutedLayer(newLayer); + stage->MuteLayer(newLayer->GetIdentifier()); + } +} + +void updateLockedLayers( + const std::string& proxyPath, + const SdfLayerRefPtr& oldLayer, + const SdfLayerRefPtr& newLayer) +{ + if (!oldLayer) + return; + if (!newLayer) + return; + + if (MayaUsd::isLayerSystemLocked(oldLayer)) { + MayaUsd::lockLayer(proxyPath, newLayer, MayaUsd::LayerLock_SystemLocked); + } else if (MayaUsd::isLayerLocked(oldLayer)) { + MayaUsd::lockLayer(proxyPath, newLayer, MayaUsd::LayerLock_Locked); + } +} + void updateTargetLayer(const std::string& proxyNodeName, const SdfLayerRefPtr& layer) { if (MayaUsdProxyShapeBase* proxyShape = UsdMayaUtil::GetProxyShapeByProxyName(proxyNodeName)) { @@ -321,11 +360,26 @@ SdfLayerRefPtr saveAnonymousLayer( SdfLayerRefPtr anonLayer, LayerParent parent, const std::string& basename, - std::string formatArg) + std::string formatArg, + std::string* errorMsg) { PathInfo pathInfo; pathInfo.absolutePath = generateUniqueLayerFileName(basename, anonLayer); - return saveAnonymousLayer(stage, anonLayer, pathInfo, parent, formatArg); + return saveAnonymousLayer(stage, anonLayer, pathInfo, parent, formatArg, errorMsg); +} + +static void formatErrorMsg( + const char* message, + const SdfLayerRefPtr& anonLayer, + const std::string absPath, + std::string* errorMsg) +{ + if (!errorMsg) + return; + + MString text; + text.format(message, anonLayer->GetDisplayName().c_str(), absPath.c_str()); + *errorMsg = text.asChar(); } SdfLayerRefPtr saveAnonymousLayer( @@ -333,40 +387,60 @@ SdfLayerRefPtr saveAnonymousLayer( SdfLayerRefPtr anonLayer, const PathInfo& pathInfo, LayerParent parent, - std::string formatArg) + std::string formatArg, + std::string* errorMsg) { - // TODO: the code below is very similar to LayerTreeItem::saveAnonymousLayer(). - // When fixing bug here or there, we need to fix it in the other. Refactor to have a - // single copy. + UsdMayaUtilFileSystem::FileBackup backup(pathInfo.absolutePath); + std::string filePath(pathInfo.absolutePath); + + if (!anonLayer) { + formatErrorMsg("No layer provided to save to \"^2s\"", anonLayer, filePath, errorMsg); + return nullptr; + } + + if (!anonLayer->IsAnonymous()) { + formatErrorMsg( + "Cannot save non-anonymous layer \"^1\" under a different file name", + anonLayer, + filePath, + errorMsg); + return nullptr; + } - if (!anonLayer || !anonLayer->IsAnonymous()) { + if (isLayerSystemLocked(anonLayer)) { + formatErrorMsg( + "Cannot save layer \"^1\" when system-locked", anonLayer, filePath, errorMsg); return nullptr; } - std::string filePath(pathInfo.absolutePath); ensureUSDFileExtension(filePath); const bool wasTargetLayer = (stage->GetEditTarget().GetLayer() == anonLayer); if (!saveLayerWithFormat(anonLayer, filePath, formatArg)) { + formatErrorMsg("Failed to save layer \"^1\" to \"^2s\"", anonLayer, filePath, errorMsg); return nullptr; } - const bool isSubLayer = (parent._layerParent != nullptr); - std::string relativePathAnchor; + auto parentLayer = parent._layerParent; + const bool isSubLayer = (parentLayer != nullptr); if (pathInfo.savePathAsRelative) { if (!pathInfo.customRelativeAnchor.empty()) { - relativePathAnchor = pathInfo.customRelativeAnchor; + std::string relativePathAnchor = pathInfo.customRelativeAnchor; filePath = UsdMayaUtilFileSystem::makePathRelativeTo(filePath, relativePathAnchor).first; } else if (isSubLayer) { - filePath - = UsdMayaUtilFileSystem::getPathRelativeToLayerFile(filePath, parent._layerParent); - relativePathAnchor = UsdMayaUtilFileSystem::getLayerFileDir(parent._layerParent); + filePath = UsdMayaUtilFileSystem::getPathRelativeToLayerFile(filePath, parentLayer); + if (ghc::filesystem::path(filePath).is_absolute()) { + UsdMayaUtilFileSystem::markPathAsPostponedRelative(parentLayer, filePath); + } } else { filePath = UsdMayaUtilFileSystem::getPathRelativeToMayaSceneFile(filePath); - relativePathAnchor = UsdMayaUtilFileSystem::getMayaSceneFileDir(); + } + } else { + if (isSubLayer) { + UsdMayaUtilFileSystem::unmarkPathAsPostponedRelative(parentLayer, filePath); } } @@ -378,16 +452,24 @@ SdfLayerRefPtr saveAnonymousLayer( // after saving a layer with a relative path. SdfLayerRefPtr newLayer = SdfLayer::FindOrOpen(pathInfo.absolutePath); + if (!newLayer) { + formatErrorMsg("Failed to reload layer \"^1\" from \"^2\"", anonLayer, filePath, errorMsg); + return nullptr; + } + // Now replace the layer in the parent, using a relative path if requested. - if (newLayer) { - if (isSubLayer) { - updateSubLayer(parent._layerParent, anonLayer, filePath); - updateTargetLayer(parent._proxyPath, newLayer); - } else if (!parent._proxyPath.empty()) { - updateRootLayer(parent._proxyPath, filePath, newLayer, wasTargetLayer); - } + if (isSubLayer) { + updateSubLayer(parentLayer, anonLayer, filePath); + } else if (!parent._proxyPath.empty()) { + updateRootLayer(parent._proxyPath, filePath, newLayer, wasTargetLayer); } + updateTargetLayer(parent._proxyPath, newLayer); + updateMutedLayers(stage, anonLayer, newLayer); + updateLockedLayers(parent._proxyPath, anonLayer, newLayer); + + backup.commit(); + return newLayer; } diff --git a/lib/mayaUsd/utils/utilSerialization.h b/lib/mayaUsd/utils/utilSerialization.h index d816266510..d0d3f06b16 100644 --- a/lib/mayaUsd/utils/utilSerialization.h +++ b/lib/mayaUsd/utils/utilSerialization.h @@ -130,7 +130,8 @@ PXR_NS::SdfLayerRefPtr saveAnonymousLayer( PXR_NS::SdfLayerRefPtr anonLayer, LayerParent parent, const std::string& basename, - std::string formatArg = ""); + std::string formatArg = "", + std::string* errorMsg = nullptr); /*! \brief Save an anonymous layer to disk and update the sublayer path array in the parent layer. @@ -141,7 +142,8 @@ PXR_NS::SdfLayerRefPtr saveAnonymousLayer( PXR_NS::SdfLayerRefPtr anonLayer, const PathInfo& pathInfo, LayerParent parent, - std::string formatArg = ""); + std::string formatArg = "", + std::string* errorMsg = nullptr); /*! \brief Update the list of sub-layers with a new layer identity. * The new sub-layer is identified by its path explicitly, diff --git a/lib/usd/ui/layerEditor/layerTreeItem.cpp b/lib/usd/ui/layerEditor/layerTreeItem.cpp index 521802dbbc..3cedb42b74 100644 --- a/lib/usd/ui/layerEditor/layerTreeItem.cpp +++ b/lib/usd/ui/layerEditor/layerTreeItem.cpp @@ -436,77 +436,50 @@ void LayerTreeItem::saveEditsNoPrompt() // helper to save anon layers called by saveEdits() void LayerTreeItem::saveAnonymousLayer() { - // TODO: the code below is very similar to mayaUsd::utils::saveAnonymousLayer(). - // When fixing bug here or there, we need to fix it in the other. Refactor to have a - // single copy. - - auto sessionState = parentModel()->sessionState(); + SessionState* sessionState = parentModel()->sessionState(); + // the path we have is an absolute path std::string fileName; - if (sessionState->saveLayerUI(nullptr, &fileName, parentLayer())) { - - MayaUsd::utils::ensureUSDFileExtension(fileName); + if (!sessionState->saveLayerUI(nullptr, &fileName, parentLayer())) + return; - // the path we have is an absolute path - const QString dialogTitle = StringResources::getAsQString(StringResources::kSaveLayer); - std::string formatTag = MayaUsd::utils::usdFormatArgOption(); - if (saveSubLayer(dialogTitle, parentLayerItem(), layer(), fileName, formatTag)) { + MayaUsd::utils::ensureUSDFileExtension(fileName); - const std::string absoluteFileName = fileName; + const QString dialogTitle = StringResources::getAsQString(StringResources::kSaveLayer); - // now replace the layer in the parent - if (isRootLayer()) { - if (UsdMayaUtilFileSystem::requireUsdPathsRelativeToMayaSceneFile()) { - fileName = UsdMayaUtilFileSystem::getPathRelativeToMayaSceneFile(fileName); - } - sessionState->rootLayerPathChanged(fileName); - } else { - auto parentItem = parentLayerItem(); - auto parentLayer = parentItem ? parentItem->layer() : nullptr; - if (parentLayer) { - if (UsdMayaUtilFileSystem::requireUsdPathsRelativeToParentLayer()) { - if (!parentLayer->IsAnonymous()) { - fileName = UsdMayaUtilFileSystem::getPathRelativeToLayerFile( - fileName, parentLayer); - } else { - UsdMayaUtilFileSystem::markPathAsPostponedRelative( - parentLayer, fileName); - } - } else { - UsdMayaUtilFileSystem::unmarkPathAsPostponedRelative(parentLayer, fileName); - } - } + if (!checkIfPathIsSafeToAdd(dialogTitle, parentLayerItem(), fileName)) + return; - // Note: we need to open the layer with the absolute path. The relative path is only - // used by the parent layer to refer to the sub-layer relative to itself. When - // opening the layer in isolation, we need to use the absolute path. Failure - // to do so will make finding the layer by its own identifier fail! A symptom - // of this failure is that drag-and-drop in the Layer Manager UI fails - // immediately after saving a layer with a relative path. - SdfLayerRefPtr newLayer = SdfLayer::FindOrOpen(absoluteFileName); - - // Now replace the layer in the parent, using a relative path if requested. - if (newLayer) { - bool setTarget = _isTargetLayer; - auto model = parentModel(); - MayaUsd::utils::updateSubLayer(parentItem->layer(), layer(), fileName); - if (setTarget) { - auto stage = sessionState->stage(); - if (stage) { - stage->SetEditTarget(newLayer); - } - } - model->selectUsdLayerOnIdle(newLayer); - } else { - QMessageBox::critical( - nullptr, - dialogTitle, - StringResources::getAsQString(StringResources::kErrorFailedToReloadLayer)); - } - } - } + MayaUsd::utils::PathInfo pathInfo; + pathInfo.absolutePath = fileName; + pathInfo.savePathAsRelative = isRootLayer() + ? UsdMayaUtilFileSystem::requireUsdPathsRelativeToMayaSceneFile() + : UsdMayaUtilFileSystem::requireUsdPathsRelativeToParentLayer(); + pathInfo.customRelativeAnchor = ""; // TODO, see calculateParentLayerDir() + + MayaUsd::utils::LayerParent layerParent; + layerParent._layerParent = parentLayer(); + layerParent._proxyPath = sessionState->stageEntry()._proxyShapePath; + + std::string errMsg; + std::string formatTag = MayaUsd::utils::usdFormatArgOption(); + SdfLayerRefPtr newLayer = MayaUsd::utils::saveAnonymousLayer( + sessionState->stage(), layer(), pathInfo, layerParent, formatTag, &errMsg); + if (!newLayer) { + warningDialog(dialogTitle, errMsg.c_str()); + return; } + + const std::string absoluteFileName = fileName; + + // now replace the layer in the parent + if (isRootLayer()) + sessionState->rootLayerPathChanged(fileName); + + if (auto model = parentModel()) + model->selectUsdLayerOnIdle(newLayer); } + void LayerTreeItem::discardEdits() { if (isAnonymous() || !isDirty()) { diff --git a/lib/usd/ui/layerEditor/layerTreeView.cpp b/lib/usd/ui/layerEditor/layerTreeView.cpp index d8380caa58..1639b9ab5f 100644 --- a/lib/usd/ui/layerEditor/layerTreeView.cpp +++ b/lib/usd/ui/layerEditor/layerTreeView.cpp @@ -143,12 +143,18 @@ void LayerTreeView::selectLayerRquest(const QModelIndex& index) void LayerTreeView::onItemDoubleClicked(const QModelIndex& index) { - if (index.isValid()) { - auto layerTreeItem = layerItemFromIndex(index); - if (layerTreeItem->needsSaving()) { - layerTreeItem->saveEdits(); - } - } + if (!index.isValid()) + return; + + auto layerTreeItem = layerItemFromIndex(index); + if (!layerTreeItem->needsSaving()) + return; + + // Note: system-locked layers cannot be saved. + if (layerTreeItem->isSystemLocked() || layerTreeItem->appearsSystemLocked()) + return; + + layerTreeItem->saveEdits(); } bool LayerTreeView::shouldExpandOrCollapseAll() const diff --git a/lib/usd/ui/layerEditor/pathChecker.cpp b/lib/usd/ui/layerEditor/pathChecker.cpp index 65c6321a55..99ec5775a4 100644 --- a/lib/usd/ui/layerEditor/pathChecker.cpp +++ b/lib/usd/ui/layerEditor/pathChecker.cpp @@ -162,56 +162,4 @@ bool checkIfPathIsSafeToAdd( return false; } -bool saveSubLayer( - const QString& in_errorTitle, - LayerTreeItem* in_parentItem, - PXR_NS::SdfLayerRefPtr in_layer, - const std::string& in_absolutePath, - const std::string& in_formatTag) -{ - std::string backupFileName; - QFileInfo fileInfo(QString::fromStdString(in_absolutePath)); - bool fileError = false; - bool success = false; - if (fileInfo.exists()) { - // backup existing file - backupFileName = in_absolutePath + ".backup"; - remove(backupFileName.c_str()); - if (rename(in_absolutePath.c_str(), backupFileName.c_str()) != 0) { - fileError = true; - } - } - - if (!fileError) { - if (!MayaUsd::utils::saveLayerWithFormat(in_layer, in_absolutePath, in_formatTag)) { - fileError = true; - } else { - // parent item is null if we're saving the root later - if (in_parentItem != nullptr) { - success = checkIfPathIsSafeToAdd(in_errorTitle, in_parentItem, in_absolutePath); - } else { - success = true; - } - } - } - - // place back the file on failure - if (!success && !backupFileName.empty()) { - remove(in_absolutePath.c_str()); - rename(backupFileName.c_str(), in_absolutePath.c_str()); - } - - if (fileError) { - MString msg; - msg.format( - StringResources::getAsMString(StringResources::kErrorFailedToSaveFile), - in_absolutePath.c_str()); - - warningDialog(in_errorTitle, MQtUtil::toQString(msg)); - return false; - } - - return success; -} - } // namespace UsdLayerEditor diff --git a/lib/usd/ui/layerEditor/pathChecker.h b/lib/usd/ui/layerEditor/pathChecker.h index 2461e144f4..716cde6263 100644 --- a/lib/usd/ui/layerEditor/pathChecker.h +++ b/lib/usd/ui/layerEditor/pathChecker.h @@ -37,20 +37,6 @@ bool checkIfPathIsSafeToAdd( LayerTreeItem* in_parentItem, const std::string& in_pathToAdd); -// check if it's safe to save an anon sublayer to the given path -// and then does it. -// stategy: -// save the layer, then use the same logic as loadLayers to -// see if it actually can add this path without creating a recursion -// if that fails, delete the file we created. -// for now, assumes an absolute input path -bool saveSubLayer( - const QString& in_errorTitle, - LayerTreeItem* in_parentItem, - PXR_NS::SdfLayerRefPtr in_layer, - const std::string& in_absolutePath, - const std::string& in_formatTag); - } // namespace UsdLayerEditor #endif // PATHCHECKER_H diff --git a/test/lib/mayaUsd/fileio/CMakeLists.txt b/test/lib/mayaUsd/fileio/CMakeLists.txt index 0746ba37f1..c7c39eaaff 100644 --- a/test/lib/mayaUsd/fileio/CMakeLists.txt +++ b/test/lib/mayaUsd/fileio/CMakeLists.txt @@ -9,6 +9,7 @@ set(TEST_SCRIPT_FILES testJobContextRegistry.py testDisplayLayerSaveRestore.py testSaveMutedAnonLayer.py + testSaveLockedAnonLayer.py # Once of the tests in this file requires UsdMaya (from the Pixar plugin). That test # will be skipped if not found (probably because BUILD_PXR_PLUGIN is off). diff --git a/test/lib/mayaUsd/fileio/testSaveLockedAnonLayer.py b/test/lib/mayaUsd/fileio/testSaveLockedAnonLayer.py new file mode 100644 index 0000000000..66e4eb0f18 --- /dev/null +++ b/test/lib/mayaUsd/fileio/testSaveLockedAnonLayer.py @@ -0,0 +1,188 @@ +#!/usr/bin/env python + +# +# Copyright 2024 Autodesk +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import fixturesUtils + +from maya import cmds +from maya import standalone + +import mayaUtils +import mayaUsd +import mayaUsd_createStageWithNewLayer + +import usdUtils + +from pxr import Usd, Sdf + +import unittest + +class SaveLockedLayerTest(unittest.TestCase): + ''' + Test saving locked layers and reloading the scene to verify they are not lost. + ''' + + pluginsLoaded = False + + @classmethod + def setUpClass(cls): + fixturesUtils.readOnlySetUpClass(__file__, loadPlugin=False) + + if not cls.pluginsLoaded: + cls.pluginsLoaded = mayaUtils.isMayaUsdPluginLoaded() + + @classmethod + def tearDownClass(cls): + standalone.uninitialize() + + def setUp(self): + cmds.file(new=True, force=True) + + def testSaveLockedAnonLayerInUSD(self): + # Save the file. Make sure the USD edits will go to a USD file. + self._runTestSaveLockedAnonLayer(1) + + def testSaveLockedAnonLayerInMaya(self): + # Save the file. Make sure the USD edits will go to the Maya file. + self._runTestSaveLockedAnonLayer(2) + + def _runTestSaveLockedAnonLayer(self, saveLocation): + ''' + The goal is to create an anonymous sub-layer, lock it and verify the locking + is not lost when reloaded. + ''' + # Create a stage with a sub-layer. + psPathStr = mayaUsd_createStageWithNewLayer.createStageWithNewLayer() + stage = mayaUsd.lib.GetPrim(psPathStr).GetStage() + rootLayer = stage.GetRootLayer() + subLayer = usdUtils.addNewLayerToLayer(rootLayer, anonymous=True) + + # Add a prim in the root and in the sub-layer. + with Usd.EditContext(stage, Usd.EditTarget(rootLayer)): + stage.DefinePrim('/InRoot', 'Sphere') + + with Usd.EditContext(stage, Usd.EditTarget(subLayer)): + stage.DefinePrim('/InSub', 'Cube') + + def verifyPrims(stage): + inRoot = stage.GetPrimAtPath("/InRoot") + self.assertTrue(inRoot) + inRoot = None + inSub = stage.GetPrimAtPath("/InSub") + self.assertTrue(inSub) + inSub = None + + verifyPrims(stage) + + # Lock the sub-layer. + mayaUsd.lib.lockLayer(psPathStr, subLayer) + + # Save the file. Make sure the edit will go where requested by saveLocation. + tempMayaFile = 'saveLockedAnonLayer.ma' + cmds.optionVar(intValue=('mayaUsd_SerializedUsdEditsLocation', saveLocation)) + cmds.file(rename=tempMayaFile) + cmds.file(save=True, force=True, type='mayaAscii') + + # Clear and reopen the file. + stage = None + rootLayer = None + subLayer = None + cmds.file(new=True, force=True) + cmds.file(tempMayaFile, open=True) + + # Retrieve the stage, root and sub-layer. + stage = mayaUsd.lib.GetPrim(psPathStr).GetStage() + rootLayer = stage.GetRootLayer() + self.assertEqual(len(rootLayer.subLayerPaths), 1) + subLayerPath = rootLayer.subLayerPaths[0] + self.assertIsNotNone(subLayerPath) + self.assertTrue(subLayerPath) + subLayer = Sdf.Layer.FindOrOpen(subLayerPath) + self.assertIsNotNone(subLayer) + + # Verify the layer was reloaded as locked. + self.assertTrue(mayaUsd.lib.isLayerLocked(subLayer)) + + # Verify the two objects are still present. + verifyPrims(stage) + + def testSaveSystemLockedAnonLayerInUSD(self): + # Save the file. Make sure the USD edits will go to a USD file. + self._runTestSaveLockedAnonLayer(1) + + def testSaveSystemLockedAnonLayerInMaya(self): + # Save the file. Make sure the USD edits will go to the Maya file. + self._runTestSaveLockedAnonLayer(2) + + def _runTestSaveSystemLockedAnonLayer(self, saveLocation): + ''' + The goal is to create an anonymous sub-layer, system-lock it and + verify the layer and its locking is not lost when saved. + + System-locked layer cannot be saved, so don't try to reload the scene. + There used to be a bug where the layer would be lost. + ''' + # Create a stage with a sub-layer. + psPathStr = mayaUsd_createStageWithNewLayer.createStageWithNewLayer() + stage = mayaUsd.lib.GetPrim(psPathStr).GetStage() + rootLayer = stage.GetRootLayer() + subLayer = usdUtils.addNewLayerToLayer(rootLayer, anonymous=True) + + # Add a prim in the root and in the sub-layer. + with Usd.EditContext(stage, Usd.EditTarget(rootLayer)): + stage.DefinePrim('/InRoot', 'Sphere') + + with Usd.EditContext(stage, Usd.EditTarget(subLayer)): + stage.DefinePrim('/InSub', 'Cube') + + def verifyPrims(stage): + inRoot = stage.GetPrimAtPath("/InRoot") + self.assertTrue(inRoot) + inRoot = None + inSub = stage.GetPrimAtPath("/InSub") + self.assertTrue(inSub) + inSub = None + + verifyPrims(stage) + + # System-lock the sub-layer. + mayaUsd.lib.systemLockLayer(psPathStr, subLayer) + + # Save the file. Make sure the edit will go where requested by saveLocation. + tempMayaFile = 'saveLockedAnonLayer.ma' + cmds.optionVar(intValue=('mayaUsd_SerializedUsdEditsLocation', saveLocation)) + cmds.file(rename=tempMayaFile) + cmds.file(save=True, force=True, type='mayaAscii') + + # Retrieve the stage, root and sub-layer again. + stage = mayaUsd.lib.GetPrim(psPathStr).GetStage() + rootLayer = stage.GetRootLayer() + self.assertEqual(len(rootLayer.subLayerPaths), 1) + subLayerPath = rootLayer.subLayerPaths[0] + self.assertIsNotNone(subLayerPath) + self.assertTrue(subLayerPath) + subLayer = Sdf.Layer.FindOrOpen(subLayerPath) + self.assertIsNotNone(subLayer) + + # Verify the layer was reloaded as locked. + self.assertTrue(mayaUsd.lib.isLayerSystemLocked(subLayer)) + + # Verify the two objects are still present. + verifyPrims(stage) + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py b/test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py index 6636599ba1..2e0586b37b 100644 --- a/test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py +++ b/test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py @@ -52,7 +52,19 @@ def tearDownClass(cls): def setUp(self): cmds.file(new=True, force=True) - def testSaveMutedAnonLayer(self): + def testSaveMutedAnonLayerInUSD(self): + # Save the file. Make sure the USD edits will go to a USD file. + self._runTestSaveMutedAnonLayer(1) + + def testSaveMutedAnonLayerInMaya(self): + # Save the file. Make sure the USD edits will go to the Maya file. + self._runTestSaveMutedAnonLayer(2) + + def _runTestSaveMutedAnonLayer(self, saveLocation): + ''' + The goal is to create an anonymous sub-layer, mute it and verify the muting + is not lost when reloaded. + ''' # Create a stage with a sub-layer. psPathStr = mayaUsd_createStageWithNewLayer.createStageWithNewLayer() stage = mayaUsd.lib.GetPrim(psPathStr).GetStage() @@ -79,9 +91,9 @@ def verifyPrims(stage): # Mute the sub-layer. stage.MuteLayer(subLayer.identifier) - # Save the file. Make sure the edit will go to the Maya file. + # Save the file. Make sure the edit will go where requested by saveLocation. tempMayaFile = 'saveMutedAnonLayer.ma' - cmds.optionVar(intValue=('mayaUsd_SerializedUsdEditsLocation', 2)) + cmds.optionVar(intValue=('mayaUsd_SerializedUsdEditsLocation', saveLocation)) cmds.file(rename=tempMayaFile) cmds.file(save=True, force=True, type='mayaAscii') @@ -99,7 +111,7 @@ def verifyPrims(stage): subLayerPath = rootLayer.subLayerPaths[0] self.assertIsNotNone(subLayerPath) self.assertTrue(subLayerPath) - subLayer = Sdf.Layer.Find(subLayerPath) + subLayer = Sdf.Layer.FindOrOpen(subLayerPath) self.assertIsNotNone(subLayer) # Verify the layer was reloaded as muted. @@ -108,3 +120,6 @@ def verifyPrims(stage): # Verify the two objects are still present. stage.UnmuteLayer(subLayer.identifier) verifyPrims(stage) + +if __name__ == '__main__': + unittest.main(verbosity=2) From a016759b1e486d5375f5af11cb32ab644364e2d5 Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Thu, 21 Mar 2024 09:55:43 -0400 Subject: [PATCH 2/3] EMSUSD-1098 use the layer identifier when asserting In older version of USD, the layer muting check can fail due to case change in the file path, even though on Windows the file paths are case-insensitive. --- test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py b/test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py index 2e0586b37b..c9d279b4f3 100644 --- a/test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py +++ b/test/lib/mayaUsd/fileio/testSaveMutedAnonLayer.py @@ -115,7 +115,7 @@ def verifyPrims(stage): self.assertIsNotNone(subLayer) # Verify the layer was reloaded as muted. - self.assertTrue(stage.IsLayerMuted(subLayerPath)) + self.assertTrue(stage.IsLayerMuted(subLayer.identifier)) # Verify the two objects are still present. stage.UnmuteLayer(subLayer.identifier) From 500814f1764baba3faea4a76af4b4e76af1b27c0 Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Thu, 21 Mar 2024 10:02:56 -0400 Subject: [PATCH 3/3] EMSUSD-1098 update copyright year --- lib/mayaUsd/python/wrapLayerLocking.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mayaUsd/python/wrapLayerLocking.cpp b/lib/mayaUsd/python/wrapLayerLocking.cpp index 1505a37546..f201dbc66c 100644 --- a/lib/mayaUsd/python/wrapLayerLocking.cpp +++ b/lib/mayaUsd/python/wrapLayerLocking.cpp @@ -1,5 +1,5 @@ // -// Copyright 2023 Autodesk +// Copyright 2024 Autodesk // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License.