Update AL_USDMaya Selection code to work with UFE Selection #1855
Conversation
There was a problem hiding this comment.
My lack of familiarity with the AL plugin most certainly means that some of the comments are off-base, but I'm perplexed by the change to the proxyShapeBase code to provide UFE selection observability. Given that the UFE global selection is already observable, this might suggest that we are missing something to make the UFE global selection observation as useful as possible to its clients.
plugin/al/lib/AL_USDMaya/AL/usdmaya/cmds/ProxyShapeCommands.cpp
Outdated
Show resolved
Hide resolved
plugin/al/plugin/AL_USDMayaTestPlugin/AL/usdmaya/commands/test_ProxyShapeSelect.cpp
Show resolved
Hide resolved
plugin/al/plugin/AL_USDMayaTestPlugin/AL/usdmaya/commands/test_ProxyShapeSelect.cpp
Show resolved
Hide resolved
plugin/al/plugin/AL_USDMayaTestPlugin/AL/usdmaya/commands/test_ProxyShapeSelect.cpp
Outdated
Show resolved
Hide resolved
plugin/al/plugin/AL_USDMayaTestPlugin/AL/usdmaya/commands/test_ProxyShapeSelect.cpp
Outdated
Show resolved
Hide resolved
| if (index != std::string::npos) { | ||
| m_proxy->m_selectedPaths.insert(SdfPath(pathStr.c_str() + index)); | ||
| } else { | ||
| // Presumably the root node has been selected, but it doesn't appear I |
There was a problem hiding this comment.
Typically we use the proxy shape node itself as a proxy for the USD root, e.g. in the UFE hierarchy interface the proxy shape children are in fact the USD root children.
| MGlobal::getActiveSelectionList(sl); | ||
| sl.add(m_proxy->thisMObject()); | ||
|
|
||
| // UGH. This nukes the UFE selection, and it doesn't appear it's possible to create a |
There was a problem hiding this comment.
Hopefully using the proxy shape node will be useful here.
| } | ||
|
|
||
| /// UGH! This clears the UFE selection :( | ||
| MGlobal::setActiveSelectionList(sl); |
There was a problem hiding this comment.
The UFE selection should be a superset of the the Maya selection, as far as objects are concerned. Therefore, following this call, the UFE selection should have all the Maya objects in sl.
There was a problem hiding this comment.
Is this in response to the comment?
| for (auto& selectedPath : orderedPaths) { | ||
| // when selecting the root path, select the ProxyShape node using normal maya | ||
| // selection | ||
| if (selectedPath == root) { |
There was a problem hiding this comment.
This difference between using the Maya selection and the UFE selection to deal with the pseudo-root feels tricky. Why not just add the proxy shape to the UFE selection? I'm far from being an expert in the AL plugin, so ignore this suggestion if it's not relevant.
| @@ -1108,6 +1108,9 @@ bool ProxyRenderDelegate::getInstancedSelectionPath( | |||
| return false; | |||
| } | |||
|
|
|||
| if (_proxyShapeData->ProxyShape()) | |||
There was a problem hiding this comment.
Is this going to catch only marquee selections in the viewport? What about selections made in the Outliner, or any other future UFE-enabled node viewer or editor? How would they receive these pre selection and post selection changed notifications? Is there no way to reformulate this as standard UFE selection changed notifications? It feels awkward to go through the proxy shape to distribute the notifications, when the UFE global selection itself is already observable, and is not tied to viewport-based selection.
There was a problem hiding this comment.
We're looking into removing (or at least moving somewhere else) these notifications.
|
Thanks for the review @ppt-adsk , we'll get back to you shortly. |
|
The code found in |
|
Thanks for the feedback @ppt-adsk, after some discussions within the team we're potentially looking to retire code related to proxy draw override and proxy shape selection as per discussion topic #1879. I'll close the PR now, given that it's almost entirely about those two. Again, thank you for the suggestions, it's helped move things along quite a bit! |
|
Excellent, glad I could help! |
This PR implements various changes additionally supporting UFE selection across the AL plugin.