Improve batched model rendering

[loshjawrence]

@xintongxia

Few issues I saw looking at it this morning:

Btw, I was using the examples/cesium/3d-tiles example since the deck.gl/3d-tiles one seems broken at the moment.

1) math.gl/culling/bounding-sphere.js distanceSquaredTo can produce negative distances. Sqrt of this is NaN and corrupts all further downstream calculations. Fixing this does help de-congest traversal when the camera is inside the root bounding sphere.

2) The renderTilePrimitive function in tileset-loader.js is just blindly adding tiles as they load (definitely should not do this). tileset3d.selectedTiles tells you all the tiles that need to be rendered this frame for the given view and loading state of the tree.

tileset-loader.js code (app code) needs to be doing something like the old app (deck.gl 3d-tiles) where there's some coordination between selectedTiles (an array on tileset-3d that refreshes every frame update. These are the tiles that are visible and ready to render) and the layerMap / viewer.scene.primitives

3. it looks like we are doing two redundant loads/conversion steps: 1 inside tileset-3d.js (module code) and one inside tile-parsers.js (app code). It looks like Omar did this because cesiums render classes consume the payload directly and would get confused about the loaders parsed version.

4. Another problem in the loading lies in tile-3d-header::loadContent. I forget what parse does in there but if remember correctly its' just stuff like draco decompress or preparing some metadata about the binary. Anyway in that function tile._contentState is getting set to READY when that's not the case. READY is for ready-to-render so this state can only be set when the app code is done converting the payload to whatever it uses to render 3dtiles.

[ibgreen]

tile._contentState is getting set to READY when that's not the case. READY is for ready-to-render so this state can only be set when the app code is done converting the payload to whatever it uses to render 3dtiles.

Yes this is definitely true in the Cesium code base. However, since loaders.gl doesn't handle rendering, my assumption when porting this code was that we would just redefine the meaning of READY to just mean fully loaded. If we don't do this, we will mix application managed state with loaders.gl managed state.

If this is an issue for Cesium integration, we can of course change it. Perhaps we could avoid confusion if if we just changed the name of the flag to LOADED (instead of READY) and updated the docs? Or just update the docs to clarify?

[ibgreen]

it looks like we are doing two redundant loads/conversion steps: 1 inside tileset-3d.js (module code) and one inside tile-parsers.js (app code). It looks like Omar did this because cesiums render classes consume the payload directly and would get confused about the loaders parsed version.

Not sure if there is a quick fix, but this seems like a good (i.e. relatively small) next step on the Cesium side towards full loaders.gl integration, i.e. split the classes in the Cesium repo so the file parsing part is optional and can be cleanly replaced with a small adapter method that takes the parsed tile from loaders.gl and renames the fields to what Cesium expects?

Also I think @OmarShehata indicated the loaders.gl integration actually worked reasonably well for the b3dm since Tile3DLoader provides options['3d-tiles'].parseGLTF which can be set to false to glTF parsing and instead just return a field with the extracted glTF ArrayBuffer (or URL) that you can then just pass to Cesium's own glTF parser, and all remaining batched/instanced 3d tile parsing could be handled by loaders.gl.

[ibgreen]

math.gl/culling/bounding-sphere.js distanceSquaredTo can produce negative distances. Sqrt of this is NaN and corrupts all further downstream calculations. Fixing this does help de-congest traversal when the camera is inside the root bounding sphere.

@loshjawrence This sounds like an important finding!

Just took a look. While we can add some checks or an assert inside the

• BoundingSphere.distanceSquaredTo() method in math.gl, it seems to be a direct port of the
• Cesium equivalent

which makes me wonder if we are corrupting the bounding spheres or forgetting to transform them or something along those lines.

@xintongxia Maybe put a conditional breakpoint that triggers on negative distances and see what inputs are causing these?

[loshjawrence]

Looks like the cesium code still allows a negative result. if the cam pos is in the sphere it would go negative. I don't see a great reason that it would allow negative distances and is confusing from API point of view since you would assume that a squared value is positive. Anyway, in cesium the calling code does a Math.max(result, epsilon7) somewhere down the line. We should do this clamp to epsilon in tile-3d-header any way to avoid divide by 0, guess this got lost somewhere along the way.

[loshjawrence]

There's a state called PROCESSING to indicate that we've received the request but we are still processing the payload it for rendering.When the app code finally converts it, the state should switch to READY.

[loshjawrence]

There's a some-fixes branch. It's still quite buggy but can produce things like

It has still has a disappearing parents issue. Not sure why this is happening. There's also some strange culling issues that remain.

[loshjawrence]

@xintongxia The missing parents issue only happens during tile loading but once it's loaded there's no missing parent issue. Like if you cycle zoom out / zoom in, the replacement happens correctly but only for potions of the tree that we've already fetched and processed. This means we are still switching to READY when that's not the case during processing, so the primitives being returned from loadBatchedModelTile(uri, tileHeader) are actually still being processed.

To confirm this issue with loadBatchedModelTile, if I delay the makeReady() call to 10 frames in the future, the disappearing parent issue goes away, at least on my machine.

@OmarShehata any idea why this might be the case? Is the call to Model doing something funny that we need to listen to or wait on?