MAYA-102266 Selection highlight color for lead and active selection #671
Conversation
To be consistent with Maya's convention, lead selection and active selection should be highlighted with different colors.
|
I tried compiling this, against f937f9c ( + this PR ) with Maya Beta PR116, and received this error :
Logged as : #674 I'm going to see if I can re-wind to my last successful build, and apply the PR there. |
|
Compiled against dev @f937f9c and now it works : Spheres : Basis Curves : ( mixed results ) Instanced ones do not highlight properly in shaded mode, but work in wireframe : Wireframe OK : |
|
@cfmoore007 The selection highlight refresh issue of basisCurves should be a pre-existing one? |
|
Yes, it is a pre-existing issue: #259 To fix it, we need another PR that adds a dedicated selection highlight item which can cover the shaded item when being selected. |
|
@HdC-adsk - no problem, thanks! |
|
@cfmoore007 Thanks for super-quick validating. |
| return; | ||
| } | ||
|
|
||
| SdfPath usdPath(segments[1].string()); |
There was a problem hiding this comment.
@ppt-adsk do we have a better way to validate the selected item and extract sdfpath?
There was a problem hiding this comment.
At this point, because of lines 131-133, we know the Ufe::SceneItem is a UsdSceneItem, so we can dynamic_pointer_cast to that and simply call prim() on it. In fact, dynamic_pointer_cast and item->runTimeId() != USD_UFE_RUNTIME_ID serve exactly the same purpose, so I would replace lines 131-133 with the dynamic_pointer_cast.
There was a problem hiding this comment.
@ppt-adsk The new code looks nicer. However, we need to filter the UFE scene items which don't belong to the current proxy shape, see Line 135. Any suggestion how to handle this logic with the new code?
There was a problem hiding this comment.
Changing a Ufe::SceneItem::Ptr to a UsdSceneItem::Ptr wouldn't change lines 135-138, it would only make line 140 less awkward, since you could ask the UsdPrim directly for its SdfPath without going through a string.
However, lines 135-138 might be more elegant as
if (!item->path().startsWith(proxyPath)) {
return;
}
if I'm not mistaken.
| // Start reverse iteration from the second last item in UFE global selection | ||
| // and populate active selection. | ||
| for (; it != globalSelection->crend(); it++) { | ||
| PopulateSelection(*it, proxyPath, *_sceneDelegate, _activeSelection); |
There was a problem hiding this comment.
Nothing big, but since you are already checking if the global selection list is empty or not at line 808, you could pull this loop inside the condition.
To be consistent with Maya's convention, lead selection and active selection should be highlighted with different colors.