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

Show file tree
Hide file tree
Changes from all commits
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
102 changes: 49 additions & 53 deletions lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,28 @@
//

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

#include <ufe/log.h>
#include <ufe/scene.h>
#include <ufe/sceneNotification.h>
#include <ufe/log.h>

#include <pxr/base/tf/token.h>
#include <pxr/usd/sdf/copyUtils.h>
#include <pxr/usd/usd/editContext.h>
#include <pxr/usd/usd/stage.h>

#include <mayaUsdUtils/util.h>

#ifdef UFE_V2_FEATURES_AVAILABLE
#define UFE_ENABLE_ASSERTS
#include <ufe/ufeAssert.h>
#else
#include <cassert>
#endif

#include <mayaUsdUtils/util.h>

#include <pxr/usd/usd/stage.h>
#include <pxr/usd/usd/editContext.h>
#include <pxr/usd/sdf/copyUtils.h>
#include <pxr/base/tf/token.h>

namespace {
// shared_ptr requires public ctor, dtor, so derive a class for it.
template<class T>
Expand All @@ -50,61 +52,55 @@ struct MakeSharedEnabler : public T {
MAYAUSD_NS_DEF {
namespace ufe {

UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(
const UsdSceneItem::Ptr& parent,
const UsdSceneItem::Ptr& child,
const UsdSceneItem::Ptr& /* pos */
) : Ufe::UndoableCommand()
UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& parent,
const UsdSceneItem::Ptr& child,
const UsdSceneItem::Ptr& /* pos */)
: Ufe::UndoableCommand()
, _stage(child->prim().GetStage())
, _ufeSrcItem(child)
, _usdSrcPath(child->prim().GetPath())
{
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.
auto childName = uniqueChildName(parent, child->path());
const auto& childName = uniqueChildName(parent, child->path());

// Set up all paths to perform the reparent.
auto childPrim = child->prim();
fStage = childPrim.GetStage();
fUfeSrcItem = child;
fUsdSrcPath = childPrim.GetPath();
// Create a new segment if parent and child are in different run-times.
// parenting a USD node to the proxy shape node implies two different run-times
auto cRtId = child->path().runTimeId();
if (parent->path().runTimeId() == cRtId) {
fUfeDstPath = parent->path() + childName;
_ufeDstPath = parent->path() + childName;
}
else {
auto cSep = child->path().getSegments().back().separator();
fUfeDstPath = parent->path() + Ufe::PathSegment(
_ufeDstPath = parent->path() + Ufe::PathSegment(
Ufe::PathComponent(childName), cRtId, cSep);
}
fUsdDstPath = parent->prim().GetPath().AppendChild(TfToken(childName));
_usdDstPath = parent->prim().GetPath().AppendChild(TfToken(childName));

fChildLayer = MayaUsdUtils::defPrimSpecLayer(childPrim);
if (!fChildLayer) {
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();

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

if (!fParentLayer) {
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);
}

UsdUndoInsertChildCommand::~UsdUndoInsertChildCommand()
{
}

/*static*/
UsdUndoInsertChildCommand::Ptr UsdUndoInsertChildCommand::create(
const UsdSceneItem::Ptr& parent,
const UsdSceneItem::Ptr& child,
const UsdSceneItem::Ptr& pos
)
UsdUndoInsertChildCommand::Ptr
UsdUndoInsertChildCommand::create(const UsdSceneItem::Ptr& parent,
const UsdSceneItem::Ptr& child,
const UsdSceneItem::Ptr& pos)
{
// Error if requested parent is currently a child of requested child.
if (parent->path().startsWith(child->path())) {
Expand All @@ -117,10 +113,10 @@ UsdUndoInsertChildCommand::Ptr UsdUndoInsertChildCommand::create(
bool UsdUndoInsertChildCommand::insertChildRedo()
{
// See comments in UsdUndoRenameCommand.cpp.
bool status = SdfCopySpec(fChildLayer, fUsdSrcPath, fParentLayer, fUsdDstPath);
bool status = SdfCopySpec(_childLayer, _usdSrcPath, _parentLayer, _usdDstPath);
if (status)
{
auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath);
auto srcPrim = _stage->GetPrimAtPath(_usdSrcPath);
#ifdef UFE_V2_FEATURES_AVAILABLE
UFE_ASSERT_MSG(srcPrim, "Invalid prim cannot be inactivated.");
#else
Expand All @@ -129,14 +125,14 @@ bool UsdUndoInsertChildCommand::insertChildRedo()
status = srcPrim.SetActive(false);

if (status) {
fUfeDstItem = UsdSceneItem::create(fUfeDstPath, ufePathToPrim(fUfeDstPath));
_ufeDstItem = UsdSceneItem::create(_ufeDstPath, ufePathToPrim(_ufeDstPath));

sendNotification<Ufe::ObjectReparent>(fUfeDstItem, fUfeSrcItem->path());
sendNotification<Ufe::ObjectReparent>(_ufeDstItem, _ufeSrcItem->path());
}
}
else {
UFE_LOG(std::string("Warning: SdfCopySpec(") +
fUsdSrcPath.GetString() + std::string(") failed."));
_usdSrcPath.GetString() + std::string(") failed."));
}

return status;
Expand All @@ -149,11 +145,11 @@ bool UsdUndoInsertChildCommand::insertChildUndo()
// Regardless of where the edit target is currently set, switch to the
// layer where we copied the source prim into the destination, then
// restore the edit target.
UsdEditContext ctx(fStage, fParentLayer);
status = fStage->RemovePrim(fUsdDstPath);
UsdEditContext ctx(_stage, _parentLayer);
status = _stage->RemovePrim(_usdDstPath);
}
if (status) {
auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath);
auto srcPrim = _stage->GetPrimAtPath(_usdSrcPath);
#ifdef UFE_V2_FEATURES_AVAILABLE
UFE_ASSERT_MSG(srcPrim, "Invalid prim cannot be activated.");
#else
Expand All @@ -163,14 +159,14 @@ bool UsdUndoInsertChildCommand::insertChildUndo()

if (status) {

sendNotification<Ufe::ObjectReparent>(fUfeSrcItem, fUfeDstItem->path());
sendNotification<Ufe::ObjectReparent>(_ufeSrcItem, _ufeDstItem->path());

fUfeDstItem = nullptr;
_ufeDstItem = nullptr;
}
}
else {
UFE_LOG(std::string("Warning: RemovePrim(") +
fUsdDstPath.GetString() + std::string(") failed."));
_usdDstPath.GetString() + std::string(") failed."));
}

return status;
Expand Down
45 changes: 23 additions & 22 deletions lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
//
#pragma once

#include <mayaUsd/base/api.h>
#include <mayaUsd/ufe/UsdSceneItem.h>
#include <ufe/undoableCommand.h>

#include <pxr/usd/usd/prim.h>

#include <ufe/undoableCommand.h>
#include <mayaUsd/base/api.h>
#include <mayaUsd/ufe/UsdSceneItem.h>

PXR_NAMESPACE_USING_DIRECTIVE

Expand All @@ -31,31 +31,28 @@ namespace ufe {
class MAYAUSD_CORE_PUBLIC UsdUndoInsertChildCommand : public Ufe::UndoableCommand
{
public:
typedef std::shared_ptr<UsdUndoInsertChildCommand> Ptr;
using Ptr = std::shared_ptr<UsdUndoInsertChildCommand>;

~UsdUndoInsertChildCommand() override;

// Delete the copy/move constructors assignment operators.
UsdUndoInsertChildCommand(const UsdUndoInsertChildCommand&) = delete;
UsdUndoInsertChildCommand& operator=(const UsdUndoInsertChildCommand&) = delete;
UsdUndoInsertChildCommand(UsdUndoInsertChildCommand&&) = delete;
UsdUndoInsertChildCommand& operator=(UsdUndoInsertChildCommand&&) = delete;

//! Create a UsdUndoInsertChildCommand. Note that as of 4-May-2020 the
//! pos argument is ignored, and only append is supported.
static UsdUndoInsertChildCommand::Ptr create(
const UsdSceneItem::Ptr& parent,
const UsdSceneItem::Ptr& child,
const UsdSceneItem::Ptr& pos
);
static UsdUndoInsertChildCommand::Ptr create(const UsdSceneItem::Ptr& parent,
const UsdSceneItem::Ptr& child,
const UsdSceneItem::Ptr& pos);

protected:
//! Construct a UsdUndoInsertChildCommand. Note that as of 4-May-2020 the
//! pos argument is ignored, and only append is supported.
UsdUndoInsertChildCommand(
const UsdSceneItem::Ptr& parent,
const UsdSceneItem::Ptr& child,
const UsdSceneItem::Ptr& pos
);
UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& parent,
const UsdSceneItem::Ptr& child,
const UsdSceneItem::Ptr& pos);

private:
void undo() override;
Expand All @@ -64,14 +61,18 @@ class MAYAUSD_CORE_PUBLIC UsdUndoInsertChildCommand : public Ufe::UndoableComman
bool insertChildRedo();
bool insertChildUndo();

UsdStageWeakPtr fStage;
SdfLayerHandle fChildLayer;
SdfLayerHandle fParentLayer;
UsdSceneItem::Ptr fUfeSrcItem;
SdfPath fUsdSrcPath;
UsdSceneItem::Ptr fUfeDstItem;
Ufe::Path fUfeDstPath;
SdfPath fUsdDstPath;
UsdStageWeakPtr _stage;

UsdSceneItem::Ptr _ufeSrcItem;
UsdSceneItem::Ptr _ufeDstItem;

SdfPath _usdSrcPath;
SdfPath _usdDstPath;

SdfLayerHandle _childLayer;
SdfLayerHandle _parentLayer;

Ufe::Path _ufeDstPath;

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 this class according to our guidelines.

}; // UsdUndoInsertChildCommand

Expand Down
Loading