From 120ae476d5dbf61f5f3cc0d898bc7639ebef854a Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Thu, 3 Dec 2020 07:59:47 -0500 Subject: [PATCH 1/4] Fix undo/redo crash when switching between layers. --- lib/mayaUsd/python/wrapUsdUndoManager.cpp | 2 +- lib/mayaUsd/ufe/StagesSubject.cpp | 4 ++-- lib/mayaUsd/undo/UsdUndoManager.cpp | 11 +++++++++-- lib/mayaUsd/undo/UsdUndoManager.h | 10 ++++++++-- lib/mayaUsd/undo/UsdUndoStateDelegate.cpp | 5 ----- lib/mayaUsd/undo/UsdUndoStateDelegate.h | 2 -- 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/mayaUsd/python/wrapUsdUndoManager.cpp b/lib/mayaUsd/python/wrapUsdUndoManager.cpp index 0ae547c636..89de999c25 100644 --- a/lib/mayaUsd/python/wrapUsdUndoManager.cpp +++ b/lib/mayaUsd/python/wrapUsdUndoManager.cpp @@ -64,7 +64,7 @@ class PythonUndoBlock void _trackLayerStates(const SdfLayerHandle& layer) { - MayaUsd::UsdUndoManager::instance().trackLayerStates(layer); + MayaUsd::UsdUndoManager::instance().trackStatesOnNewStage(layer); } } // namespace diff --git a/lib/mayaUsd/ufe/StagesSubject.cpp b/lib/mayaUsd/ufe/StagesSubject.cpp index 5904b10d43..47a9f398fd 100644 --- a/lib/mayaUsd/ufe/StagesSubject.cpp +++ b/lib/mayaUsd/ufe/StagesSubject.cpp @@ -307,7 +307,7 @@ void StagesSubject::stageEditTargetChanged( UsdStageWeakPtr const& sender) { // Track the edit target layer's state - UsdUndoManager::instance().trackLayerStates(notice.GetStage()->GetEditTarget().GetLayer()); + UsdUndoManager::instance().trackStatesOnEditTargetChange(notice.GetStage()->GetEditTarget().GetLayer()); } #endif @@ -319,7 +319,7 @@ void StagesSubject::onStageSet(const MayaUsdProxyStageSetNotice& notice) // invalid stage. if (noticeStage) { // Track the edit target layer's state - UsdUndoManager::instance().trackLayerStates(noticeStage->GetEditTarget().GetLayer()); + UsdUndoManager::instance().trackStatesOnNewStage(noticeStage->GetEditTarget().GetLayer()); } #endif diff --git a/lib/mayaUsd/undo/UsdUndoManager.cpp b/lib/mayaUsd/undo/UsdUndoManager.cpp index f7f14c0721..df5ab77cf2 100644 --- a/lib/mayaUsd/undo/UsdUndoManager.cpp +++ b/lib/mayaUsd/undo/UsdUndoManager.cpp @@ -29,9 +29,16 @@ UsdUndoManager& UsdUndoManager::instance() return undoManager; } -void UsdUndoManager::trackLayerStates(const SdfLayerHandle& layer) +void UsdUndoManager::trackStatesOnNewStage(const SdfLayerHandle& layer) { - layer->SetStateDelegate(UsdUndoStateDelegate::New()); + _layerStateDelegate = TfCreateRefPtr(new UsdUndoStateDelegate); + + layer->SetStateDelegate(_layerStateDelegate); +} + +void UsdUndoManager::trackStatesOnEditTargetChange(const SdfLayerHandle& layer) +{ + layer->SetStateDelegate(_layerStateDelegate); } void UsdUndoManager::addInverse(InvertFunc func) diff --git a/lib/mayaUsd/undo/UsdUndoManager.h b/lib/mayaUsd/undo/UsdUndoManager.h index 5695bc4e8f..6b8a623cf6 100644 --- a/lib/mayaUsd/undo/UsdUndoManager.h +++ b/lib/mayaUsd/undo/UsdUndoManager.h @@ -18,6 +18,7 @@ #define MAYAUSD_UNDO_UNDOMANAGER_H #include "UsdUndoableItem.h" +#include "UsdUndoStateDelegate.h" #include @@ -52,8 +53,11 @@ class MAYAUSD_CORE_PUBLIC UsdUndoManager UsdUndoManager(UsdUndoManager&&) = delete; UsdUndoManager& operator=(UsdUndoManager&&) = delete; - // tracks layer states by spawning a new UsdUndoStateDelegate - void trackLayerStates(const SdfLayerHandle& layer); + // tracks layer states on a new stage + void trackStatesOnNewStage(const SdfLayerHandle& layer); + + // tracks layer states when switching between stage's layers + void trackStatesOnEditTargetChange(const SdfLayerHandle& layer); private: friend class UsdUndoStateDelegate; @@ -68,6 +72,8 @@ class MAYAUSD_CORE_PUBLIC UsdUndoManager private: InvertFuncs _invertFuncs; + + UsdUndoStateDelegateRefPtr _layerStateDelegate; }; } // namespace MAYAUSD_NS_DEF diff --git a/lib/mayaUsd/undo/UsdUndoStateDelegate.cpp b/lib/mayaUsd/undo/UsdUndoStateDelegate.cpp index 2afa603e56..1add86480f 100644 --- a/lib/mayaUsd/undo/UsdUndoStateDelegate.cpp +++ b/lib/mayaUsd/undo/UsdUndoStateDelegate.cpp @@ -33,11 +33,6 @@ UsdUndoStateDelegate::UsdUndoStateDelegate() UsdUndoStateDelegate::~UsdUndoStateDelegate() { } -UsdUndoStateDelegateRefPtr UsdUndoStateDelegate::New() -{ - return TfCreateRefPtr(new UsdUndoStateDelegate()); -} - bool UsdUndoStateDelegate::_IsDirty() { return _dirty; } void UsdUndoStateDelegate::_MarkCurrentStateAsClean() { _dirty = false; } diff --git a/lib/mayaUsd/undo/UsdUndoStateDelegate.h b/lib/mayaUsd/undo/UsdUndoStateDelegate.h index 96f7ffb301..27c0b07412 100644 --- a/lib/mayaUsd/undo/UsdUndoStateDelegate.h +++ b/lib/mayaUsd/undo/UsdUndoStateDelegate.h @@ -45,8 +45,6 @@ class MAYAUSD_CORE_PUBLIC UsdUndoStateDelegate : public SdfLayerStateDelegateBas UsdUndoStateDelegate(); ~UsdUndoStateDelegate() override; - static UsdUndoStateDelegateRefPtr New(); - private: void invertSetField(const SdfPath& path, const TfToken& fieldName, const VtValue& inverse); void invertCreateSpec(const SdfPath& path, bool inert); From 2981cca6b185f54c26cca5e28b62451d9d01a10c Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Fri, 4 Dec 2020 15:44:57 -0500 Subject: [PATCH 2/4] Revert "Fix undo/redo crash when switching between layers." This reverts commit 120ae476d5dbf61f5f3cc0d898bc7639ebef854a. --- lib/mayaUsd/python/wrapUsdUndoManager.cpp | 2 +- lib/mayaUsd/ufe/StagesSubject.cpp | 4 ++-- lib/mayaUsd/undo/UsdUndoManager.cpp | 11 ++--------- lib/mayaUsd/undo/UsdUndoManager.h | 10 ++-------- lib/mayaUsd/undo/UsdUndoStateDelegate.cpp | 5 +++++ lib/mayaUsd/undo/UsdUndoStateDelegate.h | 2 ++ 6 files changed, 14 insertions(+), 20 deletions(-) diff --git a/lib/mayaUsd/python/wrapUsdUndoManager.cpp b/lib/mayaUsd/python/wrapUsdUndoManager.cpp index 89de999c25..0ae547c636 100644 --- a/lib/mayaUsd/python/wrapUsdUndoManager.cpp +++ b/lib/mayaUsd/python/wrapUsdUndoManager.cpp @@ -64,7 +64,7 @@ class PythonUndoBlock void _trackLayerStates(const SdfLayerHandle& layer) { - MayaUsd::UsdUndoManager::instance().trackStatesOnNewStage(layer); + MayaUsd::UsdUndoManager::instance().trackLayerStates(layer); } } // namespace diff --git a/lib/mayaUsd/ufe/StagesSubject.cpp b/lib/mayaUsd/ufe/StagesSubject.cpp index 47a9f398fd..5904b10d43 100644 --- a/lib/mayaUsd/ufe/StagesSubject.cpp +++ b/lib/mayaUsd/ufe/StagesSubject.cpp @@ -307,7 +307,7 @@ void StagesSubject::stageEditTargetChanged( UsdStageWeakPtr const& sender) { // Track the edit target layer's state - UsdUndoManager::instance().trackStatesOnEditTargetChange(notice.GetStage()->GetEditTarget().GetLayer()); + UsdUndoManager::instance().trackLayerStates(notice.GetStage()->GetEditTarget().GetLayer()); } #endif @@ -319,7 +319,7 @@ void StagesSubject::onStageSet(const MayaUsdProxyStageSetNotice& notice) // invalid stage. if (noticeStage) { // Track the edit target layer's state - UsdUndoManager::instance().trackStatesOnNewStage(noticeStage->GetEditTarget().GetLayer()); + UsdUndoManager::instance().trackLayerStates(noticeStage->GetEditTarget().GetLayer()); } #endif diff --git a/lib/mayaUsd/undo/UsdUndoManager.cpp b/lib/mayaUsd/undo/UsdUndoManager.cpp index df5ab77cf2..f7f14c0721 100644 --- a/lib/mayaUsd/undo/UsdUndoManager.cpp +++ b/lib/mayaUsd/undo/UsdUndoManager.cpp @@ -29,16 +29,9 @@ UsdUndoManager& UsdUndoManager::instance() return undoManager; } -void UsdUndoManager::trackStatesOnNewStage(const SdfLayerHandle& layer) +void UsdUndoManager::trackLayerStates(const SdfLayerHandle& layer) { - _layerStateDelegate = TfCreateRefPtr(new UsdUndoStateDelegate); - - layer->SetStateDelegate(_layerStateDelegate); -} - -void UsdUndoManager::trackStatesOnEditTargetChange(const SdfLayerHandle& layer) -{ - layer->SetStateDelegate(_layerStateDelegate); + layer->SetStateDelegate(UsdUndoStateDelegate::New()); } void UsdUndoManager::addInverse(InvertFunc func) diff --git a/lib/mayaUsd/undo/UsdUndoManager.h b/lib/mayaUsd/undo/UsdUndoManager.h index 6b8a623cf6..5695bc4e8f 100644 --- a/lib/mayaUsd/undo/UsdUndoManager.h +++ b/lib/mayaUsd/undo/UsdUndoManager.h @@ -18,7 +18,6 @@ #define MAYAUSD_UNDO_UNDOMANAGER_H #include "UsdUndoableItem.h" -#include "UsdUndoStateDelegate.h" #include @@ -53,11 +52,8 @@ class MAYAUSD_CORE_PUBLIC UsdUndoManager UsdUndoManager(UsdUndoManager&&) = delete; UsdUndoManager& operator=(UsdUndoManager&&) = delete; - // tracks layer states on a new stage - void trackStatesOnNewStage(const SdfLayerHandle& layer); - - // tracks layer states when switching between stage's layers - void trackStatesOnEditTargetChange(const SdfLayerHandle& layer); + // tracks layer states by spawning a new UsdUndoStateDelegate + void trackLayerStates(const SdfLayerHandle& layer); private: friend class UsdUndoStateDelegate; @@ -72,8 +68,6 @@ class MAYAUSD_CORE_PUBLIC UsdUndoManager private: InvertFuncs _invertFuncs; - - UsdUndoStateDelegateRefPtr _layerStateDelegate; }; } // namespace MAYAUSD_NS_DEF diff --git a/lib/mayaUsd/undo/UsdUndoStateDelegate.cpp b/lib/mayaUsd/undo/UsdUndoStateDelegate.cpp index 1add86480f..2afa603e56 100644 --- a/lib/mayaUsd/undo/UsdUndoStateDelegate.cpp +++ b/lib/mayaUsd/undo/UsdUndoStateDelegate.cpp @@ -33,6 +33,11 @@ UsdUndoStateDelegate::UsdUndoStateDelegate() UsdUndoStateDelegate::~UsdUndoStateDelegate() { } +UsdUndoStateDelegateRefPtr UsdUndoStateDelegate::New() +{ + return TfCreateRefPtr(new UsdUndoStateDelegate()); +} + bool UsdUndoStateDelegate::_IsDirty() { return _dirty; } void UsdUndoStateDelegate::_MarkCurrentStateAsClean() { _dirty = false; } diff --git a/lib/mayaUsd/undo/UsdUndoStateDelegate.h b/lib/mayaUsd/undo/UsdUndoStateDelegate.h index 27c0b07412..96f7ffb301 100644 --- a/lib/mayaUsd/undo/UsdUndoStateDelegate.h +++ b/lib/mayaUsd/undo/UsdUndoStateDelegate.h @@ -45,6 +45,8 @@ class MAYAUSD_CORE_PUBLIC UsdUndoStateDelegate : public SdfLayerStateDelegateBas UsdUndoStateDelegate(); ~UsdUndoStateDelegate() override; + static UsdUndoStateDelegateRefPtr New(); + private: void invertSetField(const SdfPath& path, const TfToken& fieldName, const VtValue& inverse); void invertCreateSpec(const SdfPath& path, bool inert); From a87fc0b7df6485a042b889cd547235d4161a88e8 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Fri, 4 Dec 2020 15:54:50 -0500 Subject: [PATCH 3/4] Make sure the USD state delegate is registered for current existing PR. --- lib/mayaUsd/ufe/StagesSubject.cpp | 8 ++++---- lib/mayaUsd/ufe/StagesSubject.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/mayaUsd/ufe/StagesSubject.cpp b/lib/mayaUsd/ufe/StagesSubject.cpp index 5904b10d43..3c48b3706a 100644 --- a/lib/mayaUsd/ufe/StagesSubject.cpp +++ b/lib/mayaUsd/ufe/StagesSubject.cpp @@ -21,7 +21,7 @@ #include #include #include -#if UFE_PREVIEW_VERSION_NUM >= 2029 +#if UFE_PREVIEW_VERSION_NUM >= 2025 #include #endif @@ -301,7 +301,7 @@ void StagesSubject::stageChanged( } } -#if UFE_PREVIEW_VERSION_NUM >= 2029 +#if UFE_PREVIEW_VERSION_NUM >= 2025 void StagesSubject::stageEditTargetChanged( UsdNotice::StageEditTargetChanged const& notice, UsdStageWeakPtr const& sender) @@ -313,7 +313,7 @@ void StagesSubject::stageEditTargetChanged( void StagesSubject::onStageSet(const MayaUsdProxyStageSetNotice& notice) { -#if UFE_PREVIEW_VERSION_NUM >= 2029 +#if UFE_PREVIEW_VERSION_NUM >= 2025 auto noticeStage = notice.GetStage(); // Check if stage received from notice is valid. We could have cases where a ProxyShape has an // invalid stage. @@ -336,7 +336,7 @@ void StagesSubject::onStageSet(const MayaUsdProxyStageSetNotice& notice) NoticeKeys noticeKeys; noticeKeys[0] = TfNotice::Register(me, &StagesSubject::stageChanged, stage); -#if UFE_PREVIEW_VERSION_NUM >= 2029 +#if UFE_PREVIEW_VERSION_NUM >= 2025 noticeKeys[1] = TfNotice::Register(me, &StagesSubject::stageEditTargetChanged, stage); #endif fStageListeners[stage] = noticeKeys; diff --git a/lib/mayaUsd/ufe/StagesSubject.h b/lib/mayaUsd/ufe/StagesSubject.h index 74a8e7fdcc..0b0bba6699 100644 --- a/lib/mayaUsd/ufe/StagesSubject.h +++ b/lib/mayaUsd/ufe/StagesSubject.h @@ -73,7 +73,7 @@ class MAYAUSD_CORE_PUBLIC StagesSubject : public TfWeakBase //! Call the stageChanged() methods on stage observers. void stageChanged(UsdNotice::ObjectsChanged const& notice, UsdStageWeakPtr const& sender); -#if UFE_PREVIEW_VERSION_NUM >= 2029 +#if UFE_PREVIEW_VERSION_NUM >= 2025 //! Call the stageEditTargetChanged() methods on stage observers. void stageEditTargetChanged( UsdNotice::StageEditTargetChanged const& notice, @@ -88,7 +88,7 @@ class MAYAUSD_CORE_PUBLIC StagesSubject : public TfWeakBase void onStageInvalidate(const MayaUsdProxyStageInvalidateNotice& notice); // Array of Notice::Key for registered listener -#if UFE_PREVIEW_VERSION_NUM >= 2029 +#if UFE_PREVIEW_VERSION_NUM >= 2025 using NoticeKeys = std::array; #else using NoticeKeys = std::array; From 677db401054bde8e5d34f150bfe6ca32fc4438e7 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Fri, 4 Dec 2020 15:57:40 -0500 Subject: [PATCH 4/4] Address feedback. Use a dynamic_cast to determine if a layer is already registered with our UsdUndoStateDelegate. If the cast fails, that means it needs to be set with a new one. --- lib/mayaUsd/undo/UsdUndoManager.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/mayaUsd/undo/UsdUndoManager.cpp b/lib/mayaUsd/undo/UsdUndoManager.cpp index f7f14c0721..9dbb491318 100644 --- a/lib/mayaUsd/undo/UsdUndoManager.cpp +++ b/lib/mayaUsd/undo/UsdUndoManager.cpp @@ -31,7 +31,13 @@ UsdUndoManager& UsdUndoManager::instance() void UsdUndoManager::trackLayerStates(const SdfLayerHandle& layer) { - layer->SetStateDelegate(UsdUndoStateDelegate::New()); + // Check if the layer has already been given a UsdUndoStateDelegate + // if the cast fails that means we need to set a new one. + auto usdUndoStateDelegatePtr + = TfDynamic_cast(layer->GetStateDelegate()); + if (!usdUndoStateDelegatePtr) { + layer->SetStateDelegate(UsdUndoStateDelegate::New()); + } } void UsdUndoManager::addInverse(InvertFunc func)