Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong bounding volume computations for model #12108

Open
javagl opened this issue Aug 5, 2024 · 1 comment
Open

Wrong bounding volume computations for model #12108

javagl opened this issue Aug 5, 2024 · 1 comment

Comments

@javagl
Copy link
Contributor

javagl commented Aug 5, 2024

This code in ModelSceneGraph.js is trying to compute the model bounding volume (modelPositionMin/Max) from the primitive (primitivePositionMin/Max). This is done by transforming the min/max of the primitive with the node transforms, and computing the component-wise minimum and maximum from the transformed minimums/maximums.

This will yield wrong results in most cases.

A 2D illustration:

Cesium BoundingVolumes 0001

  • Assume that there is a primitive with a min/max of (0,-1)-(6,1).
  • It is once attached to a node with an identity transform (right)
  • And once attached to a node with a rotation about 180° (left)
  • The maximum point will become the "leftmost" one due to the rotation. But it will not contribute to the global minimum (because it was the maximum, originally).

The following is an archive that contains two glTF assets, and a Sandcastle for testing this:

Cesium Model Bounding Volume Issue 0001.zip

The glTF assets are visually equal, and similar to the 2D case above:

  • They contain one primitive, which is a box that ranges from (0.0, -0.5, -0.5) to (10.0, -0.5, -0.5)
  • The primitive is attached to node with the identity transform
  • In the translated asset, it is also attached to a node with a translation of -10 along x
  • In the rotated asset, it is also attached to a node with a rotation of 180° around the z-axis

So both assets look like this:

Cesium Model Bounding Volume

And gltf.report prints the proper bounding box for both of them.

The sandcastle loads both models, and prints their bounding sphere information:

Cesium Model Bounding Volume Sandcastle

The bounding sphere is wrong in the case of the 'rotated' second primitive.

Yeah... the bounding sphere that is shown via debugShowBoundingVolume is wrong in both cases ... I'm not sure where this is coming from - and/or whether it should be tracked as its own issue, or whether this issue sufficiently covers that ~"a few things are wrong here...".


Possible solutions:

There are at least three possible ways of improving this:

1. Simple but probably bad

One could compute the model minimum/maximum with

modelPositionMin = minimumByComponent(modelPositionMin, primitivePositionMin);
modelPositionMin = minimumByComponent(modelPositionMin, primitivePositionMax); // Max as well!

This should fix it for the overly narrow test case here. But this could also have corner cases (no pun intended) with rotations about 45° and non-uniform scaling and such.

2. Still simple, and probably OK

One could compute the bounding box from the transformed corner points of the primitive bounding box. So for each primitive, its min/max (as taken from the POSITIONS accessor) are converted into an oriented bounding box. This oriented bounding box is transformed with the node transform. The (corners of) the transformed bounding box are used for computing the min/max for the whole model.

3. Most precise

All vertex positions from the POSITIONS attribute are transformed with the node transform, and the bounding volume of the whole model is computed from these transformed vertex positions.
This is likely prohibitively expensive for larger models (not to mention having to ~"keep stuff on the CPU" and whatnot).


All this is still not taking into account skinning, animations, morphing, or instancing. The latter is what I actually tried to address via #12105 , and where me wondering why I wasn't able to compute "the right" bounding volumes led me to this issue...

@javagl
Copy link
Contributor Author

javagl commented Aug 7, 2024

I started drafting something locally to at least conveniently test this. It's in a rough shape, but I'll dump it here: It's a sandcastle that creates a glTF asset that contains

  1. one unit cube with identity transform
  2. one unit cube with (optional) translation/rotation/scale

It creates these assets in-memory, and shows them, together with their bounding sphere.

(I mean, their real bounding sphere - not the wrong one that is displayed with debugShowBoundingVolume)

Cesium Bounding Volume Test Sandcastle

The sandcasle (work in progress):

https://sandcastle.cesium.com/index.html#c=xRhrb9s28K9w/iRjrmQ7aZrZSTDGSTtvbhPESbeuDgZaom2u1AMk5TQb8t93pCybkqXESQtMECTd8d48Ho/y40gqtGT0jgp0jCJ6hwZUsjR0PxqcM2n4Bh7EkSIsomLSaKF/JxFCcx5PaQ/NCJe0NYkemv1JNIlmaeQrFkfIF5Qoeh5OaRDQ4CZiapBO6TuuZo4SJJKcaLIWErFafUmfcNrMhPvGLkFlyhXYZXAIER+MkbGQPfQ5w6B8SF/TdDajQhveQ+2Whb9X9GI2k1QV8X4cJnFEI3V9n4AnrzvdvcJoGgHD3oGFU4Zw0hgP8AhfQSg2QyH5ClZ1925tHIsA196gHlpPmN15kdkH22Z39yvM/ng+2KswuuO2Wyh/VNivR/LHd/Cle3j4P3rzygxtntsO5Rgijblr7+YU0p+oWGjlv8759Vs0E3GIFkolsud5c6YW6dQFh7y/yZLMuWeILBOXVEhIdc3fddvrgVxxFrya5E4F03wBUaRHkoQz3ywbbw4r6lXG2Z8SSQ/2WxjjAcbn8MJnGA/xKcbvML7R718w/qQHf8P4i36PMI410QXGbf2+xHhfc47x6bl+X+PTIX6L8e/49Ea//8Cnn/D2NcCXd9vw/OywQLWBLfoh9gpEFmzoDWzJs+BK/Y/Ql+2p1F9pb9mean8q4Up/yvqr7d/pGg68b4PxC+BH4r8jvPwm+JH5eQ48ehlcKDq6so1oNFeLHjrYP6ytJpvK+OjutdvOZSt907VrIxFzTby3/9PB3i6lul5fQa6t8PWbg9LIWAkWQE3u1JnSrQ1LSOWC1kQkESxkii0L4yUaU6gV6J+mStOVhhC6vBgPr4cXH4r7UXZ9uLh6j0ewvZRGHkowiwLma+nt0kAYa7f3C1ibuX5ziYCzxm0dEltV5eyViSDem66qZwMWSd5r9TZd12bQtF+97FVrtvRhE9zoNWCNG7mHeiOui8NDXz8FVamIVt0eYB50I8mpQn4qBHQGZ3Sazk+hEwhYNB8nCyroeaSYuu8XyN6DQn6Z50ypG+WUiAo5zqrfZDPkPKUN/XB8jABNZ9AHB83c16x5dqmmYVS6gobxkj4prdnP2J/Uauk0LA+rAK19k4v4rsq1aQHMG+vaQPStvhuUFrldQQKWyozmaYvLMSFB4KzClcSSZTlY0gCppKhYpQflnCUyZoG1orUJ0ANZZ5QBEbDoGYn2HAEnCbib60QL4fQhGOG9NXHMY+FenZ+5d9CoYZ4siNN2uxuOOFWc6exWIqVlrOHellXo4MwR6GE770xiVmRaMWEfTy+z0txNRSylWVFUKbdKeuoSisj7yEeFtMos3+W8ZvlpVl6eSrpCclBJ7ghTefgMoaubZ30exFpvnh66hb0kepuZNPLvvyYN9CN6D1+QhlEQh04+abr57b34qLkS4nmBTuUx+Jun88eYp+EqE9YzqymNO655gj2CfQXXcqcMvG/cut6odTJGVJO3rzr6XAV3s2VTrIQ5zaa95LYmsi459HozRuaTkdkNcQruz5cgSVOYjxGTSp9rHMjP45M86fTcxZy6PJ47k0Y2h8Xl2ps01llmEWd6Klf2zuRZqcnJ66pbFWvGU7EMYWuhakBCOL85hT8MvsGVI5lhs8ibTzevWsV/JNY8ZrPYaTdtroAJ6q/YtljciM4hbZ3Mze3hmw/D67/+zNuFSqVNPVjQmCaVqnwOR2ynRsendbw87/g7XloeGsOC9YlUnKKbIYJZSJNexX+iMQkTTi8Ss2J2qDd624/AZfDWooYqoX9PmXKR89k4I8FuOVbpruhXaFS1vFW4ITcoh6nroVJRdJqb/SgraTuWyX5ho8gDniWhtJyX4JHppirisi7bLVT1mdWzCr7Pr7MSc/tMPov4cycTUZKyE6teFy9nXXE/l9WW0m0hfd8+X+XTrFZsH2G9hQnfrARdfK/jmE9hy6RR6hTmP6vYlcSnqVJ6Q2lc6XK2WvI6ua3aXah0/fVv2TK60WocSXXP6UmezT+zMImFQqngjut6ioJN4Kz0pqn/hSrXl+uSfOTZrEcBWyIWHFf8KYa2gEgJI7OU8zH7h04aJ0ce0G+x8pjoMn6xpIKTe0226JyMMqTrukcegNWcKotNSfJ/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment