Improved Story viewport brackets and helpers

[sdumetz]

My initial goal was to provide a visual indicator for an object's current pivot point when brackets are shown.

While I was at it I improved the style of the various light helpers. to have a more distinctive look. It should make it easier to visually identify a light type and know how it will affect the scene by looking at its viewport helper.

I ended up rewriting the brackets/helpers rendering code because I couldn't get rid of some positioning and synchronization inconsistencies otherwise. I like the new system though, it saves a render pass. I did not test whether it improved performance but it shouldn't hurt.

I took the opportunity to narrow down some property constraints where possible (mainly min: 0 where negative values wouldn't make sense).

All light types should now be usable and easy-enough to configure with the visual helpers, they just lack some UI elements to add/remove lights from a scene, for which I will have a PR coming real soon.

[gjcope]

A few issues/questions:

1. The pivot point with active brackets is easy to miss and looks to only show up where the model is not rendered. Maybe needs a different render order?
2. What's the intended use/value prop for the axes in the pose task? If it's to correlate with manual use of the transform inputs, we should probably color code those as well. Otherwise I'm not clear on how much value they are adding. I'm also seeing quite a bit of z-buffer flicker with the overlapping grid lines.
3. The directional light helpers seem to be quite small and also sometimes disconnected from the directional ray indicator (see pic below).
   

[sdumetz]

1. depends on the model's scale. Fixed in #fb9a2e02

2. Coloring the transform inputs might be a very good idea, I'll try it in the future. I made them because figuring out where we are in the pose task can be really difficult when an object is far off-center or has a weird shape. We already discussed this regarding the zoomExtentsmethod in #266 and I think this is satisfactory : It gives the user a sense of how the camera moved to make the model fit.

3. the helper's length is tied to the light's shadow near/far distances in an attempt to help users diagnose "why isn't my light casting proper shadows", which is a question I get a lot. Maybe that's not a good approach? I don't know what would be the proper size though.

[sdumetz]

Hi,
just a friendly reminder, do you think you could get this merged?

I think everything was OK except maybe the directional light helper's size. Do you want me to scale it up?

Let me know if there is still issues on your side.

[gjcope]

Thanks for the nudge. I believe there were some usability questions as well but I will revisit and update here.

[gjcope]

Hi @sdumetz,

I think there are still some issues to resolve.

1. I understand now what you are trying to do with the shadow camera near/far visualization, but I think there are a couple of issues. The placement of the shadow range visualization isn't accurate in some cases. For instance in the pic originally posted above, the actual near/far range encompasses the model and casts an accurate shadow. But even if this is fixed, I think it's not very intuitive. I think the directional vector should always start at the helper to align with standard representations in other viewers/3D software. Maybe some additional coloring or geometry could be added to show this range?
2. The scene bounding box no longer shows when selecting the scene.
3. The axes visualization is dependent on render order. If the object has a higher index, the axes will be hidden. We should probably do this in a pos-render callback similar to other 'always on top' helpers.
4. I did some light user testing and most everyone though that the axes were a widget that could be manipulated. Specifically for the lights, I think the colored axis inputs would really help to connect the concepts and add usefulness. Otherwise it seems to be a little distracting to people. Have you piloted/user tested this at all? Curious what the feedback was.
5. At certain scales the axes and the grid in the pose task visualization are the same size and result in z-buffer flicker.

[gjcope]

Scene scaling (via an item unit change in the Pose task) also seems to cause issues with the light widget vector lines.

[gjcope]

@sdumetz Had a chance to dig into the issues above a little today. Proof of concept for some fixes found at https://github.com/Smithsonian/dpo-voyager/tree/Holusion-origin_helpers_debug - not suggestions for merge - some are pretty hacky, just demonstrating the issues.

1. Looks like the sizing issue (and scene scaling comment later on) are due to the light group transform scaling that we are doing with directional lights to ensure good shadow coverage and appropriate widget placement. If we compensate for that (hacky example:

   dpo-voyager/libs/ff-three/source/lights/DirectionalLightHelper.ts

   Line 46 in 22d27c6

   this.target.scale.setScalar((this.light.shadow.camera.far - this.light.shadow.camera.near)/this.parent.parent.scale.x);

   ) it looks as expected to me. I still think it would be useful though (for lights with a direction) to have the direction vector go from the helper through the object to have a consistent visualization of direction and then layer this shadow range visualization on top. Or maybe via a multi-point line so that the center section can have a different color.

Also noting that the directional light auto-scaling in CVScene was meant to address the "why aren't my lights casting the correct shadow" issues without user expertise/intervention. It would be great to revisit the issues you've seen here as, at least on our end, I'm not sure most of our users would understand the concept well enough to fix.

2. This is due to the changes in CPickSelection. To get the right component for scene selection, it needs to be getting the CTransform (as previously). Another hacky proof-of-concept here (

   dpo-voyager/libs/ff-scene/source/components/CPickSelection.ts

   Line 111 in 22d27c6

   const transform = node.typeName === "NVScene" ? node.getComponent(CTransform, true) : node.getComponent(CObject3D, true);

   ). This also required creating another map to track axes separately from brackets since with the scene the component and transform are one and the same.

3. I was wrong here. We weren't post-rendering the other helpers. The difference here is that the axes are children of the object where the brackets are not. The code for changing model render order was updating all children to the same value. I modified it to a delta instead. It should work, but not fully tested yet.

Didn't get a chance to look at 4 or 5, but they shouldn't be too serious.

[sdumetz]

Branch updated to latest release.

1. Updated with what should be a more robust implementation of your scaling proposition (iterating through all parents). I removed the near plane materialization completely.
2. Seems Right.Storing brackets and axes in the same map wasn't very robust. I disabled Axes helper for the scene though because it didn't make much sense.
3. I think we were post-rendering, but not anymore. I picked your fix, saw no problems with it.
4. Yes. I'd like to make them clickable in the future. For now coloring the property view would already be an improvement. I see two ways to do it : Through the property's semantic attribute (already used for colors), or by surfacing the property's name in the component's attributes and special-casing anything we want in the CSS.
5. I'll look into it while we sort out the rest.

I kept the color-coding minimal for now with just the labels, not sure if coloring the whole input would be better. I'll ask around for opinions.

Thanks for the feedback and the fixes!

[sdumetz]

def5eec should fix z-fighting by delegating the axes drawing to the CVGrid and @ff/three/Grid components.

The normal Axes helper can still flicker a bit with the grid if it is outside it's object's occlusion, but I don't think it would matter too much.

[gjcope]

Nice update, thanks! Everything looks great except still seeing some scaling issues like in 1) above. In some cases the light vector is significantly shorter than the actual far plane (shadows are projecting fine) and the scale also seems to change with unit changes.

[sdumetz]

Going to the far plane wasn't any good because it's generally too far away to be meaningful (50*shadowSize).

I think I nailed the origin thing. I was very confused by the maths in CVScene.updateLights but now I have something that always points from the source to the scene's origin point even when changing scene units.

Of course if you edit the light's transform it stops pointing at the origin. An elevation / azimuth control for directional lights would be easier to control but that'd be another matter.

I also had to add a scale factor when changing the scene units. Also took the opportunity to test with Spotlight helpers which now scale OK when switching scene units.

I'm still seeing some inconsistencies when changing scene units due to how some values are internally scaled (model.position) vs transform-scaled (DirectionalLights) and I don't know which one I should apply to other lights.

Due to this, Axes helpers for lights are not properly scaled when changing the scene's units.

[gjcope]

I think I nailed the origin thing. I was very confused by the maths in CVScene.updateLights but now I have something that always points from the source to the scene's origin point even when changing scene units.

Looks good. Nice work!

Of course if you edit the light's transform it stops pointing at the origin. An elevation / azimuth control for directional lights would be easier to control but that'd be another matter.

Of course, that makes sense. Improved light controls have been on our list for some time.

I'm still seeing some inconsistencies when changing scene units due to how some values are internally scaled (model.position) vs transform-scaled (DirectionalLights) and I don't know which one I should apply to other lights.

Due to this, Axes helpers for lights are not properly scaled when changing the scene's units.

Do you have a specific example I can test? I may be able to provide some clarity. I played around with it and did not see any scaling issues.

[sdumetz]

I'm still seeing some inconsistencies when changing scene units due to how some values are internally scaled (model.position) vs transform-scaled (DirectionalLights) and I don't know which one I should apply to other lights.

Due to this, Axes helpers for lights are not properly scaled when changing the scene's units.

Do you have a specific example I can test? I may be able to provide some clarity. I played around with it and did not see any scaling issues.

That's because it doesn't happen on DirectionalLights. They get their transforms rescaled on unit change, which changes their axes' scale.
But lights that have a physical size (spots, areas) should have a helper that matches this size so messing with their scale breaks them (eg: A spot light's effective distance isn't multiplied when it's scaled).

Models don't have this issue because their axes are sized from their bounding box so they are always OK.

So Axes for those lights are always "1 scene-unit" for now. I could force scale to affect the light's distance by internally multiplying them.

That should probably not be considered blocking though because there is no practical way to create those light types anyway. I can always come back to it later.

[gjcope]

That's because it doesn't happen on DirectionalLights. They get their transforms rescaled on unit change, which changes their axes' scale. But lights that have a physical size (spots, areas) should have a helper that matches this size so messing with their scale breaks them (eg: A spot light's effective distance isn't multiplied when it's scaled).

Yes, of course. It would still be useful to have a functional example of you're talking about. Maybe I'm interpreting it wrong, but if a spot light is scaled, it's effective distance should change accordingly.

That should probably not be considered blocking though because there is no practical way to create those light types anyway. I can always come back to it later.

That's fine with me if you don't mind opening an issue so it doesn't get lost as a todo.

Thanks for all the work!

[gjcope]

Merged to RC-52

Nothing actionable here at the moment, but did want to note that this functionality adds some size to the Explorer build, even though the functionality isn't used there. I don't see a way it's easily avoidable at this stage due to the core integration of CSelection, but my goal is always to try and keep as much bloat out of Explorer as possible. Again, nothing actionable now but just something to keep in mind. Thanks!