PointInstancer : Add new node for making PointInstancer objects

[johnhaddon]

This converts any points-based primitive to a PointInstancer, collecting prototypes and parenting them below. It works around the limitations of the USD instancing spec by allowing variations of the prototypes to be generated using time offsets and arbitrary context variables specified as primitive variables on the source points.

Like ContactSheet, the implementation is split between an internal C++ node and a user-facing node authored as a Box and exported via ExtensionAlgo. I'm pretty open to opinions about the user-facing design - questions on my mind include :

• Is the shuffling of the transform primitive variables actually useful, or would we be better off making people do that beforehand?
• Should the None prototype modes be removed or renamed? Would the add-variation-to-existing-instancer feature they enable be better off in a separate PrototypeVariations node?
• Is the ordering of sections in the UI right? Should we do more to guide a user to making the minimal configuration to get useful output (specifying the prototype locations)?

[murraystevenson]

Thanks John, a few comments inline for what I managed to get through today. I've also pushed two fixups with spelling/typo corrections (seemed simpler to just fix them rather than make a bunch more comments).

Is the shuffling of the transform primitive variables actually useful, or would we be better off making people do that beforehand?

It may be useful, but it does feel secondary/tertiary to setting up your prototypes, their variations and any shading attributes. Maybe the Transform and IDs sections are better demoted to below Prototypes, Prototype Variation, and Attributes?

Should the None prototype modes be removed or renamed? Would the add-variation-to-existing-instancer feature they enable be better off in a separate PrototypeVariations node?

I haven't tried using it too much just yet, but the None modes do seem like they're muddying the waters of the node a bit. I'm also wondering about the current behaviour of the attributes plug when you're using the PointInstancer node to modify the prototypes of an existing point instancer. You need to remember to set attributes to * to ensure any previously included primitive variables aren't deleted, which seems easy to overlook. Maybe this would all be simpler with a dedicated PrototypeVariations node?

[johnhaddon]

Thanks John, a few comments inline for what I managed to get through today.

Thanks Murray - I've pushed fixups for everything and noted them inline.

Maybe this would all be simpler with a dedicated PrototypeVariations node?

That is seeming pretty reasonable. I started prototyping such a node and the first thing that crops up is the layout of the prototypes on output.

For PointInstancer I felt it was pretty reasonable to be opinionated, enforcing that prototypes are always parented below the instancer, always in a flat list, etc. It's still possible to import a less/differently structured instancer from USD, or build one from primitives in Gaffer.

But for PrototypeVariations I wonder if a user might reasonably expect us to build the variations in-place, respecting the original layout of prototypes. Might it be better to lay them out as path/to/original/prototype/{variation} rather than conform them in the same way as PointInstancer? Worth getting this hashed out as it might affect the design of PointInstancerCore.

[johnhaddon]

Testing this a bit today, I'm finding the attributes plug annoying. Maybe we default it to *, or just remove it?

[johnhaddon]

Based on an out-of-band conversation, I've pushed c3ad326 to remove the None modes. We think these muddy the water a bit, and would be better presented in a separate PrototypeVariations node.

I think the main question remaining now is what to do with the attributes plug.

[murraystevenson]

I've pushed c3ad326 to remove the None modes. We think these muddy the water a bit, and would be better presented in a separate PrototypeVariations node.

Thanks, with these removed and the visibilityActivators added, the UI is feeling a lot more straightforward. I think you're good to squash everything down at this point.

I think the main question remaining now is what to do with the attributes plug.

I'd vote for keeping the attributes plug and defaulting it to *. I was considering removing it entirely for simplicity as people could always use a DeletePrimitiveVariables to clean up unnecessary primvars on pointInstancers, but that does get a bit fiddly if you're doing that downstream and having to make sure you're not also deleting the pointInstancer specific primitive variables, so I think attributes justifies its existence...

[johnhaddon]

I think you're good to squash everything down at this point.

Done.

I'd vote for keeping the attributes plug and defaulting it to *.

Done, and squashed into c2723f9.