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

Making visibility Undo/Redo correct. #1056

Merged
merged 9 commits into from
Jan 13, 2021
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
2 changes: 2 additions & 0 deletions lib/mayaUsd/ufe/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ if(CMAKE_UFE_V2_FEATURES_AVAILABLE)
UsdUndoCreateGroupCommand.cpp
UsdUndoInsertChildCommand.cpp
UsdUndoReorderCommand.cpp
UsdUndoVisibleCommand.cpp
)
if(UFE_PREVIEW_VERSION_NUM GREATER_EQUAL 2022)
target_sources(${PROJECT_NAME}
Expand Down Expand Up @@ -107,6 +108,7 @@ if(CMAKE_UFE_V2_FEATURES_AVAILABLE)
UsdUndoCreateGroupCommand.h
UsdUndoInsertChildCommand.h
UsdUndoReorderCommand.h
UsdUndoVisibleCommand.h
)
endif()

Expand Down
18 changes: 16 additions & 2 deletions lib/mayaUsd/ufe/UsdContextOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

#include "private/UfeNotifGuard.h"

#if UFE_PREVIEW_VERSION_NUM >= 2034
#include <mayaUsd/ufe/UsdObject3d.h>
#endif
#include <mayaUsd/ufe/UsdSceneItem.h>
#include <mayaUsd/ufe/UsdUndoAddNewPrimCommand.h>
#include <mayaUsd/ufe/Utils.h>
Expand All @@ -35,6 +38,9 @@
#include <maya/MGlobal.h>
#include <ufe/attribute.h>
#include <ufe/attributes.h>
#if UFE_PREVIEW_VERSION_NUM >= 2034
#include <ufe/object3d.h>
#endif
#include <ufe/path.h>

#include <algorithm>
Expand Down Expand Up @@ -567,15 +573,23 @@ Ufe::UndoableCommand::Ptr UsdContextOps::doOpCmd(const ItemPath& itemPath)
return std::make_shared<SetVariantSelectionUndoableCommand>(prim(), itemPath);
} // Variant sets
else if (itemPath[0] == kUSDToggleVisibilityItem) {
#if UFE_PREVIEW_VERSION_NUM < 2034
auto attributes = Ufe::Attributes::attributes(sceneItem());
assert(attributes);
TF_AXIOM(attributes);
auto visibility = std::dynamic_pointer_cast<Ufe::AttributeEnumString>(
attributes->attribute(UsdGeomTokens->visibility));
assert(visibility);
TF_AXIOM(visibility);
auto current = visibility->get();
return visibility->setCmd(
current == UsdGeomTokens->invisible ? UsdGeomTokens->inherited
: UsdGeomTokens->invisible);
#else
auto object3d = UsdObject3d::create(fItem);
TF_AXIOM(object3d);
auto current = object3d->visibility();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we check that the scene item supports the Object3d interface? At this point it should, because we put up the menu entry for it, so perhaps we can just assert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A couple of things to fix, but really what I feel is missing from this PR are the automated tests. You can write an automated test that first tests the UsdUndoVisibleCommand, and calls undo and redo on that object, for the lowest-level test.

Great point. I am working on it as we speak.

Addressed in 7004631

Copy link
Contributor Author

@HamedSabri-adsk HamedSabri-adsk Jan 11, 2021

Choose a reason for hiding this comment

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

@ppt-adsk @seando-adsk

Added a test for backing the new UsdUndoVisibleCommand :

865d479

Added a new test for backing up the UFE command integration on Maya side for both "hide" and "showHidden" commands :

43bf1b1

return object3d->setVisibleCmd(!current);
#endif

} // Visibility
else if (itemPath[0] == kUSDToggleActiveStateItem) {
return std::make_shared<ToggleActiveStateCommand>(prim());
Expand Down
11 changes: 11 additions & 0 deletions lib/mayaUsd/ufe/UsdObject3d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
//
#include "UsdObject3d.h"

#if UFE_PREVIEW_VERSION_NUM >= 2034
#include <mayaUsd/ufe/UsdUndoVisibleCommand.h>
#endif

#include <mayaUsd/ufe/Utils.h>

#include <pxr/usd/usd/timeCode.h>
Expand Down Expand Up @@ -92,5 +96,12 @@ void UsdObject3d::setVisibility(bool vis)
vis ? UsdGeomImageable(fPrim).MakeVisible() : UsdGeomImageable(fPrim).MakeInvisible();
}

#if UFE_PREVIEW_VERSION_NUM >= 2034
Ufe::UndoableCommand::Ptr UsdObject3d::setVisibleCmd(bool vis)
{
return UsdUndoVisibleCommand::create(fPrim, vis);
}
#endif

} // namespace ufe
} // namespace MAYAUSD_NS_DEF
4 changes: 4 additions & 0 deletions lib/mayaUsd/ufe/UsdObject3d.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ class MAYAUSD_CORE_PUBLIC UsdObject3d : public Ufe::Object3d
bool visibility() const override;
void setVisibility(bool vis) override;

#if UFE_PREVIEW_VERSION_NUM >= 2034
Ufe::UndoableCommand::Ptr setVisibleCmd(bool vis) override;
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added guard around "setVisibleCmd" since this feature is available in UFE V.2034 or later.

private:
UsdSceneItem::Ptr fItem;
UsdPrim fPrim;
Expand Down
54 changes: 54 additions & 0 deletions lib/mayaUsd/ufe/UsdUndoVisibleCommand.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//
// Copyright 2020 Autodesk
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
#include "UsdUndoVisibleCommand.h"

#include <mayaUsd/undo/UsdUndoBlock.h>

#include <pxr/usd/usdGeom/imageable.h>

namespace MAYAUSD_NS_DEF {
namespace ufe {

UsdUndoVisibleCommand::UsdUndoVisibleCommand(const UsdPrim& prim, bool vis)
: Ufe::UndoableCommand()
, _prim(prim)
, _visible(vis)
{
}

UsdUndoVisibleCommand::~UsdUndoVisibleCommand() { }

UsdUndoVisibleCommand::Ptr UsdUndoVisibleCommand::create(const UsdPrim& prim, bool vis)
{
if (!prim) {
return nullptr;
}
return std::make_shared<UsdUndoVisibleCommand>(prim, vis);
}

void UsdUndoVisibleCommand::execute()
{
UsdUndoBlock undoBlock(&_undoableItem);

_visible ? UsdGeomImageable(_prim).MakeVisible() : UsdGeomImageable(_prim).MakeInvisible();
}

void UsdUndoVisibleCommand::redo() { _undoableItem.redo(); }

void UsdUndoVisibleCommand::undo() { _undoableItem.undo(); }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On execute(), we create an undoBlock to capture the edit and then visibility is set via UsdGeomImageable MakeVisible()/MakeInvisible() methods.

} // namespace ufe
} // namespace MAYAUSD_NS_DEF
62 changes: 62 additions & 0 deletions lib/mayaUsd/ufe/UsdUndoVisibleCommand.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//
// Copyright 2020 Autodesk
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
#pragma once

#include <mayaUsd/base/api.h>
#include <mayaUsd/undo/UsdUndoableItem.h>

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

#include <ufe/undoableCommand.h>

PXR_NAMESPACE_USING_DIRECTIVE

namespace MAYAUSD_NS_DEF {
namespace ufe {

//! \brief UsdUndoVisibleCommand
class MAYAUSD_CORE_PUBLIC UsdUndoVisibleCommand : public Ufe::UndoableCommand
{
public:
typedef std::shared_ptr<UsdUndoVisibleCommand> Ptr;

UsdUndoVisibleCommand(const UsdPrim& prim, bool vis);
~UsdUndoVisibleCommand() override;

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

//! Create a UsdUndoVisibleCommand object
static UsdUndoVisibleCommand::Ptr create(const UsdPrim& prim, bool vis);

private:
void execute() override;
void undo() override;
void redo() override;

UsdPrim _prim;

bool _visible;

UsdUndoableItem _undoableItem;

}; // UsdUndoVisibleCommand

} // namespace ufe
} // namespace MAYAUSD_NS_DEF
174 changes: 174 additions & 0 deletions test/lib/ufe/testObject3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

from pxr import Usd, UsdGeom

import mayaUsd.ufe

import maya.cmds as cmds
import maya.api.OpenMaya as OpenMaya

Expand Down Expand Up @@ -228,3 +230,175 @@ def testVisibility(self):
ufe.Object3d.removeObserver(visObs)
self.assertFalse(ufe.Object3d.hasObserver(visObs))
self.assertEqual(ufe.Object3d.nbObservers(), 0)

@unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '2034', 'testUndoVisibleCmd is only available in UFE preview version 0.2.34 and greater')
def testUndoVisibleCmd(self):

''' Verify the token / attribute values for visibility after performing undo/redo '''

cmds.file(new=True, force=True)

# create a Capsule via contextOps menu
import mayaUsd_createStageWithNewLayer
mayaUsd_createStageWithNewLayer.createStageWithNewLayer()
proxyShapePath = ufe.PathString.path('|stage1|stageShape1')
proxyShapeItem = ufe.Hierarchy.createItem(proxyShapePath)
proxyShapeContextOps = ufe.ContextOps.contextOps(proxyShapeItem)
proxyShapeContextOps.doOp(['Add New Prim', 'Capsule'])

# create an Object3d interface.
capsulePath = ufe.PathString.path('|stage1|stageShape1,/Capsule1')
capsuleItem = ufe.Hierarchy.createItem(capsulePath)
capsulePrim = mayaUsd.ufe.ufePathToPrim(ufe.PathString.string(capsulePath))
object3d = ufe.Object3d.object3d(capsuleItem)

# stage / primSpec
stage = mayaUsd.ufe.getStage(str(proxyShapePath))
primSpec = stage.GetEditTarget().GetPrimSpecForScenePath('/Capsule1');

# initially capsuleItem should be visible.
self.assertTrue(object3d.visibility())

# make it invisible.
visibleCmd = object3d.setVisibleCmd(False)
visibleCmd.execute()

# get the visibility "attribute"
visibleAttr = capsulePrim.GetAttribute('visibility')

# expect the visibility attribute to be 'invisible'
self.assertEqual(visibleAttr.Get(), 'invisible')

# visibility "token" must exists now in the USD data model
self.assertTrue(bool(primSpec and UsdGeom.Tokens.visibility in primSpec.attributes))

# undo
visibleCmd.undo()

# expect the visibility attribute to be 'inherited'
self.assertEqual(visibleAttr.Get(), 'inherited')

# visibility token must not exists now in the USD data model after undo
self.assertFalse(bool(primSpec and UsdGeom.Tokens.visibility in primSpec.attributes))

# capsuleItem must be visible now
self.assertTrue(object3d.visibility())

# redo
visibleCmd.redo()

# expect the visibility attribute to be 'invisible'
self.assertEqual(visibleAttr.Get(), 'invisible')

# visibility token must exists now in the USD data model after redo
self.assertTrue(bool(primSpec and UsdGeom.Tokens.visibility in primSpec.attributes))

# capsuleItem must be invisible now
self.assertFalse(object3d.visibility())

@unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '2034', 'testMayaHideAndShowHiddenUndoCommands is only available in UFE preview version 0.2.34 and greater')
def testMayaHideAndShowHiddenUndoCommands(self):
''' Verify the token / attribute values for visibility via "hide", "showHidden" commands + Undo/Redo '''

cmds.file(new=True, force=True)

# create a Capsule and Cylinder via contextOps menu
import mayaUsd_createStageWithNewLayer
mayaUsd_createStageWithNewLayer.createStageWithNewLayer()
proxyShapePath = ufe.PathString.path('|stage1|stageShape1')
proxyShapeItem = ufe.Hierarchy.createItem(proxyShapePath)
proxyShapeContextOps = ufe.ContextOps.contextOps(proxyShapeItem)
proxyShapeContextOps.doOp(['Add New Prim', 'Capsule'])
proxyShapeContextOps.doOp(['Add New Prim', 'Cylinder'])

# capsule
capsulePath = ufe.PathString.path('|stage1|stageShape1,/Capsule1')
capsuleItem = ufe.Hierarchy.createItem(capsulePath)
capsulePrim = mayaUsd.ufe.ufePathToPrim(ufe.PathString.string(capsulePath))

# cylinder
cylinderPath = ufe.PathString.path('|stage1|stageShape1,/Cylinder1')
cylinderItem = ufe.Hierarchy.createItem(cylinderPath)
cylinderPrim = mayaUsd.ufe.ufePathToPrim(ufe.PathString.string(cylinderPath))

# stage / primSpec
stage = mayaUsd.ufe.getStage(str(proxyShapePath))
primSpecCapsule = stage.GetEditTarget().GetPrimSpecForScenePath('/Capsule1');
primSpecCylinder = stage.GetEditTarget().GetPrimSpecForScenePath('/Cylinder1');

# select capsule and cylinder prims
ufe.GlobalSelection.get().append(capsuleItem)
ufe.GlobalSelection.get().append(cylinderItem)

# hide selected items
cmds.hide(cs=True)

# get the visibility "attribute"
capsuleVisibleAttr = capsulePrim.GetAttribute('visibility')
cylinderVisibleAttr = cylinderPrim.GetAttribute('visibility')

# expect the visibility attribute to be 'invisible'
self.assertEqual(capsuleVisibleAttr.Get(), 'invisible')
self.assertEqual(cylinderVisibleAttr.Get(), 'invisible')

# visibility "token" must exists now in the USD data model
self.assertTrue(bool(primSpecCapsule and UsdGeom.Tokens.visibility in primSpecCapsule.attributes))
self.assertTrue(bool(primSpecCylinder and UsdGeom.Tokens.visibility in primSpecCylinder.attributes))

# undo
cmds.undo()

# expect the visibility attribute to be 'inherited'
self.assertEqual(capsuleVisibleAttr.Get(), 'inherited')
self.assertEqual(cylinderVisibleAttr.Get(), 'inherited')

# visibility token must not exists now in the USD data model after undo
self.assertFalse(bool(primSpecCapsule and UsdGeom.Tokens.visibility in primSpecCapsule.attributes))
self.assertFalse(bool(primSpecCylinder and UsdGeom.Tokens.visibility in primSpecCylinder.attributes))

# undo
cmds.redo()

# expect the visibility attribute to be 'invisible'
self.assertEqual(capsuleVisibleAttr.Get(), 'invisible')
self.assertEqual(cylinderVisibleAttr.Get(), 'invisible')

# visibility "token" must exists now in the USD data model
self.assertTrue(bool(primSpecCapsule and UsdGeom.Tokens.visibility in primSpecCapsule.attributes))
self.assertTrue(bool(primSpecCylinder and UsdGeom.Tokens.visibility in primSpecCylinder.attributes))

# hide selected items again
cmds.hide(cs=True)

# right after, call showHidden -all to make everything visible again
cmds.showHidden( all=True )

# expect the visibility attribute to be 'inherited'
self.assertEqual(capsuleVisibleAttr.Get(), 'inherited')
self.assertEqual(cylinderVisibleAttr.Get(), 'inherited')

# This time, expect the visibility token to exists in the USD data model
self.assertTrue(bool(primSpecCapsule and UsdGeom.Tokens.visibility in primSpecCapsule.attributes))
self.assertTrue(bool(primSpecCylinder and UsdGeom.Tokens.visibility in primSpecCylinder.attributes))

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would have been nice to undo / redo showHidden --- should be quick to add and we'd put this one to rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Addressed in 585cca0

# undo the showHidden command
cmds.undo()

# expect the visibility attribute to be 'invisible'
self.assertEqual(capsuleVisibleAttr.Get(), 'invisible')
self.assertEqual(cylinderVisibleAttr.Get(), 'invisible')

# visibility "token" must exists now in the USD data model
self.assertTrue(bool(primSpecCapsule and UsdGeom.Tokens.visibility in primSpecCapsule.attributes))
self.assertTrue(bool(primSpecCylinder and UsdGeom.Tokens.visibility in primSpecCylinder.attributes))

# redo
cmds.redo()

# expect the visibility attribute to be 'inherited'
self.assertEqual(capsuleVisibleAttr.Get(), 'inherited')
self.assertEqual(cylinderVisibleAttr.Get(), 'inherited')

# visibility "token" must exists now in the USD data model
self.assertTrue(bool(primSpecCapsule and UsdGeom.Tokens.visibility in primSpecCapsule.attributes))
self.assertTrue(bool(primSpecCylinder and UsdGeom.Tokens.visibility in primSpecCylinder.attributes))