Optimize size of BoundingBox

[Project-PLATEAU-Admin]

Builds on (#1045).
This makes BoundingBoxes smallest for each primitive.

[cesium-concierge]

Thank you so much for the pull request @Project-PLATEAU! I noticed this is your first pull request and I wanted to say welcome to the Cesium community!

The Pull Request Guidelines is a handy reference for making sure your PR gets accepted quickly, so make sure to skim that.

• ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

• Cesium for Unreal Samples works.

[kring]

Thanks for the PR @Project-PLATEAU!

I think there are two mostly unrelated changes in this PR. The first changes how an axis-aligned bounding box is computed from a primitive. Previously, it would trust the min/max values on the position accessor, if available, or compute them from all the positions if not.

Now it never trusts the accessor's min/max, and computes them from positions referenced by the index accessor.

In the common case where a tile has only one primitive, these are equivalent, except that the old implementation is much more efficient because it doesn't need to loop over the index array and use it to random access into the position array. They're also equivalent where the tile has multiple primitives but those primitives each have their own vertices. The only time the new implementation provides a better result is when the primitives have different indices but the same vertices. Oh and also the vertices used by the two sets of indices also need to be in different places within the model, otherwise the bounding volumes won't be very different, anyway.

In my experience, this is really not common. In fact, if you have this scenario, it will be quite inefficient in Cesium for Unreal in general because Cesium for Unreal will create a separate copy of the vertices for each primitive that uses them. I'm not sure whether Unreal's StaticMeshComponent gives us the tools to easily address this.

The other change in this PR is to avoid setting the primitive's boundingVolume property, which has the effect of disabling the bounding volume improvement we implemented in #823 and #841. Again, I don't think this a worthwhile tradeoff, as those PRs made a significant improvement in our bounding volumes, especially near the georeference origin. We don't want to lose those improvements without a very good reason.

So in all, I'm not completely sold on the value of this change, because it seems to make common cases less efficient while making an uncommon one faster, which is probably not a good tradeoff. But I'm interested in hearing more about how you've ended up with this seemingly-uncommon scenario and what improvements you've seen from the change in this PR.

But I think it's worth noting that if you really do have this scenario, it may be very worthwhile to restructure your tileset to avoid it. The 3D Tiles selection algorithm is driven by the bounding volumes in tileset.json. If your bounding volumes are not representative of your content, because there are multiple different "parts" in different locations all lumped into the same tile, then the selection algorithm is going to do a lot more loading than is necessary. Tile loading is usually much more expensive than rendering, so optimizing the rendering-time bounding volumes will yield only a small improvement. Restructuring the tileset so that tiles have good spatial locality and a minimal number of draw calls (primitives) will improvement performance significantly, and also avoid the need for the sort of changes in this PR.

Happy to discuss further!

[Project-PLATEAU-Admin]

The first change minimizes the bounding box of primitives, even when multiple gltf primitives share one position accessor. However, this change is applied only when primitives share the position accessor, considering other cases as well.

The second change is a modification to reflect the bounding box of each primitive in physics calculations. Since this may be inefficient in use cases that do not require physics calculations, it has modified to make it an option for 3DTilesetActor, so it can be chosen as needed.
This change is required when the CreateNavCollision option added in #1044 is enabled, so it only applies when that option is enabled.

These changes improve physics simulation when moving characters using navigation on the Tileset.
For example, if a building defines primitives for each floor, the physics calculations for character movement can be reduced to only the floor the character is on, rather than the entire building. This is more effective when moving many characters.

[Project-PLATEAU-Admin]

Our apologies.
There was a line that was accidentally deleted and I have added a commit to undo it.

[kring]

Sorry I'm so long in coming back to this, @Project-PLATEAU. I'm still not convinced of why this is needed, though. I understand you have a single tile with a separate primitive for each floor. And it's more efficient if each primitive's bounding volume only covers that floor, rather than the entire building. But since floors are unlikely to share (many) vertices anyway, I think you would find it is much more efficient - and avoids the need for this PR - to have a separate position accessor for each floor. Using a single position buffer with multiple index buffers will be much less efficient, with or without this PR, because each floor primitive will get a copy of all the positions, even if they're not used.

So, can you tell me what I'm missing there, if anything? And if you still think this PR is necessary in spite of that, can you please walk me through it so that I can understand?