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.

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

Apply Restriction Rules for Parenting #673

Merged
merged 6 commits into from
Jul 28, 2020
Merged

Apply Restriction Rules for Parenting #673

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c358550
Clean up member variable names, header orders, etc.. according to gui…
Jul 20, 2020
0202eec
Apply restriction rules for child and parent prims before performing …
Jul 20, 2020
baf1595
Fix failing unit tests ( testGroupCmd, testParentCmd ).
Jul 21, 2020
23d2f9b
put back the test codes that I removed earlier.
Jul 21, 2020
54ed18a
Make MacOS/Linux compiler happy by changing the initializing order .
Jul 21, 2020
e123bad
change required version to 2018.
Jul 28, 2020
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
22 changes: 9 additions & 13 deletions lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "UsdUndoInsertChildCommand.h"
#include "private/InPathChange.h"
#include "private/Utils.h"
#include "Utils.h"

#include <ufe/log.h>
Expand Down Expand Up @@ -62,6 +63,10 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& pa
const auto& childPrim = child->prim();
const auto& parentPrim = parent->prim();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cleaned up initializing class member variables.

// Apply restriction rules
ufe::applyCommandRestriction(childPrim, "reparent");
ufe::applyCommandRestriction(parentPrim, "reparent");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ppt-adsk here I am applying the restriction rules very early. These rules are applied both to the child and parent prims.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really the only change/fix here? Other than name changes I see a couple places where we've removed nullptr checks and then this is the only real change, do I have that right?

Copy link
Contributor Author

@HamedSabri-adsk HamedSabri-adsk Jul 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hey @fowlertADSK I have broken down my PR into 3 parts to make it easier to review:

1- c358550
2- 0202eec
3- baf1595

The 2 line changes that you pointed out are part of section 2. There are also 2 important changes in section 2 which are related to cleaning up _childLayer and _parentLayer.

Section 3 (Fix failing unit tests) is what affects your earlier changes. If I recall correctly you told me last time that you were planning to make changes to testGroupCmd. I wanted to make sure we are aligned on this.

// First, check if we need to rename the child.
const auto& childName = uniqueChildName(parent, child->path());

Expand All @@ -78,22 +83,13 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& pa
}
_usdDstPath = parent->prim().GetPath().AppendChild(TfToken(childName));

_childLayer = MayaUsdUtils::defPrimSpecLayer(childPrim);
if (!_childLayer) {
std::string err = TfStringPrintf("No child prim found at %s", childPrim.GetPath().GetString().c_str());
throw std::runtime_error(err.c_str());
}
_childLayer = _stage->GetEditTarget().GetLayer();

// If parent prim is the pseudo-root, no def primSpec will be found, so
// just use the edit target layer.
_parentLayer = parentPrim.IsPseudoRoot() ?
_stage->GetEditTarget().GetLayer() :
MayaUsdUtils::defPrimSpecLayer(parentPrim);

if (!_parentLayer) {
std::string err = TfStringPrintf("No parent prim found at %s", parentPrim.GetPath().GetString().c_str());
throw std::runtime_error(err.c_str());
}
_parentLayer = parentPrim.IsPseudoRoot()
? _stage->GetEditTarget().GetLayer()
: MayaUsdUtils::defPrimSpecLayer(parentPrim);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both "_childLayer" and "_parentLayer" should be now in the correct target layers, therefore they can simply take "_stage->GetEditTarget().GetLayer()".

@ppt-adsk does "_parentLayer" still need to check for "parentPrim.IsPseudoRoot()"? I would like to understand when this happens.


UsdUndoInsertChildCommand::~UsdUndoInsertChildCommand()
Expand Down