Selection Modification

[ericmehl]

This adds a mechanism for modifying what scene locations are actually selected when making a viewport selection. Users can register functions in Python or C++ that get a scene and the selection made by the user, and return the selection they want as a result.

It also includes a first use of the mechanism in the form of USD Kind selection. Starting at the viewport selected location, the first ancestor, or the original location itself, that is the USD Kind chosen in the selectionModifier plug is what is actually selected.

I tried a few different methods of presenting the USD Kinds in the selectionModifier widget. USD Kinds are hierarchical (https://openusd.org/dev/api/kind_page_front.html#mainpage_kind) so, for example, setting the selectionModifier to select USD Kind of model will also select component, group and assembly. Currently I'm expressing that by spelling out all the Kinds that will be considered a match in the menu item.

That was originally driven by using the standard presets metadata and dropdown. I think a nicer widget is what I have now, very similar to the displayTransform.name widget. Before settling on this presentation, I also looked at indenting based on the hierarchy, and even a little ASCII art to draw out elbows and lines to form a tree look.

Indenting with the default presets widget meant that when you make your selection, the label in the collapsed dropdown is indented too, which felt rather odd.

Now that I'm using a custom widget anyways, maybe the indented presentation is worth another look?

Checklist

• I have read the contribution guidelines.
• I have updated the documentation, if applicable.
• I have tested my change(s) in the test suite, and added new test cases where necessary.
• My code follows the Gaffer project's prevailing coding style and conventions.

[ericmehl]

I made the changes we talked about yesterday in the team call, namely :

• Using just "Select" as the label for the plug in the UI. That seemed a little unclear to use by itself for the actual plug name, so I went with selectModePlug and similar selectMode* throughout.
• Along with that, the default is now called "Any".
• Use attributes() instead of fullAttributes() so we get the selection to the location that actually holds the attribute we're looking for, instead of one that's inheriting it.
• Remove the verbose mode labeling and go with "Assembly", "Group", etc.

I'm looking at the jumpy UI when switching from Select to Translate and other tools that add additional UI elements alongside this new selectMode plug.

[ericmehl]

I found a couple of ways to fix the jump in the UI. One is to add a 4px margin-top and margin-bottom to the style sheet, targeted at the widget for SelectionToolUI.SelectModePlugValueWidget.

What I went with instead is adding a GafferUI.Frame around the SelectionToolUI.SelectModePlugValueWidget.__menuButton. That works just as well, and is less complicated because it doesn't modify the style sheet. Curiously, this solution needs as little as a 1px borderWidth for the frame wrapper. I would have expected it to need 4px to match the TransformTool._SselectionWidget frame.

I'm open to other solutions if something seems better.

Otherwise, ready for review!

[johnhaddon]

Thanks Eric, and once again, sorry for the delay in getting to the review. This is a nice addition, and I like how you're querying the kind registry to work with anything in there, rather than just hardcoding it.

In addition to the inline comments, here's a couple of other talking points :

• I think we discussed synchronising the select mode across all the tools? Currently they each have their own mode, which is going to get pretty confusing I think.
• I don't have a strong rationale either way, but I do wonder if "Standard" might be better than "Any" as the default mode. Or maybe "Default"?
• I wonder if there are any non-usd modes we could provide out of the box? First-ancestor-with-a-shader-assignment perhaps? Not that I can think of a concise label for that right now.

Cheers...
John

[ericmehl]

• I think we discussed synchronising the select mode across all the tools? Currently they each have their own mode, which is going to get pretty confusing I think.

Right! That had slipped my mind. I'll look at that now that I have the other feedback addressed.

• I don't have a strong rationale either way, but I do wonder if "Standard" might be better than "Any" as the default mode. Or maybe "Default"?

"Standard" seems pretty good to me, it sounds a bit more confident than "Default" to my mind. That is fixed over the course of 5aa08c4, f16556d and 4b460cf for the purposes of squishability.

• I wonder if there are any non-usd modes we could provide out of the box? First-ancestor-with-a-shader-assignment perhaps? Not that I can think of a concise label for that right now.

That sounds like a nice one to have too, I'll add that in with something better than First-ancestor-with-a-shader-assignment or, my working title, "Shader-Bequether"

[ericmehl]

I added selection synchronization across the tools in dc7c007. And in 65ea3da, I added a mode to select the ancestor (or current location) that has either a surface or displacement shader.

Ready for a new look.

[johnhaddon]

Ready for a new look.

Thanks Eric, looks great! I like the new shader-bequeather modes :) Two small things :

• Could we capitalize the names for the USD modes please? (in the registration code, not the menu-building code).
• Should we target this to main since that is where StyleSheet : Reserve space for valueChanged icon #5737? We're really close to getting 1.4 out the door, and I don't think this is urgent enough that it needs to go in 1.3.x.

[ericmehl]

• Could we capitalize the names for the USD modes please? (in the registration code, not the menu-building code).

USD Kind comparisons are case-sensitive, so I didn't do this initially in order to not make assumptions when converting to and from the native Kind name. Do you think IECore.CamelCase.toSpaced() and .fromSpaced() would be appropriate? Or just assuming all USD Kinds, including custom Kinds a user might register, are lower case?

• Should we target this to main

Yeah, with #5737 is on main and 1.4 is so close, that sounds good to me.

[johnhaddon]

Do you think IECore.CamelCase.toSpaced() and .fromSpaced() would be appropriate?

I think toSpaced() might be a reasonable way of formatting the first argument to registerMode(). But I don't know why we would need fromSpaced() at all. Oh, I see, you're deriving the original kind from the formatted name in __kindSelectionModifier - we don't need to do that. We should just pass the original true kind name to that...

[ericmehl]

We should just pass the original true kind name to that...

Oh yes, of course. Had a brain blockage on that one.

[johnhaddon]

Cool - if you could get this all squashed down and ready to merge for main that would be awesome. Thanks!

[johnhaddon]

Thanks Eric!