diff --git a/lib/mayaUsd/ufe/UsdSetXformOpUndoableCommandBase.cpp b/lib/mayaUsd/ufe/UsdSetXformOpUndoableCommandBase.cpp index c705978347..3cbc5bfc69 100644 --- a/lib/mayaUsd/ufe/UsdSetXformOpUndoableCommandBase.cpp +++ b/lib/mayaUsd/ufe/UsdSetXformOpUndoableCommandBase.cpp @@ -21,68 +21,127 @@ PXR_NAMESPACE_USING_DIRECTIVE -namespace { -void warnUnimplemented(const char* msg) { TF_WARN("Illegal call to unimplemented %s", msg); } -} // namespace - namespace MAYAUSD_NS_DEF { namespace ufe { -template -UsdSetXformOpUndoableCommandBase::UsdSetXformOpUndoableCommandBase( - const Ufe::Path& path, - const UsdTimeCode& writeTime) +UsdSetXformOpUndoableCommandBase::UsdSetXformOpUndoableCommandBase( + const PXR_NS::VtValue& value, + const Ufe::Path& path, + const PXR_NS::UsdTimeCode& writeTime) : Ufe::SetVector3dUndoableCommand(path) - , _readTime(getTime(path)) // Always read from proxy shape time. , _writeTime(writeTime) + , _newOpValue(value) + , _isPrepared(false) + , _canUpdateValue(true) + , _opCreated(false) +{ +} + +UsdSetXformOpUndoableCommandBase::UsdSetXformOpUndoableCommandBase( + const Ufe::Path& path, + const UsdTimeCode& writeTime) + : UsdSetXformOpUndoableCommandBase({}, path, writeTime) +{ +} + +void UsdSetXformOpUndoableCommandBase::execute() +{ + // Create the attribute and cache the initial value, + // if this is the first time we're executed, or redo + // the attribute creation. + recreateOpIfNeeded(); + + // Set the new value. + prepareAndSet(_newOpValue); + _canUpdateValue = true; +} + +void UsdSetXformOpUndoableCommandBase::undo() +{ + // If the command was never called at all, do nothing. + // Maya can start by calling undo. + if (!_isPrepared) + return; + + // Restore the initial value and potentially remove the creatd attributes. + setValue(_initialOpValue, _writeTime); + removeOpIfNeeded(); + _canUpdateValue = false; +} + +void UsdSetXformOpUndoableCommandBase::redo() { + // Redo the attribute creation if the attribute was already created + // but then undone. + recreateOpIfNeeded(); + + // Set the new value, potentially creating the attribute if it + // did not exists or caching the initial value if this is the + // first time the command is executed, redone or undone. + prepareAndSet(_newOpValue); + _canUpdateValue = true; } -template void UsdSetXformOpUndoableCommandBase::execute() +void UsdSetXformOpUndoableCommandBase::updateNewValue(const VtValue& v) { - warnUnimplemented("UsdSetXformOpUndoableCommandBase::execute()"); + // Redo the attribute creation if the attribute was already created + // but then undone. + recreateOpIfNeeded(); + + // Update the value that will be set. + if (_canUpdateValue) + _newOpValue = v; + + // Set the new value, potentially creating the attribute if it + // did not exists or caching the initial value if this is the + // first time the command is executed, redone or undone. + prepareAndSet(_newOpValue); + _canUpdateValue = true; } -template void UsdSetXformOpUndoableCommandBase::undo() +void UsdSetXformOpUndoableCommandBase::prepareAndSet(const VtValue& v) { - if (_state == kInitial) { - // Spurious call from Maya, ignore. - _state = kInitialUndoCalled; + if (v.IsEmpty()) return; - } - _undoableItem.undo(); - _state = kUndone; + + prepareOpIfNeeded(); + setValue(v, _writeTime); } -template void UsdSetXformOpUndoableCommandBase::redo() +void UsdSetXformOpUndoableCommandBase::prepareOpIfNeeded() { - warnUnimplemented("UsdSetXformOpUndoableCommandBase::redo()"); + if (_isPrepared) + return; + + createOpIfNeeded(_opCreationUndo); + _initialOpValue = getValue(_writeTime); + _isPrepared = true; + _opCreated = true; } -template void UsdSetXformOpUndoableCommandBase::handleSet(const T& v) +void UsdSetXformOpUndoableCommandBase::recreateOpIfNeeded() { - if (_state == kInitialUndoCalled) { - // Spurious call from Maya, ignore. Otherwise, we set a value that - // is identical to the previous, the UsdUndoBlock does not capture - // any invertFunc's, and subsequent undo() calls undo nothing. - _state = kInitial; - } else if (_state == kInitial) { - UsdUndoBlock undoBlock(&_undoableItem); - setValue(v); - _state = kExecute; - } else if (_state == kExecute) { - setValue(v); - } else if (_state == kUndone) { - _undoableItem.redo(); - _state = kRedone; - } + if (!_isPrepared) + return; + + if (_opCreated) + return; + + _opCreationUndo.redo(); + _opCreated = true; } -// Explicit instantiation for transform ops that can be set from matrices and -// vectors. -template class UsdSetXformOpUndoableCommandBase; -template class UsdSetXformOpUndoableCommandBase; -template class UsdSetXformOpUndoableCommandBase; +void UsdSetXformOpUndoableCommandBase::removeOpIfNeeded() +{ + if (!_isPrepared) + return; + + if (!_opCreated) + return; + + _opCreationUndo.undo(); + _opCreated = false; +} } // namespace ufe } // namespace MAYAUSD_NS_DEF diff --git a/lib/mayaUsd/ufe/UsdSetXformOpUndoableCommandBase.h b/lib/mayaUsd/ufe/UsdSetXformOpUndoableCommandBase.h index d925bafb2c..97c71fae62 100644 --- a/lib/mayaUsd/ufe/UsdSetXformOpUndoableCommandBase.h +++ b/lib/mayaUsd/ufe/UsdSetXformOpUndoableCommandBase.h @@ -19,6 +19,7 @@ #include +#include #include #include @@ -31,48 +32,83 @@ namespace ufe { // Helper class to factor out common code for translate, rotate, scale // undoable commands. It is templated on the type of the transform op. // -// Developing commands to work with Maya TRS commands is made more difficult -// because Maya calls undo(), but never calls redo(): it simply calls set() -// with the new value again. We must distinguish cases where set() must -// capture state, so that undo() can completely remove any added primSpecs or -// attrSpecs. This class implements state tracking to allow this: state is -// saved on transition between kInitial and kExecute. -// UsdTransform3dMayaXformStack has a state machine based implementation that -// avoids conditionals, but UsdSetXformOpUndoableCommandBase is less invasive -// from a development standpoint. - -template +// We must do a careful dance due to historic reasons and the way Maya handle +// interactive commands: +// +// - These commands can be wrapped inside other commands which may +// use their own UsdUndoBlock. In particular, we must not try to +// undo an attribute creation if it was not yet created. +// +// - Maya can call undo and set-value before first executing the +// command. In particular, when using manipualtion tools, Maya +// will usually do loops of undo/set-value/redo, thus beginning +// by undoing a command that was never executed. +// +// - As a general rule, when undoing, we want to remove any attributes +// that were created when first executed. +// +// - When redoing some commands after an undo, Maya will update the +// value to be set with an incorrect value when operating in object +// space, which must be ignored. +// +// Those things are what the prepare-op/recreate-op/remove-op functions are +// aimed to support. Also, we must only capture the initial value the first +// time the value is modified, to support both the inital undo/set-value and +// avoid losing the initial value on repeat set-value. class UsdSetXformOpUndoableCommandBase : public Ufe::SetVector3dUndoableCommand { - const PXR_NS::UsdTimeCode _readTime; - const PXR_NS::UsdTimeCode _writeTime; - UsdUfe::UsdUndoableItem _undoableItem; - enum State - { - kInitial, - kInitialUndoCalled, - kExecute, - kUndone, - kRedone - }; - State _state { kInitial }; - public: + UsdSetXformOpUndoableCommandBase( + const PXR_NS::VtValue& newValue, + const Ufe::Path& path, + const PXR_NS::UsdTimeCode& writeTime); UsdSetXformOpUndoableCommandBase(const Ufe::Path& path, const PXR_NS::UsdTimeCode& writeTime); // Ufe::UndoableCommand overrides. - // No-op: Maya calls set() rather than execute(). void execute() override; void undo() override; - // No-op: Maya calls set() rather than redo(). void redo() override; - PXR_NS::UsdTimeCode readTime() const { return _readTime; } PXR_NS::UsdTimeCode writeTime() const { return _writeTime; } - virtual void setValue(const T&) = 0; +protected: + // Create the XformOp attributes if they do not exists. + // The attribute creation must be capture in the UsdUndoableItem by using a + // UsdUndoBlock, so that removeOpIfNeeded and recreateOpIfNeeded can undo + // and redo the attribute creation if needed. + virtual void createOpIfNeeded(UsdUndoableItem&) = 0; + + // Get the attribute at the given time. + virtual PXR_NS::VtValue getValue(const PXR_NS::UsdTimeCode& time) const = 0; - void handleSet(const T& v); + // Set the attribute at the given time. The value is guaranteed to either be + // the initial value that was returned by the getValue function above or a + // new value passed to the updateNewValue function below. So you are guaranteed + // that the type contained in the VtValue is the type you want. + virtual void setValue(const PXR_NS::VtValue&, const PXR_NS::UsdTimeCode& time) = 0; + + // Function called by sub-classed when they want to set a new value. + void updateNewValue(const PXR_NS::VtValue& v); + +private: + void prepareAndSet(const PXR_NS::VtValue&); + + // Create the XformOp attributes if they do not exists and cache the initial value. + void prepareOpIfNeeded(); + + // Recreate the attribute after being removed if it was created. + void recreateOpIfNeeded(); + + // Remove the attribute if it was created. + void removeOpIfNeeded(); + + const PXR_NS::UsdTimeCode _writeTime; + PXR_NS::VtValue _initialOpValue; + PXR_NS::VtValue _newOpValue; + UsdUndoableItem _opCreationUndo; + bool _isPrepared; + bool _canUpdateValue; + bool _opCreated; }; } // namespace ufe diff --git a/lib/mayaUsd/ufe/UsdTransform3dCommonAPI.cpp b/lib/mayaUsd/ufe/UsdTransform3dCommonAPI.cpp index 5999ef47f2..06f8519f35 100644 --- a/lib/mayaUsd/ufe/UsdTransform3dCommonAPI.cpp +++ b/lib/mayaUsd/ufe/UsdTransform3dCommonAPI.cpp @@ -31,7 +31,7 @@ namespace ufe { namespace { -class CommonAPITranslateUndoableCmd : public UsdSetXformOpUndoableCommandBase +class CommonAPITranslateUndoableCmd : public UsdSetXformOpUndoableCommandBase { public: CommonAPITranslateUndoableCmd(const UsdSceneItem::Ptr& item, const UsdTimeCode& writeTime) @@ -40,11 +40,31 @@ class CommonAPITranslateUndoableCmd : public UsdSetXformOpUndoableCommandBase(), writeTime); + } + + VtValue getValue(const UsdTimeCode& readTime) const override + { + GfVec3d translation; + _commonAPI.GetXformVectors(&translation, nullptr, nullptr, nullptr, nullptr, readTime); + return VtValue(translation); + } bool set(double x, double y, double z) override { - handleSet(GfVec3d(x, y, z)); + updateNewValue(VtValue(GfVec3d(x, y, z))); return true; } @@ -52,7 +72,7 @@ class CommonAPITranslateUndoableCmd : public UsdSetXformOpUndoableCommandBase +class CommonAPIRotateUndoableCmd : public UsdSetXformOpUndoableCommandBase { public: CommonAPIRotateUndoableCmd(const UsdSceneItem::Ptr& item, const UsdTimeCode& writeTime) @@ -61,14 +81,31 @@ class CommonAPIRotateUndoableCmd : public UsdSetXformOpUndoableCommandBase(), UsdGeomXformCommonAPI::RotationOrderXYZ, writeTime); + } + + VtValue getValue(const UsdTimeCode& readTime) const override + { + GfVec3f rotation; + _commonAPI.GetXformVectors(nullptr, &rotation, nullptr, nullptr, nullptr, readTime); + return VtValue(rotation); } bool set(double x, double y, double z) override { - handleSet(GfVec3f(x, y, z)); + updateNewValue(VtValue(GfVec3f(x, y, z))); return true; } @@ -76,7 +113,7 @@ class CommonAPIRotateUndoableCmd : public UsdSetXformOpUndoableCommandBase +class CommonAPIScaleUndoableCmd : public UsdSetXformOpUndoableCommandBase { public: @@ -86,11 +123,31 @@ class CommonAPIScaleUndoableCmd : public UsdSetXformOpUndoableCommandBase(), writeTime); + } + + VtValue getValue(const UsdTimeCode& readTime) const override + { + GfVec3f scale; + _commonAPI.GetXformVectors(nullptr, nullptr, &scale, nullptr, nullptr, readTime); + return VtValue(scale); + } bool set(double x, double y, double z) override { - handleSet(GfVec3f(x, y, z)); + updateNewValue(VtValue(GfVec3f(x, y, z))); return true; } @@ -98,7 +155,7 @@ class CommonAPIScaleUndoableCmd : public UsdSetXformOpUndoableCommandBase +class CommonAPIPivotUndoableCmd : public UsdSetXformOpUndoableCommandBase { public: CommonAPIPivotUndoableCmd(const UsdSceneItem::Ptr& item, const UsdTimeCode& writeTime) @@ -107,11 +164,31 @@ class CommonAPIPivotUndoableCmd : public UsdSetXformOpUndoableCommandBase(), writeTime); + } + + VtValue getValue(const UsdTimeCode& readTime) const override + { + GfVec3f pivot; + _commonAPI.GetXformVectors(nullptr, nullptr, nullptr, &pivot, nullptr, readTime); + return VtValue(pivot); + } bool set(double x, double y, double z) override { - handleSet(GfVec3f(x, y, z)); + updateNewValue(VtValue(GfVec3f(x, y, z))); return true; } diff --git a/lib/mayaUsd/ufe/UsdTransform3dMatrixOp.cpp b/lib/mayaUsd/ufe/UsdTransform3dMatrixOp.cpp index 7dd2221f2b..4f81924957 100644 --- a/lib/mayaUsd/ufe/UsdTransform3dMatrixOp.cpp +++ b/lib/mayaUsd/ufe/UsdTransform3dMatrixOp.cpp @@ -130,7 +130,7 @@ class UsdSetMatrix4dUndoableCmd : public UsdUndoableCommand +class MatrixOpUndoableCmdBase : public UsdSetXformOpUndoableCommandBase { UsdGeomXformOp _op; @@ -144,11 +144,25 @@ class MatrixOpUndoableCmdBase : public UsdSetXformOpUndoableCommandBase @@ -39,7 +40,7 @@ PXR_NAMESPACE_USING_DIRECTIVE namespace { using BaseUndoableCommand = Ufe::BaseUndoableCommand; -using OpFunc = std::function; +using OpFunc = std::function; using namespace MayaUsd::ufe; @@ -189,141 +190,86 @@ createTransform3d(const Ufe::SceneItem::Ptr& item, NextTransform3dFn nextTransfo // Helper class to factor out common code for translate, rotate, scale // undoable commands. -class UsdTRSUndoableCmdBase : public Ufe::SetVector3dUndoableCommand +// +// We must do a careful dance due to historic reasons and the way Maya handle +// interactive commands: +// +// - These commands can be wrapped inside other commands which may +// use their own UsdUndoBlock. In particular, we must not try to +// undo an attribute creation if it was not yet created. +// +// - Maya can call undo and set-value before first executing the +// command. In particular, when using manipualtion tools, Maya +// will usually do loops of undo/set-value/execute, thus beginning +// by undoing a command that was never executed. +// +// - As a general rule, when undoing, we want to remove any attributes +// that were created when first executed. +// +// - When redoing some commands after an undo, Maya will update the +// value to be set with an incorrect value when operating in object +// space, which must be ignored. +// +// Those things are what the prepare-op/recreate-op/remove-op functions are +// aimed to support. Also, we must only capture the initial value the first +// time thevalue is modified, to support both the inital undo/set-value and +// avoid losing the initial value on repeat set-value. +class UsdTRSUndoableCmdBase : public UsdSetXformOpUndoableCommandBase { -private: - const UsdTimeCode _writeTime; - VtValue _newOpValue; - UsdGeomXformOp _op; - OpFunc _opFunc; - UsdUndoableItem _undoableItem; - public: - struct State - { - virtual void handleUndo(UsdTRSUndoableCmdBase*) - { - TF_CODING_ERROR( - "Illegal handleUndo() call in UsdTRSUndoableCmdBase for state '%s'.", - typeid(*this).name()); - } - virtual void handleSet(UsdTRSUndoableCmdBase*, const VtValue&) - { - TF_CODING_ERROR( - "Illegal handleSet() call in UsdTRSUndoableCmdBase for state '%s'.", - typeid(*this).name()); - } - }; - - struct InitialState : public State - { - void handleUndo(UsdTRSUndoableCmdBase* cmd) override - { - // Maya triggers an undo on command creation, ignore it. - cmd->_state = &UsdTRSUndoableCmdBase::_initialUndoCalledState; - } - void handleSet(UsdTRSUndoableCmdBase* cmd, const VtValue& v) override - { - // Add undoblock to capture edits - UsdUndoBlock undoBlock(&cmd->_undoableItem); - - // Going from initial to executing / executed state, save value. - cmd->_op = cmd->_opFunc(*cmd); - cmd->setValue(v); - cmd->_state = &UsdTRSUndoableCmdBase::_executeState; - } - }; - - struct InitialUndoCalledState : public State - { - void handleSet(UsdTRSUndoableCmdBase* cmd, const VtValue&) override - { - // Maya triggers a redo on command creation, ignore it. - cmd->_state = &UsdTRSUndoableCmdBase::_initialState; - } - }; - - struct ExecuteState : public State - { - void handleUndo(UsdTRSUndoableCmdBase* cmd) override - { - // Undo - cmd->_undoableItem.undo(); - cmd->_state = &UsdTRSUndoableCmdBase::_undoneState; - } - void handleSet(UsdTRSUndoableCmdBase* cmd, const VtValue& v) override { cmd->setValue(v); } - }; - - struct UndoneState : public State - { - void handleSet(UsdTRSUndoableCmdBase* cmd, const VtValue&) override - { - // Redo - cmd->_undoableItem.redo(); - cmd->_state = &UsdTRSUndoableCmdBase::_redoneState; - } - }; - - struct RedoneState : public State - { - void handleUndo(UsdTRSUndoableCmdBase* cmd) override - { - // Undo - cmd->_undoableItem.undo(); - cmd->_state = &UsdTRSUndoableCmdBase::_undoneState; - } - // The redone state should normally be reached only once manipulation - // is over, after undo, so setting new values in the redone state seems - // illogical. However, during point snapping manipulation, within a - // single drag, the Maya move command repeatedly calls undo, then redo, - // setting new values after the redo. Treat such events identically to - // the Execute state. - void handleSet(UsdTRSUndoableCmdBase* cmd, const VtValue& v) override { cmd->setValue(v); } - }; - UsdTRSUndoableCmdBase( const VtValue& newOpValue, const Ufe::Path& path, OpFunc opFunc, const UsdTimeCode& writeTime) - : Ufe::SetVector3dUndoableCommand(path) - , _writeTime(writeTime) - , _newOpValue(newOpValue) + : UsdSetXformOpUndoableCommandBase(newOpValue, path, writeTime) , _op() , _opFunc(std::move(opFunc)) { } - // Ufe::UndoableCommand overrides. - void execute() override { handleSet(_newOpValue); } - void undo() override { _state->handleUndo(this); } - void redo() override { handleSet(_newOpValue); } +protected: + void createOpIfNeeded(UsdUndoableItem& undoableItem) override + { + if (_op) + return; - void handleSet(const VtValue& v) { _state->handleSet(this, v); } + _op = _opFunc(*this, undoableItem); + } - void setValue(const VtValue& v) + void setValue(const VtValue& v, const UsdTimeCode& writeTime) override { + if (!_op) + return; + + if (v.IsEmpty()) + return; + auto attr = _op.GetAttr(); - if (attr) { - _newOpValue = v; - _op.GetAttr().Set(v, _writeTime); - } + if (!attr) + return; + + attr.Set(v, writeTime); } - static InitialState _initialState; - static InitialUndoCalledState _initialUndoCalledState; - static ExecuteState _executeState; - static UndoneState _undoneState; - static RedoneState _redoneState; + VtValue getValue(const UsdTimeCode& readTime) const override + { + if (!_op) + return {}; - State* _state { &_initialState }; -}; + auto attr = _op.GetAttr(); + if (!attr) + return {}; + + VtValue value; + attr.Get(&value, readTime); + return value; + } -UsdTRSUndoableCmdBase::InitialState UsdTRSUndoableCmdBase::_initialState; -UsdTRSUndoableCmdBase::InitialUndoCalledState UsdTRSUndoableCmdBase::_initialUndoCalledState; -UsdTRSUndoableCmdBase::ExecuteState UsdTRSUndoableCmdBase::_executeState; -UsdTRSUndoableCmdBase::UndoneState UsdTRSUndoableCmdBase::_undoneState; -UsdTRSUndoableCmdBase::RedoneState UsdTRSUndoableCmdBase::_redoneState; +private: + UsdGeomXformOp _op; + OpFunc _opFunc; +}; // UsdRotatePivotTranslateUndoableCmd uses hard-coded USD common transform API // single pivot attribute name, not reusable. @@ -344,7 +290,7 @@ template class UsdVecOpUndoableCmd : public UsdTRSUndoableCmdBase { VtValue v; v = V(x, y, z); - handleSet(v); + updateNewValue(v); return true; } }; @@ -368,7 +314,7 @@ class UsdRotateOpUndoableCmd : public UsdTRSUndoableCmdBase { VtValue v; v = _cvtRotXYZToAttr(x, y, z); - handleSet(v); + updateNewValue(v); return true; } @@ -479,32 +425,33 @@ UsdTransform3dMayaXformStack::rotateCmd(double x, double y, double z) GfVec3f v(x, y, z); CvtRotXYZToAttrFn cvt = hasRotate ? getCvtRotXYZToAttrFn(op.GetOpName()) : toXYZ; - auto f = OpFunc( - [attrName, opSuffix = getTRSOpSuffix(), setXformOpOrderFn = getXformOpOrderFn(), v]( - const BaseUndoableCommand& cmd) { - SceneItemHolder usdSceneItem(cmd); - - auto attr = getUsdPrimAttribute(usdSceneItem.item().prim(), attrName); - if (attr) { - return UsdGeomXformOp(attr); - } else { - // Use notification guard, otherwise will generate one notification - // for the xform op add, and another for the reorder. - UsdUfe::InTransform3dChange guard(cmd.path()); - UsdGeomXformable xformable(usdSceneItem.item().prim()); - - auto r = xformable.AddRotateXYZOp(UsdGeomXformOp::PrecisionFloat, opSuffix); - if (!r) { - throw std::runtime_error("Cannot add rotation transform operation"); - } - r.Set(v); - if (!setXformOpOrderFn(xformable)) { - throw std::runtime_error("Cannot set rotation transform operation"); - } - - return r; - } - }); + auto f + = OpFunc([attrName, opSuffix = getTRSOpSuffix(), setXformOpOrderFn = getXformOpOrderFn()]( + const BaseUndoableCommand& cmd, UsdUndoableItem& undoableItem) { + SceneItemHolder usdSceneItem(cmd); + + auto attr = getUsdPrimAttribute(usdSceneItem.item().prim(), attrName); + if (attr) { + return UsdGeomXformOp(attr); + } else { + UsdUndoBlock undoBlock(&undoableItem); + + // Use notification guard, otherwise will generate one notification + // for the xform op add, and another for the reorder. + UsdUfe::InTransform3dChange guard(cmd.path()); + UsdGeomXformable xformable(usdSceneItem.item().prim()); + + auto r = xformable.AddRotateXYZOp(UsdGeomXformOp::PrecisionFloat, opSuffix); + if (!r) { + throw std::runtime_error("Cannot add rotation transform operation"); + } + if (!setXformOpOrderFn(xformable)) { + throw std::runtime_error("Cannot set rotation transform operation"); + } + + return r; + } + }); return std::make_shared( v, path(), std::move(f), cvt, UsdTimeCode::Default()); @@ -527,30 +474,31 @@ Ufe::ScaleUndoableCommand::Ptr UsdTransform3dMayaXformStack::scaleCmd(double x, } GfVec3f v(x, y, z); - auto f = OpFunc( - [attrName, opSuffix = getTRSOpSuffix(), setXformOpOrderFn = getXformOpOrderFn(), v]( - const BaseUndoableCommand& cmd) { - SceneItemHolder usdSceneItem(cmd); - - auto attr = getUsdPrimAttribute(usdSceneItem.item().prim(), attrName); - if (attr) { - return UsdGeomXformOp(attr); - } else { - UsdUfe::InTransform3dChange guard(cmd.path()); - UsdGeomXformable xformable(usdSceneItem.item().prim()); - - auto s = xformable.AddScaleOp(UsdGeomXformOp::PrecisionFloat, opSuffix); - if (!s) { - throw std::runtime_error("Cannot add scaling transform operation"); - } - s.Set(v); - if (!setXformOpOrderFn(xformable)) { - throw std::runtime_error("Cannot set scaling transform operation"); - } - - return s; - } - }); + auto f + = OpFunc([attrName, opSuffix = getTRSOpSuffix(), setXformOpOrderFn = getXformOpOrderFn()]( + const BaseUndoableCommand& cmd, UsdUndoableItem& undoableItem) { + SceneItemHolder usdSceneItem(cmd); + + auto attr = getUsdPrimAttribute(usdSceneItem.item().prim(), attrName); + if (attr) { + return UsdGeomXformOp(attr); + } else { + UsdUndoBlock undoBlock(&undoableItem); + + UsdUfe::InTransform3dChange guard(cmd.path()); + UsdGeomXformable xformable(usdSceneItem.item().prim()); + + auto s = xformable.AddScaleOp(UsdGeomXformOp::PrecisionFloat, opSuffix); + if (!s) { + throw std::runtime_error("Cannot add scaling transform operation"); + } + if (!setXformOpOrderFn(xformable)) { + throw std::runtime_error("Cannot set scaling transform operation"); + } + + return s; + } + }); return std::make_shared>( v, path(), std::move(f), UsdTimeCode::Default()); @@ -642,20 +590,22 @@ Ufe::SetVector3dUndoableCommand::Ptr UsdTransform3dMayaXformStack::setVector3dCm // work around by calling in function body. PPT, 11-Jan-2021. // [opSuffix, setXformOpOrderFn = getXformOpOrderFn(), v](const BaseUndoableCommand& // cmd) { - [attrName, opSuffix, setXformOpOrderFn, v](const BaseUndoableCommand& cmd) { + [attrName, opSuffix, setXformOpOrderFn]( + const BaseUndoableCommand& cmd, UsdUndoableItem& undoableItem) { SceneItemHolder usdSceneItem(cmd); auto attr = getUsdPrimAttribute(usdSceneItem.item().prim(), attrName); if (attr) { return UsdGeomXformOp(attr); } else { + UsdUndoBlock undoBlock(&undoableItem); + UsdUfe::InTransform3dChange guard(cmd.path()); UsdGeomXformable xformable(usdSceneItem.item().prim()); auto op = xformable.AddTranslateOp(OpPrecision::precision, opSuffix); if (!op) { throw std::runtime_error("Cannot add translation transform operation"); } - op.Set(v); if (!setXformOpOrderFn(xformable)) { throw std::runtime_error("Cannot set translation transform operation"); } @@ -680,8 +630,8 @@ UsdTransform3dMayaXformStack::pivotCmd(const TfToken& pvtOpSuffix, double x, dou } GfVec3f v(x, y, z); - auto f = OpFunc([pvtAttrName, pvtOpSuffix, setXformOpOrderFn = getXformOpOrderFn(), v]( - const BaseUndoableCommand& cmd) { + auto f = OpFunc([pvtAttrName, pvtOpSuffix, setXformOpOrderFn = getXformOpOrderFn()]( + const BaseUndoableCommand& cmd, UsdUndoableItem& undoableItem) { SceneItemHolder usdSceneItem(cmd); auto attr = usdSceneItem.item().prim().GetAttribute(pvtAttrName); @@ -696,6 +646,8 @@ UsdTransform3dMayaXformStack::pivotCmd(const TfToken& pvtOpSuffix, double x, dou // its inverse is added --- this does not match the Maya transform // stack. Use of SdfChangeBlock is discouraged when calling USD // APIs above Sdf, so use our own guard. + + UsdUndoBlock undoBlock(&undoableItem); UsdUfe::InTransform3dChange guard(cmd.path()); UsdGeomXformable xformable(usdSceneItem.item().prim()); auto p = xformable.AddTranslateOp(UsdGeomXformOp::PrecisionFloat, pvtOpSuffix); @@ -705,7 +657,6 @@ UsdTransform3dMayaXformStack::pivotCmd(const TfToken& pvtOpSuffix, double x, dou if (!(p && pInv)) { throw std::runtime_error("Cannot add translation transform operation"); } - p.Set(v); if (!setXformOpOrderFn(xformable)) { throw std::runtime_error("Cannot set translation transform operation"); } diff --git a/test/lib/ufe/testTRSBase.py b/test/lib/ufe/testTRSBase.py index d3eb4e092a..8d840fbedb 100644 --- a/test/lib/ufe/testTRSBase.py +++ b/test/lib/ufe/testTRSBase.py @@ -72,9 +72,9 @@ def fforwardMemento(self): # For a memento list of length n, n-1 redo operations sets us current. self.assertEqual(self.memento[0], self.snapshotRunTimeUFE()) # Skip first - for m in self.memento[1:]: + for i in range(1, len(self.memento)): cmds.redo() - self.assertEqual(m, self.snapshotRunTimeUFE()) + self.assertEqual(self.memento[i], self.snapshotRunTimeUFE(), 'Differing for memento %s of %s' % (i, len(self.memento))) def multiSelectRewindMemento(self, items): '''Undo through all items in memento.