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.

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

MAYA-111834 - Crash when transforming multiple objects with duplicate stage #1554

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
323 changes: 152 additions & 171 deletions lib/mayaUsd/ufe/UsdTransform3dMayaXformStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,111 +433,97 @@ UsdTransform3dMayaXformStack::translateCmd(double x, double y, double z)
Ufe::RotateUndoableCommand::Ptr
UsdTransform3dMayaXformStack::rotateCmd(double x, double y, double z)
{
GfVec3f v(x, y, z);
UsdGeomXformOp op;
TfToken attrName;
const bool hasRotate = hasOp(NdxRotate);
if (hasRotate) {
op = getOp(NdxRotate);
attrName = op.GetOpName();
}

// Return null command if the attribute edit is not allowed.
std::string errMsg;
if (!isAttributeEditAllowed(attrName, errMsg)) {
MGlobal::displayError(errMsg.c_str());
return nullptr;
}

// If there is no rotate transform op, we will create a RotXYZ.
GfVec3f v(x, y, z);
CvtRotXYZToAttrFn cvt = hasRotate ? getCvtRotXYZToAttrFn(op.GetOpName()) : toXYZ;
OpFunc f = hasRotate
? OpFunc([attrName](const BaseUndoableCommand& cmd) {
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
auto attr = usdSceneItem->prim().GetAttribute(attrName);

// return an invalid XformOp if the attribute edit is not allowed.
std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(attr, &errMsg)) {
MGlobal::displayError(errMsg.c_str());
return UsdGeomXformOp();
}

return UsdGeomXformOp(attr);
})
: OpFunc([opSuffix = getTRSOpSuffix(), setXformOpOrderFn = getXformOpOrderFn(), v](
const BaseUndoableCommand& cmd) {
// Use notification guard, otherwise will generate one notification
// for the xform op add, and another for the reorder.
InTransform3dChange guard(cmd.path());
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
UsdGeomXformable xformable(usdSceneItem->prim());

// At this point we already know that writing to attribute is going
// to succeed but we do not know if writing to "transform op order" succeed since
// we could be a weaker layer.
std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(xformable.GetXformOpOrderAttr(), &errMsg)) {
MGlobal::displayError(errMsg.c_str());
return UsdGeomXformOp();
}

auto r = xformable.AddRotateXYZOp(UsdGeomXformOp::PrecisionFloat, opSuffix);
TF_AXIOM(r);
r.Set(v);
auto result = setXformOpOrderFn(xformable);
TF_AXIOM(result);

return r;
});

auto f = OpFunc(
[attrName, opSuffix = getTRSOpSuffix(), setXformOpOrderFn = getXformOpOrderFn(), v](
const BaseUndoableCommand& cmd) {
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
auto attr = usdSceneItem->prim().GetAttribute(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.
InTransform3dChange guard(cmd.path());
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
UsdGeomXformable xformable(usdSceneItem->prim());

auto r = xformable.AddRotateXYZOp(UsdGeomXformOp::PrecisionFloat, opSuffix);
TF_AXIOM(r);
r.Set(v);
auto result = setXformOpOrderFn(xformable);
TF_AXIOM(result);

return r;
}
});
Comment on lines +455 to +479
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Combined two lambda OpFunc into one. First check if the prim already has the attribute. If yes, then use it to create UsdGeomXformOp. Otherwise first add the attribute and then use the newly added one. Same changes made in all the command methods.


return std::make_shared<UsdRotateOpUndoableCmd>(
v, path(), std::move(f), cvt, UsdTimeCode::Default());
}

Ufe::ScaleUndoableCommand::Ptr UsdTransform3dMayaXformStack::scaleCmd(double x, double y, double z)
{
GfVec3f v(x, y, z);
UsdGeomXformOp op;
TfToken attrName;
const bool hasScale = hasOp(NdxScale);
if (hasScale) {
if (hasOp(NdxScale)) {
op = getOp(NdxScale);
attrName = op.GetOpName();
}
OpFunc f = hasScale
? OpFunc([attrName](const BaseUndoableCommand& cmd) {
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
auto attr = usdSceneItem->prim().GetAttribute(attrName);

// return an invalid XformOp if the attribute edit is not allowed.
std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(attr, &errMsg)) {
MGlobal::displayError(errMsg.c_str());
return UsdGeomXformOp();
}

return UsdGeomXformOp(attr);
})
: OpFunc([opSuffix = getTRSOpSuffix(), setXformOpOrderFn = getXformOpOrderFn(), v](
const BaseUndoableCommand& cmd) {
InTransform3dChange guard(cmd.path());
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
UsdGeomXformable xformable(usdSceneItem->prim());

// At this point we already know that writing to attribute is going
// to succeed but we do not know if writing to "transform op order" succeed since
// we could be a weaker layer.
std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(xformable.GetXformOpOrderAttr(), &errMsg)) {
MGlobal::displayError(errMsg.c_str());
return UsdGeomXformOp();
}

auto s = xformable.AddScaleOp(UsdGeomXformOp::PrecisionFloat, opSuffix);
TF_AXIOM(s);
s.Set(v);
auto result = setXformOpOrderFn(xformable);
TF_AXIOM(result);

return s;
});

// Return null command if the attribute edit is not allowed.
std::string errMsg;
if (!isAttributeEditAllowed(attrName, errMsg)) {
MGlobal::displayError(errMsg.c_str());
return nullptr;
}

GfVec3f v(x, y, z);
auto f = OpFunc(
[attrName, opSuffix = getTRSOpSuffix(), setXformOpOrderFn = getXformOpOrderFn(), v](
const BaseUndoableCommand& cmd) {
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);

auto attr = usdSceneItem->prim().GetAttribute(attrName);
if (attr) {
return UsdGeomXformOp(attr);
} else {

InTransform3dChange guard(cmd.path());
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
UsdGeomXformable xformable(usdSceneItem->prim());

auto s = xformable.AddScaleOp(UsdGeomXformOp::PrecisionFloat, opSuffix);
TF_AXIOM(s);
s.Set(v);
auto result = setXformOpOrderFn(xformable);
TF_AXIOM(result);

return s;
}
});

return std::make_shared<UsdVecOpUndoableCmd<GfVec3f>>(
v, path(), std::move(f), UsdTimeCode::Default());
Expand Down Expand Up @@ -618,53 +604,39 @@ Ufe::SetVector3dUndoableCommand::Ptr UsdTransform3dMayaXformStack::setVector3dCm
const TfToken& attrName,
const TfToken& opSuffix)
{
auto attr = prim().GetAttribute(attrName);
auto setXformOpOrderFn = getXformOpOrderFn();
OpFunc f = attr
? OpFunc([attrName](const BaseUndoableCommand& cmd) {
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
auto attr = usdSceneItem->prim().GetAttribute(attrName);

// return an invalid XformOp if the attribute edit is not allowed.
std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(attr, &errMsg)) {
MGlobal::displayError(errMsg.c_str());
return UsdGeomXformOp();
}

return UsdGeomXformOp(attr);
})
: OpFunc(
// MAYA-108612: generalized lambda capture below is incorrect with
// gcc 6.3.1 on Linux. Call to getXformOpOrderFn() is non-virtual;
// work around by calling in function body. PPT, 11-Jan-2021.
// [opSuffix, setXformOpOrderFn = getXformOpOrderFn(), v](const BaseUndoableCommand&
// cmd) {
[opSuffix, setXformOpOrderFn, v](const BaseUndoableCommand& cmd) {
InTransform3dChange guard(cmd.path());
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
UsdGeomXformable xformable(usdSceneItem->prim());

// At this point we already know that writing to attribute is going
// to succeed but we do not know if writing to "transform op order" succeed
// since we could be a weaker layer.
std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(
xformable.GetXformOpOrderAttr(), &errMsg)) {
MGlobal::displayError(errMsg.c_str());
return UsdGeomXformOp();
}
// Return null command if the attribute edit is not allowed.
std::string errMsg;
if (!isAttributeEditAllowed(attrName, errMsg)) {
MGlobal::displayError(errMsg.c_str());
return nullptr;
}

auto setXformOpOrderFn = getXformOpOrderFn();
auto f = OpFunc(
// MAYA-108612: generalized lambda capture below is incorrect with
// gcc 6.3.1 on Linux. Call to getXformOpOrderFn() is non-virtual;
// 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) {
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);

auto attr = usdSceneItem->prim().GetAttribute(attrName);
if (attr) {
return UsdGeomXformOp(attr);
} else {
InTransform3dChange guard(cmd.path());
UsdGeomXformable xformable(usdSceneItem->prim());
auto op = xformable.AddTranslateOp(OpPrecision<V>::precision, opSuffix);
TF_AXIOM(op);
op.Set(v);
auto result = setXformOpOrderFn(xformable);
TF_AXIOM(result);

return op;
});
}
});

return std::make_shared<UsdVecOpUndoableCmd<V>>(
v, path(), std::move(f), UsdTimeCode::Default());
Expand All @@ -675,55 +647,46 @@ UsdTransform3dMayaXformStack::pivotCmd(const TfToken& pvtOpSuffix, double x, dou
{
auto pvtAttrName = UsdGeomXformOp::GetOpName(UsdGeomXformOp::TypeTranslate, pvtOpSuffix);

// Return null command if the attribute edit is not allowed.
std::string errMsg;
if (!isAttributeEditAllowed(pvtAttrName, errMsg)) {
MGlobal::displayError(errMsg.c_str());
return nullptr;
}

GfVec3f v(x, y, z);
auto attr = prim().GetAttribute(pvtAttrName);
OpFunc f = attr
? OpFunc([pvtAttrName](const BaseUndoableCommand& cmd) {
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
auto attr = usdSceneItem->prim().GetAttribute(pvtAttrName);

// return an invalid XformOp if the attribute edit is not allowed.
std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(attr, &errMsg)) {
MGlobal::displayError(errMsg.c_str());
return UsdGeomXformOp();
}

return UsdGeomXformOp(attr);
})
: OpFunc([pvtOpSuffix, setXformOpOrderFn = getXformOpOrderFn(), v](
const BaseUndoableCommand& cmd) {
// Without a notification guard each operation (each transform op
// addition, setting the attribute value, and setting the transform
// op order) will notify. Observers would see an object in an
// inconsistent state, especially after pivot is added but before
// 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.
InTransform3dChange guard(cmd.path());
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
UsdGeomXformable xformable(usdSceneItem->prim());
auto p = xformable.AddTranslateOp(UsdGeomXformOp::PrecisionFloat, pvtOpSuffix);

// At this point we already know that writing to attribute is going
// to succeed but we do not know if writing to "transform op order" succeed since
// we could be a weaker layer.
std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(xformable.GetXformOpOrderAttr(), &errMsg)) {
MGlobal::displayError(errMsg.c_str());
return UsdGeomXformOp();
}

auto pInv = xformable.AddTranslateOp(
UsdGeomXformOp::PrecisionFloat, pvtOpSuffix, /* isInverseOp */ true);
TF_AXIOM(p && pInv);
p.Set(v);
auto result = setXformOpOrderFn(xformable);
TF_AXIOM(result);
return p;
});
auto f = OpFunc([pvtAttrName, pvtOpSuffix, setXformOpOrderFn = getXformOpOrderFn(), v](
const BaseUndoableCommand& cmd) {
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);

auto attr = usdSceneItem->prim().GetAttribute(pvtAttrName);
if (attr) {
auto attr = usdSceneItem->prim().GetAttribute(pvtAttrName);
return UsdGeomXformOp(attr);
} else {
// Without a notification guard each operation (each transform op
// addition, setting the attribute value, and setting the transform
// op order) will notify. Observers would see an object in an
// inconsistent state, especially after pivot is added but before
// 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.
InTransform3dChange guard(cmd.path());
auto usdSceneItem = std::dynamic_pointer_cast<UsdSceneItem>(cmd.sceneItem());
TF_AXIOM(usdSceneItem);
UsdGeomXformable xformable(usdSceneItem->prim());
auto p = xformable.AddTranslateOp(UsdGeomXformOp::PrecisionFloat, pvtOpSuffix);

auto pInv = xformable.AddTranslateOp(
UsdGeomXformOp::PrecisionFloat, pvtOpSuffix, /* isInverseOp */ true);
TF_AXIOM(p && pInv);
p.Set(v);
auto result = setXformOpOrderFn(xformable);
TF_AXIOM(result);
return p;
}
});

return std::make_shared<UsdVecOpUndoableCmd<GfVec3f>>(
v, path(), std::move(f), UsdTimeCode::Default());
Expand Down Expand Up @@ -808,6 +771,24 @@ UsdTransform3dMayaXformStack::getCvtRotXYZToAttrFn(const TfToken& opName) const
return cvt.at(opName);
}

bool UsdTransform3dMayaXformStack::isAttributeEditAllowed(
const PXR_NS::TfToken attrName,
std::string& errMsg) const
{
UsdAttribute attr;
if (!attrName.IsEmpty())
attr = prim().GetAttribute(attrName);
if (attr && !MayaUsd::ufe::isAttributeEditAllowed(attr, &errMsg)) {
return false;
} else if (!attr) {
UsdGeomXformable xformable(prim());
if (!MayaUsd::ufe::isAttributeEditAllowed(xformable.GetXformOpOrderAttr(), &errMsg)) {
return false;
}
}
return true;
}

//------------------------------------------------------------------------------
// UsdTransform3dMayaXformStackHandler
//------------------------------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions lib/mayaUsd/ufe/UsdTransform3dMayaXformStack.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ class MAYAUSD_CORE_PUBLIC UsdTransform3dMayaXformStack : public UsdTransform3dBa
private:
Ufe::TranslateUndoableCommand::Ptr
pivotCmd(const PXR_NS::TfToken& pvtOpSuffix, double x, double y, double z);

bool isAttributeEditAllowed(const PXR_NS::TfToken attrName, std::string& errMsg) const;
}; // UsdTransform3dMayaXformStack

//! \brief Factory to create a UsdTransform3dMayaXformStack interface object.
Expand Down