Repository: NASA-AMMOS/3DTilesRendererJS
Pull request #667 (2024-08-03 14:33:47)

Remove support for loadSiblings, stopAtEmptyTiles options
Related to #142
Related to #662 
Related to #663 

This support removes the "loadSiblings" flag and adopts the "true" behavior for "REPLACE" refine and the "false" behavior for "ADD". Now "ADD" will no longer force sibling geometry to load while "REPLACE" will.

The general goal is to ensure that when refine === "REPLACE" that we always have coherent sibling tiles loaded such that there is no "pop" when turning the camera and other tiles haven't been loaded. This is done by ensuring that any children are marked as "used" down to the next tile with content so we can be resilient to content "gaps" in the tile set from parent tiles with no geometry.

This enables the OSM tile set from #662 to load a lot faster but doesn't address the France tile set from https://github.com/iTowns/itowns/issues/2335. It's likely that the sibling behavior isn't correct - ie we could be considering tiles with no geometry as a "valid" sibling rather than traversing to children the next content-ful child (assuming the empty parent isn't an external tile set json). This would render the "loadSiblings" options and refinement difference irrelevant, I think.

OSM buildings marked in red:
<img width="400" alt="image" src="https://github.com/user-attachments/assets/e9eca737-e892-408e-9fc7-76c2b86f49f3">

cc @jailln 

_edit The Google globe tile set includes some intermediate "empty" tiles that are not external tile sets but they all use a geometric error of 1e+100 which may provide some hint as to how this should be handled._

<details>
<summary>Needed Traversal Improvements</summary>

- "REPLACE" parent tiles should only trigger loads for immediate children - or at least only up to calculated error requirements - rather than [loading up to the next set of visible children](https://github.com/NASA-AMMOS/3DTilesRendererJS/blob/master/src/base/traverseFunctions.js#L58), which may be incredibly broad and deep.
- If an in-frustum REPLACE parent tile has no children in the frustum, then the parent tile should not be considered in the frustum.
- When triggering all "REPLACE" parent children to load we can calculate the SSE even though they may be out of the frustum (see [executeTraversal](https://github.com/CesiumGS/cesium/blob/0a69f67b393ba194eefb7254600811c4b712ddc0/packages/engine/Source/Scene/Cesium3DTilesetBaseTraversal.js#L212-L214) > [updateAndPushChildren](https://github.com/CesiumGS/cesium/blob/0a69f67b393ba194eefb7254600811c4b712ddc0/packages/engine/Source/Scene/Cesium3DTilesetBaseTraversal.js#L99-L101) > [updateTile](https://github.com/CesiumGS/cesium/blob/0a69f67b393ba194eefb7254600811c4b712ddc0/packages/engine/Source/Scene/Cesium3DTilesetTraversal.js#L193) > [updateTileVisibility](https://github.com/CesiumGS/cesium/blob/0a69f67b393ba194eefb7254600811c4b712ddc0/packages/engine/Source/Scene/Cesium3DTilesetTraversal.js#L211) > [Tile.updateVisibility](https://github.com/CesiumGS/cesium/blob/0a69f67b393ba194eefb7254600811c4b712ddc0/packages/engine/Source/Scene/Cesium3DTile.js#L1041)). This information is then used in [executeEmptyTraversal](https://github.com/CesiumGS/cesium/blob/0a69f67b393ba194eefb7254600811c4b712ddc0/packages/engine/Source/Scene/Cesium3DTilesetBaseTraversal.js#L141) to load tiles up to the next valid [error value](https://github.com/CesiumGS/cesium/blob/0a69f67b393ba194eefb7254600811c4b712ddc0/packages/engine/Source/Scene/Cesium3DTilesetBaseTraversal.js#L273).
- Remove check for in-frustum in error calculation

</details>

Nmzik: In my opinion, loadSiblings should remain a configurable option for the user.

> The general goal is to ensure that when refine === "REPLACE", we always have coherent sibling tiles **loaded** so that there is no "pop" when turning the camera and other tiles haven't been loaded

 That’s the point; for example, I am limited by the number of tiles **loaded** in 3DTilesRendererJS LRUCache. I don’t care about the pop-in effect when turning the camera and I want to minimize RAM, VRAM, bandwidth usage etc...

gkjohnson: > In my opinion, loadSiblings should remain a configurable option for the user.

Are you actually using `loadSiblings` for anything? When it's set to `false` the experience feels completely broken with flashing tiles everywhere and gaps in the tile set.

> CesiumJS does provide this option.

This isn't true. Cesium has an option called `loadSiblings` but it defaults to `false` and behaves differently. This confusion is another reason why this option needs to be removed. 

> That’s the point; for example, I am limited by the number of tiles loaded in 3DTilesRendererJS LRUCache. I don’t care about the pop-in effect when turning the camera and I want to minimize RAM, VRAM, bandwidth usage etc...

There are other solutions to these issues, as well. I'd rather discuss the bottlenecks people are facing rather than hold on to a broken setting. If something similar to the current behavior (but fixed in some ways, hopefully) of `loadSiblings` needs to be brought back we can do so with a different name that doesn't conflict with Cesium's naming.

Patch:
diff --git a/README.md b/README.md
index 0801716..dfd8c63 100644
--- a/README.md
+++ b/README.md
@@ -352,14 +352,6 @@ maxDepth = Infinity : Number
 
 The max depth to which tiles will be loaded and rendered. Setting it to `1` will only render the root tile. If the tile at depth `maxDepth` is an empty tile then the next set of visible children will be rendered.
 
-### .loadSiblings
-
-```js
-loadSiblings = true : Boolean
-```
-
-If true then all sibling tiles will be loaded, as well, to ensure coherence when moving the camera. If false then only currently viewed tiles will be loaded.
-
 ### .displayActiveTiles
 
 ```js
diff --git a/example/index.js b/example/index.js
index b146a74..bed7162 100644
--- a/example/index.js
+++ b/example/index.js
@@ -65,8 +65,6 @@ const params = {
 	errorTarget: 6,
 	errorThreshold: 60,
 	maxDepth: 15,
-	loadSiblings: true,
-	stopAtEmptyTiles: true,
 	displayActiveTiles: false,
 	resolutionScale: 1.0,
 
@@ -263,7 +261,6 @@ function init() {
 	gui.width = 300;
 
 	const tileOptions = gui.addFolder( 'Tiles Options' );
-	tileOptions.add( params, 'loadSiblings' );
 	tileOptions.add( params, 'stopAtEmptyTiles' );
 	tileOptions.add( params, 'displayActiveTiles' );
 	tileOptions.add( params, 'errorTarget' ).min( 0 ).max( 50 );
@@ -481,7 +478,6 @@ function animate() {
 	// update options
 	tiles.errorTarget = params.errorTarget;
 	tiles.errorThreshold = params.errorThreshold;
-	tiles.loadSiblings = params.loadSiblings;
 	tiles.optimizeRaycast = params.optimizeRaycast;
 	tiles.stopAtEmptyTiles = params.stopAtEmptyTiles;
 	tiles.displayActiveTiles = params.displayActiveTiles;
diff --git a/example/landformSiteOverlay.js b/example/landformSiteOverlay.js
index 6af47fe..32a697d 100644
--- a/example/landformSiteOverlay.js
+++ b/example/landformSiteOverlay.js
@@ -9,6 +9,7 @@ import {
 	Group,
 	TextureLoader,
 	MeshBasicMaterial,
+	SRGBColorSpace,
 } from 'three';
 import { GUI } from 'three/examples/jsm/libs/lil-gui.module.min.js';
 import { JPLLandformSiteSceneLoader } from './src/jpl/JPLLandformSceneLoader.js';
@@ -95,6 +96,7 @@ function init() {
 			.loadAsync( url )
 			.then( tex => {
 
+				tex.colorSpace = SRGBColorSpace;
 				tex.flipY = false;
 				return tex;
 
diff --git a/example/src/plugins/overlays/TextureOverlayMaterial.js b/example/src/plugins/overlays/TextureOverlayMaterial.js
index 23b95a5..43ae0e5 100644
--- a/example/src/plugins/overlays/TextureOverlayMaterial.js
+++ b/example/src/plugins/overlays/TextureOverlayMaterial.js
@@ -1,9 +1,12 @@
+import { Color } from 'three';
+
 export const TextureOverlayMaterialMixin = base => class extends base {
 
 	constructor( ...args ) {
 
 		super( ...args );
 		this.textures = [];
+		this.overlayColor = new Color( 0x84ffff );
 		this.displayAsOverlay = false;
 
 	}
@@ -21,6 +24,14 @@ export const TextureOverlayMaterialMixin = base => class extends base {
 			},
 		};
 
+		shader.uniforms.overlayColor = {
+			get value() {
+
+				return material.overlayColor;
+
+			},
+		};
+
 		shader.defines = {
 			DISPLAY_AS_OVERLAY: Number( this.displayAsOverlay ),
 		};
@@ -31,6 +42,7 @@ export const TextureOverlayMaterialMixin = base => class extends base {
 			shader.fragmentShader = shader.fragmentShader
 				.replace( /void main/, m => /* glsl */`
 					uniform sampler2D textures[ ${ textures.length } ];
+					uniform vec3 overlayColor;
 					${ m }
 
 				` )
@@ -46,7 +58,7 @@ export const TextureOverlayMaterialMixin = base => class extends base {
 
 						#if DISPLAY_AS_OVERLAY
 
-						diffuseColor = mix( diffuseColor, vec4( 0.22, 0.73, 0.82, 1 ), col.r * 0.5 );
+						diffuseColor = mix( diffuseColor, vec4( overlayColor, 1.0 ), col.r * 0.5 );
 
 						#else
 
diff --git a/src/base/TilesRendererBase.d.ts b/src/base/TilesRendererBase.d.ts
index 1d50493..342dc7e 100644
--- a/src/base/TilesRendererBase.d.ts
+++ b/src/base/TilesRendererBase.d.ts
@@ -8,10 +8,8 @@ export class TilesRendererBase {
 
 	errorTarget : Number;
 	errorThreshold : Number;
-	loadSiblings : Boolean;
 	displayActiveTiles : Boolean;
 	maxDepth : Number;
-	stopAtEmptyTiles : Boolean;
 
 	fetchOptions : RequestInit;
 	/** function to preprocess the url for each individual tile */
diff --git a/src/base/TilesRendererBase.js b/src/base/TilesRendererBase.js
index 7c2a514..9652703 100644
--- a/src/base/TilesRendererBase.js
+++ b/src/base/TilesRendererBase.js
@@ -92,6 +92,18 @@ export class TilesRendererBase {
 
 	}
 
+	set loadSiblings( v ) {
+
+		console.warn( 'TilesRenderer: "loadSiblings" option has been removed.' );
+
+	}
+
+	set stopAtEmptyTiles( v ) {
+
+		console.warn( 'TilesRenderer: "stopAtEmptyTiles" option has been removed.' );
+
+	}
+
 	constructor( url = null ) {
 
 		// state
@@ -130,10 +142,8 @@ export class TilesRendererBase {
 		// options
 		this.errorTarget = 6.0;
 		this.errorThreshold = Infinity;
-		this.loadSiblings = true;
 		this.displayActiveTiles = false;
 		this.maxDepth = Infinity;
-		this.stopAtEmptyTiles = true;
 
 	}
 
diff --git a/src/base/traverseFunctions.js b/src/base/traverseFunctions.js
index 234db8c..481a267 100644
--- a/src/base/traverseFunctions.js
+++ b/src/base/traverseFunctions.js
@@ -130,16 +130,14 @@ export function determineFrustumSet( tile, renderer ) {
 	const frameCount = renderer.frameCount;
 	const errorTarget = renderer.errorTarget;
 	const maxDepth = renderer.maxDepth;
-	const loadSiblings = renderer.loadSiblings;
 	const lruCache = renderer.lruCache;
-	const stopAtEmptyTiles = renderer.stopAtEmptyTiles;
 	resetFrameState( tile, frameCount );
 
 	// Early out if this tile is not within view.
 	const inFrustum = renderer.tileInView( tile );
 	if ( inFrustum === false ) {
 
-		return false;
+		return;
 
 	}
 
@@ -151,22 +149,24 @@ export function determineFrustumSet( tile, renderer ) {
 
 	// Early out if this tile has less error than we're targeting but don't stop
 	// at an external tile set.
-	if ( ( stopAtEmptyTiles || ! tile.__contentEmpty ) && ! tile.__externalTileSet ) {
+	// TODO: it's possible we should remove this external tile set check, too. The tile set
+	// should embed the necessary logic internally to stop or continue traversal.
+	if ( ! tile.__externalTileSet ) {
 
-		// compute the _error and __distanceFromCamera fields
+		// compute the __error and __distanceFromCamera fields
 		renderer.calculateError( tile );
 
 		const error = tile.__error;
 		if ( error <= errorTarget ) {
 
-			return true;
+			return;
 
 		}
 
 		// Early out if we've reached the maximum allowed depth.
 		if ( renderer.maxDepth > 0 && tile.__depth + 1 >= maxDepth ) {
 
-			return true;
+			return;
 
 		}
 
@@ -178,14 +178,14 @@ export function determineFrustumSet( tile, renderer ) {
 	for ( let i = 0, l = children.length; i < l; i ++ ) {
 
 		const c = children[ i ];
-		const r = determineFrustumSet( c, renderer );
-		anyChildrenUsed = anyChildrenUsed || r;
+		determineFrustumSet( c, renderer );
+		anyChildrenUsed = anyChildrenUsed || c.__used;
 
 	}
 
 	// If there are children within view and we are loading siblings then mark
 	// all sibling tiles as used, as well.
-	if ( anyChildrenUsed && loadSiblings ) {
+	if ( anyChildrenUsed && tile.refine !== 'ADD' ) {
 
 		for ( let i = 0, l = children.length; i < l; i ++ ) {
 
@@ -196,7 +196,7 @@ export function determineFrustumSet( tile, renderer ) {
 
 	}
 
-	return true;
+	return;
 
 }
 


Test patch:
diff --git a/TESTCASES.md b/TESTCASES.md
index 8741bc8..c9a2f76 100644
--- a/TESTCASES.md
+++ b/TESTCASES.md
@@ -2,12 +2,11 @@
 
 Series of manually performed test cases for scenarios that are difficult / not feasible to test in an automated fashion.
 
-## Verify all sibling tiles load when loadSiblings = true
+## Verify all sibling tiles load
 
 #### steps
 
 1. Open the kitchen sink example.
-1. Ensure `loadSiblings` is enabled.
 1. Ensure `displayActiveTiles` is enabled.
 1. Zoom in as much as possible so much of the tileset is not visible.
 1. Wait until all tiles have loaded.
@@ -18,18 +17,6 @@ Series of manually performed test cases for scenarios that are difficult / not f
 
 Tiles are displayed all the way out to the edge of the tileset.
 
-## Verify sibling tiles do _not_ load when loadSiblings = false
-
-#### steps
-
-1. Open the kitchen sink example.
-1. Ensure `loadSiblings` is disabled.
-1. Ensure `displayActiveTiles` is enabled.
-1. Zoom in as much as possible so much of the tileset is not visible.
-1. Wait until all tiles have loaded.
-1. Disable the `enableUpdate` option.
-1. Zoom out to view the whole tileset.
-
 #### expected
 
 Only tiles that were visible when zoomed in are displayed.
@@ -39,7 +26,6 @@ Only tiles that were visible when zoomed in are displayed.
 #### steps
 
 1. Open the kitchen sink example.
-1. Ensure `loadSiblings` is enabled.
 1. Ensure `displayActiveTiles` is enabled.
 1. Zoom in as much as possible so much of the tileset is not visible.
 1. Wait until all tiles have loaded.
@@ -274,52 +260,6 @@ Verify no errors are logged.
 
 Verify that the stats display 125 geometries, 126 textures, and 1 programs.
 
-## Verify stopAtEmptyTiles works
-
-#### steps
-
-1. Open the kitchen sink example by navigating [here](https://nasa-ammos.github.io/3DTilesRendererJS/example/bundle/).
-1. Set `maxDepth` to 1.
-1. Verify that only 1 tile is visible.
-1. Open the kitchen sink example with the no root content tileset by navigating [here](https://nasa-ammos.github.io/3DTilesRendererJS/example/bundle/#../data/tileset-no-root-content.json).
-1. Set `maxDepth` to 1.
-1. Verify that all tiles disappear.
-
-#### expected
-
-No tiles are visible when the active leafs have no content.
-
-## Verify maxDepth limit does not stop at empty tiles when stopAtEmptyTiles = false
-
-1. Open the kitchen sink example by navigating [here](https://nasa-ammos.github.io/3DTilesRendererJS/example/bundle/).
-1. Set `maxDepth` to 1.
-1. Verify that only 1 tile is visible.
-1. Open the kitchen sink example with the no root content tileset by navigating [here](https://nasa-ammos.github.io/3DTilesRendererJS/example/bundle/#../data/tileset-no-root-content.json).
-1. Disable `stopAtEmptyTiles`.
-1. Set `maxDepth` to 1.
-1. Verify that 4 tiles are visible.
-
-#### expected
-
-The next shallowest tiles are visible past the `maxDepth` cutoff.
-
-## Verify tileset with missing mid tile content loads and renders correctly
-
-#### steps
-
-1. Open the kitchen sink example with the no root content tileset by navigating [here](https://nasa-ammos.github.io/3DTilesRendererJS/example/bundle/#../data/tileset-no-midtile-content.json).
-1. Zoom in slowly and verify the tiles load correctly and completely.
-1. Set `errorTarget` to 0.
-1. Set `maxDepth` to 3.
-1. Set `colorMode` to `RANDOM_COLOR`.
-1. Verify that one tile is missing.
-1. Disable `stopAtEmptyTiles`.
-1. Verify that there are four smaller tiles compared to the rest of the tileset where the missing midtile with content is.
-
-#### expected
-
-The tileset renders and loads correctly.
-
 ## Verify tileset with additive tiles renders correctly
 
 #### steps