Fix ellipsoid tracking

[bagnell]

Bounding spheres were transformed by the model matrix and cached. If the model matrix was changed, the cache was never updated. This change returns transformed bounding spheres by the current model matrix on property get.

Fixes #4929.

[mramato]

@emackey can you test this out in your specific use cases, I just want to be sure this is squashed for good.

Thanks @bagnell!

[mramato]

Just to clarify, the bounding sphere provided by Primitive is always guaranteed to already have it's transform applied now? Historically that wasn't the case, and a few months ago it became the case sometimes. I just want to understand what the behavior is supposed to be so that we don't have future issues.

[bagnell]

Just to clarify, the bounding sphere provided by Primitive is always guaranteed to already have it's transform applied now?

Yes, all bounding volumes should be in world coordinates.

[emackey]

My first test of this showed correct behavior with an example ellipsoid, horray!!

For my second test, I copied all the sample code from #4929, and replaced these two lines:

    "ellipsoid":{
      "radii":{

with this:

    "box":{
      "dimensions":{

With this edit to the demo, the selectionIndicator still works fine, but the camera tracking is broken. The camera zooms to wherever the object is when you double-click it, but as time animates forwards, the object flies away from the camera.

Note that master has the same behavior with camera tracking of dynamic boxes, so this is not something new being introduced here. It may actually be mostly unrelated, and I would be fine with merging this fix without a box fix.

[mramato]

Thanks @emackey I'd like to take a look at the box issue before merging this since it should be trivial and probably also means everything BUT ellipsoid is broken (since ellipsoid is a special case and everything else goes through the same code path).

[mramato]

This is a tough one.

1. "Dynamic" geometries are for the most part just create a new Primitive synchronous primitive every frame.
2. Since the primitive is created in the same frame that Viewer._onTick is trying to use it, the primitive is never "ready" and therefore the boundingSphere is always undefined.
3. The only reason this isn't a problem with Ellipsoid is because it reuses the same primitive in 3D and just sets the model matrix (most of the time). Switch to 2D or make something like stackPartitions dynamic and the tracking is broken again.
4. Every "easy" fix I can think of so far causes the camera to lag the primitive by a frame, which is just as broken.

One possible option I've yet to explore is having dynamic geometries provide non-primitive based bounding spheres. This will cause dynamic-ground primitives to still be broken (but they are probably too slow to be usable anyway), but would fix every other case I can think of. Unfortunately, this approach requires custom code for each dynamic geometry so it's slightly more involved.

Anyone else have any ideas here?

[emackey]

This may be a bit sweep-under-rug-ish, but I'll suggest it:

If a dynamic primitive isn't "ready", just use Entity.position or the Entity's other bounding spheres without paying attention to the missing bounding sphere.

Here's my reasoning: The main thing we need the bounding sphere for is the size of the dynamic object at the moment the camera starts tracking it. For example, if you start tracking a small box or ellipsoid, the camera will be placed closer than if you start tracking a large one. However, once you begin tracking, the size no longer matters: The camera just stays at the same relative distance, or the user adjusts that distance, but the camera tracking doesn't automatically adapt to size changes. The entity may grow and swallow the camera, and it's on the user to back the camera away (this is by design, to show the entity growing and shrinking). Therefore, getting new bounding spheres after the tracking begins shouldn't be needed, I believe, as the camera is just following along with the position after tracking has already started.

Since all that's broken here is the post-tracking camera position update, I think that should be able to happen based on position alone. The bounding sphere size is only needed at the start of tracking.

[mramato]

Since all that's broken here is the post-tracking camera position update, I think that should be able to happen based on position alone. The bounding sphere size is only needed at the start of tracking.

This is only true for items that are not clamped to terrain. If the item is clamped to terrain (or doesn't have a position like polygon) then the bounding sphere is the only place with the correct information.

[emackey]

Since this does fix a bug with Ellipsoids, and doesn't change existing behavior for non-ellipsoids, can we merge it for the next release?

[mramato]

Since this does fix a bug with Ellipsoids, and doesn't change existing behavior for non-ellipsoids, can we merge it for the next release?

I'm okay with that, but let's write up an issue that summarizes the current state and problems so we fix it.

[pjcozzi]

Also, please update CHANGES.md.

[emackey]

Moved box bug to #5164, it's unrelated to this.

Thanks much @bagnell!