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

Test if visibility edit is valid before returning command. #2725

Merged
merged 1 commit into from
Nov 16, 2022
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
26 changes: 18 additions & 8 deletions lib/mayaUsd/ufe/UsdUndoVisibleCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@
namespace MAYAUSD_NS_DEF {
namespace ufe {

UsdUndoVisibleCommand::UsdUndoVisibleCommand(const UsdPrim& prim, bool vis)
UsdUndoVisibleCommand::UsdUndoVisibleCommand(
const UsdPrim& prim,
bool vis,
const PXR_NS::SdfLayerHandle& layer)
: Ufe::UndoableCommand()
, _prim(prim)
, _visible(vis)
, _layer(getEditRouterLayer(PXR_NS::TfToken("visibility"), prim))
, _layer(layer)
{
}

Expand All @@ -39,19 +42,26 @@ UsdUndoVisibleCommand::Ptr UsdUndoVisibleCommand::create(const UsdPrim& prim, bo
if (!prim) {
return nullptr;
}
return std::make_shared<UsdUndoVisibleCommand>(prim, vis);
}

void UsdUndoVisibleCommand::execute()
{
UsdGeomImageable primImageable(_prim);
auto layer = getEditRouterLayer(PXR_NS::TfToken("visibility"), prim);

UsdGeomImageable primImageable(prim);

EditTargetGuard guard(prim, layer);

std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(primImageable.GetVisibilityAttr(), &errMsg)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perform the edit check on command creation, rather than execution. Failure on execution should be reported through an exception, but here the check really belongs to command creation: we communicate to the caller that the operation is not possible by returning a nullptr.

Furthermore, the edit guard must be set before performing the check, otherwise the check will be done on the current target, then the edit guard changes that target. In the case of an existing visibility edit in the session layer, this would mean that the check would be done on the current edit target layer, which typically is lower priority than the session layer, and the edit would fail. If the check is done when the edit target guard is set, the check properly occurs on the session layer. This accounts for many of the failures described in

#2549

MGlobal::displayError(errMsg.c_str());
return;
return nullptr;
}

return std::make_shared<UsdUndoVisibleCommand>(prim, vis, layer);
}

void UsdUndoVisibleCommand::execute()
{
UsdGeomImageable primImageable(_prim);

UsdUndoBlock undoBlock(&_undoableItem);

EditTargetGuard guard(_prim, _layer);
Expand Down
6 changes: 5 additions & 1 deletion lib/mayaUsd/ufe/UsdUndoVisibleCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ class MAYAUSD_CORE_PUBLIC UsdUndoVisibleCommand : public Ufe::UndoableCommand
public:
typedef std::shared_ptr<UsdUndoVisibleCommand> Ptr;

UsdUndoVisibleCommand(const PXR_NS::UsdPrim& prim, bool vis);
// Public for std::make_shared() access, use create() instead.
UsdUndoVisibleCommand(
const PXR_NS::UsdPrim& prim,
bool vis,
const PXR_NS::SdfLayerHandle& layer);
~UsdUndoVisibleCommand() override;

// Delete the copy/move constructors assignment operators.
Expand Down
110 changes: 73 additions & 37 deletions test/lib/ufe/testVisibilityCmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def getSessionLayer(context, routingData):
if prim is None:
print('Prim not in context')
return

routingData['layer'] = prim.GetStage().GetSessionLayer().identifier

def createUfePathSegment(usdPath):
Expand All @@ -34,86 +34,122 @@ def createUfePathSegment(usdPath):
return ufe.PathSegment(usdPath, mayaUsd.ufe.getUsdRunTimeId(), '/')

class VisibilityCmdTestCase(unittest.TestCase):
'''Verify the Maya Edit Router for visibility.'''
pluginsLoaded = False
@classmethod
def setUpClass(cls):
'''Verify the Maya Edit Router for visibility.'''
pluginsLoaded = False

@classmethod
def setUpClass(cls):
fixturesUtils.readOnlySetUpClass(__file__, loadPlugin=False)
if not cls.pluginsLoaded:
cls.pluginsLoaded = mayaUtils.isMayaUsdPluginLoaded()
@classmethod
def tearDownClass(cls):

@classmethod
def tearDownClass(cls):
standalone.uninitialize()

def setUp(self):
''' Called initially to set up the Maya test environment '''
# Load plugins
self.assertTrue(self.pluginsLoaded)
cmds.select(clear=True)

def testEditRouter(self):
'''Test edit router functionality.'''
def setUp(self):
self.assertTrue(self.pluginsLoaded)

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

# Create the following hierarchy:
#
# ps
# |_ A
# |_ B
#

psPathStr = mayaUsd_createStageWithNewLayer.createStageWithNewLayer()
stage = mayaUsd.lib.GetPrim(psPathStr).GetStage()
stage.DefinePrim('/A', 'Xform')
stage.DefinePrim('/B', 'Xform')

psPath = ufe.PathString.path(psPathStr)
psPathSegment = psPath.segments[0]
aPath = ufe.Path([psPathSegment, usdUtils.createUfePathSegment('/A')])
bPath = ufe.Path([psPathSegment, usdUtils.createUfePathSegment('/B')])
a = ufe.Hierarchy.createItem(aPath)
b = ufe.Hierarchy.createItem(bPath)
self.a = ufe.Hierarchy.createItem(aPath)
self.b = ufe.Hierarchy.createItem(bPath)

cmds.select(clear=True)

def tearDown(self):
# Restore default edit router.
mayaUsd.lib.restoreDefaultEditRouter('visibility')

def _testEditRouter(self):
'''Test edit router functionality.'''

# Select /A
sn = ufe.GlobalSelection.get()
sn.clear()
sn.append(a)

# Hide A
cmds.hide()

sn.append(self.a)

# Get the session layer
prim = mayaUsd.ufe.ufePathToPrim("|stage1|stageShape1,/A")
sessionLayer = prim.GetStage().GetSessionLayer()

# Check that the session layer is empty
self.assertTrue(sessionLayer.empty)

# Register visibility
# Send visibility edits to the session layer.
mayaUsd.lib.registerEditRouter('visibility', getSessionLayer)

# Check that something was written to the session layer
self.assertIsNotNone(sessionLayer)

# Select /B
sn = ufe.GlobalSelection.get()
sn.clear()
sn.append(b)

sn.append(self.b)
# Hide B
cmds.hide()

# Check that something was written to the session layer
self.assertIsNotNone(sessionLayer)
self.assertIsNotNone(sessionLayer.GetPrimAtPath('/B'))

# Check that any visibility changes were written to the session layer
self.assertIsNotNone(sessionLayer.GetAttributeAtPath('/B.visibility').default)

# Check that correct visibility changes were written to the session layer
self.assertEqual(filterUsdStr(sessionLayer.ExportToString()),
'over "B"\n{\n token visibility = "invisible"\n}')
'over "B"\n{\n token visibility = "invisible"\n}')

def testEditRouterShowHideMultipleSelection(self):
'''Test edit routing under show and hide scenarios with multiple selection.'''

# Get the session layer, check it's empty.
prim = mayaUsd.ufe.ufePathToPrim("|stage1|stageShape1,/A")
sessionLayer = prim.GetStage().GetSessionLayer()
self.assertTrue(sessionLayer.empty)

# Send visibility edits to the session layer.
mayaUsd.lib.registerEditRouter('visibility', getSessionLayer)

# Select /A, hide it.
sn = ufe.GlobalSelection.get()
sn.clear()
sn.append(self.a)

cmds.hide()

# Check visibility was written to the session layer.
self.assertEqual(filterUsdStr(sessionLayer.ExportToString()),
'over "A"\n{\n token visibility = "invisible"\n}')

# Hiding a multiple selection with already-hidden A must not error.
# Careful: hide command clears the selection, so must add /A again.
sn.append(self.a)
sn.append(self.b)

cmds.hide()

# Check visibility was written to the session layer.
self.assertEqual(filterUsdStr(sessionLayer.ExportToString()),
'over "A"\n{\n token visibility = "invisible"\n}\nover "B"\n{\n token visibility = "invisible"\n}')

if __name__ == '__main__':
unittest.main(verbosity=2)
unittest.main(verbosity=2)