-
Notifications
You must be signed in to change notification settings - Fork 202
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-106188 - As a user, I would like to activate and deactivate prims #773
Conversation
* Implemented new UFE interface UIInfoHandler. * Added support for child filtering with new Ufe::Hierarchy method "filteredChildren" and Ufe::HierarchyHandler method "childFilter". In USD we support a single child filter for "Inactive Prims". * Added context menu for "{Active/Deactivate} Prim". * Added test cases for new interface, new methods and new menu.
#if UFE_PREVIEW_VERSION_NUM >= 2022 | ||
Ufe::Hierarchy::ChildFilter ProxyShapeHierarchyHandler::childFilter() const | ||
{ | ||
return Ufe::Hierarchy::ChildFilter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return empty list - we don't (yet) support any child filters for the proxy shape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this mean that inactive children of the USD pseudo-root (which the proxy shape represents) can't be shown?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops I forgot about that case. I'll fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See commit e19fa54
static constexpr char kUSDToggleActiveStateItem[] = "Toggle Active State"; | ||
static constexpr char kUSDActivatePrimLabel[] = "Activate Prim"; | ||
static constexpr char kUSDDeactivatePrimLabel[] = "Deactivate Prim"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New context menu item to activate/deactivate a prim
@@ -41,19 +42,16 @@ | |||
#endif | |||
|
|||
namespace { | |||
UsdPrimSiblingRange filteredChildren( const UsdPrim& prim ) | |||
UsdPrimSiblingRange getUSDFilteredChildren(const UsdPrim& prim, const Usd_PrimFlagsPredicate pred = UsdPrimDefaultPredicate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the predicate as input (with default) so that it can be used by new filtered children method.
if (MGlobal::executeCommand("displayRGBColor -q \"outlinerInvisibleColor\"", outlinerInvisibleColor) && | ||
(outlinerInvisibleColor.length() == 3)) | ||
{ | ||
double rgb[3]; | ||
outlinerInvisibleColor.get(rgb); | ||
info.textFgColor.set(static_cast<float>(rgb[0]), static_cast<float>(rgb[1]), static_cast<float>(rgb[2])); | ||
} | ||
else | ||
{ | ||
info.textFgColor.set(0.403922f, 0.403922f, 0.403922f); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get the current Maya Outliner invisible color and set that for inactive prim. If we cannot get color just use a hard-coded gray. This happens when we run the tests. The displayRGBColor command will fail.
|
||
std::string UsdUIInfoHandler::getLongRunTimeLabel() const | ||
{ | ||
return "Universal Scene Description"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used by the menu divider (for a label).
# Active / Deactivate Prim | ||
cmd = self.contextOps.doOpCmd(['Toggle Active State']) | ||
self.assertIsNotNone(cmd) | ||
|
||
# Initially, Ball_35 should be active. | ||
self.assertTrue(self.ball35Prim.IsActive()) | ||
|
||
ufeCmd.execute(cmd) | ||
self.assertFalse(self.ball35Prim.IsActive()) | ||
|
||
cmds.undo() | ||
self.assertTrue(self.ball35Prim.IsActive()) | ||
|
||
cmds.redo() | ||
self.assertFalse(self.ball35Prim.IsActive()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test the toggle active state (with undo/redo).
# Clear selection to start off | ||
cmds.select(clear=True) | ||
|
||
def testUIInfo(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically this tests the new UIInfoHandler interface. Make sure we have a long runtime label. And that the treeViewCellInfo will do something for an inactive prim.
lib/mayaUsd/ufe/UsdContextOps.cpp
Outdated
} | ||
|
||
private: | ||
UsdPrim _prim; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On working with Hamed on parenting bugs I've become increasingly weary about storing prims (or scene items, same thing) as command data members, because they can go stale. Imagine the following sequence of commands:
- Inactivate prim (prim stored in this command)
- Reparent prim (prim removed using UsdStage::removePrim())
- Undo reparent
- Undo inactivate --- boom.
You should be able to demonstrate this fairly easily. Solution is the same everywhere, and I know you've implemented it in the past: use a path. In this case it can be the SdfPath and the UsdStage of the prim (that would get my vote), or you can use the Ufe::Path and call Hierarchy::createItem(), then downcast to the UsdSceneItem and get the prim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes good thinking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See commit e19fa54
#if UFE_PREVIEW_VERSION_NUM >= 2022 | ||
Ufe::Hierarchy::ChildFilter ProxyShapeHierarchyHandler::childFilter() const | ||
{ | ||
return Ufe::Hierarchy::ChildFilter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this mean that inactive children of the USD pseudo-root (which the proxy shape represents) can't be shown?
Ufe::ChildFilterFlag inactivePrims("InactivePrims", "Inactive Prims", true); | ||
Ufe::Hierarchy::ChildFilter childFilters; | ||
childFilters.emplace_back(inactivePrims); | ||
return childFilters; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
construct ChildFilterFlag object in place, creating inactivePrims is not necessary:
Ufe::Hierarchy::ChildFilter childFilters;
childFilters.emplace_back("InactivePrims", "Inactive Prims", true);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another good catch - thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See commit e19fa54
* Code review comments: ** Need child filter for ProxyShapeHierarchy. ** Store Stage/SdfPath instead of UsdPrim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you.
{ | ||
_active = _prim.IsActive(); | ||
_stage = prim.GetStage(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit-pick: could have put everything in the constructor initializer list, rather than having the default constructor called, then doing the assignment:
ToggleActiveStateCommand(const UsdPrim& prim) : _stage(prim.GetStage()), _primPath(prim.GetPath()), _active(prim.IsActive) {}
Ready to merge. My preflight failed on the 2020_python2_osx because of machine connection problem. All the other builds passed. |
MAYA-106188 - As a user, I would like to activate and deactivate prims