Repository: NASA-AMMOS/3DTilesRendererJS (js)

Issue:
Overfilling the cache can result in tile gaps due to eviction
Tile memory size isn't known until it's fully loaded so it can be the case that we evict visible tiles to meet the cache cap resulting in gaps in tiles upon eviction. Instead we should evict before the tile has a chance to be made visible. This may mean the most recently added tiles need to be evicted first and visibility should be part of the eviction weight. Or we should just remove tiles that haven't been displayed yet.

Related to #727

Pull request:
Update cache, dispose of loaded tiles if the cache is full
Fix #739 

- Remove unnecessary visibility setting on load
- Re-enable tests
- Dispose of tiles after loading if the cache is suddenly full
- Evict tiles if we are above the max cap

**TODO**
- Re-enable the frustum-test optimization which is causing an infinite loop of tile loads
  - Possibly need to check children's frustum before traversing for whether they are used


Patch:
diff --git a/src/base/TilesRendererBase.js b/src/base/TilesRendererBase.js
index 9f7c5cc..a1dd642 100644
--- a/src/base/TilesRendererBase.js
+++ b/src/base/TilesRendererBase.js
@@ -733,17 +733,16 @@ export class TilesRendererBase {
 
 				stats.parsing --;
 				tile.__loadingState = LOADED;
-				lruCache.updateMemoryUsage( tile );
 
-				if ( tile.__wasSetVisible ) {
+				// if the cache is full due to newly loaded memory then lets discard this tile - it will
+				// be loaded again later from the disk cache if needed.
+				if ( lruCache.isFull() ) {
 
-					this.setTileVisible( tile, true );
+					lruCache.remove( tile );
 
-				}
-
-				if ( tile.__wasSetActive ) {
+				} else {
 
-					this.setTileActive( tile, true );
+					lruCache.updateMemoryUsage( tile );
 
 				}
 
diff --git a/src/utilities/LRUCache.js b/src/utilities/LRUCache.js
index c33c9f5..bb14c1d 100644
--- a/src/utilities/LRUCache.js
+++ b/src/utilities/LRUCache.js
@@ -206,7 +206,7 @@ class LRUCache {
 		const excessNodes = Math.max( Math.min( itemList.length - minSize, unused ), 0 );
 		const excessBytes = this.cachedBytes - minBytesSize;
 		const unloadPriorityCallback = this.unloadPriorityCallback || this.defaultPriorityCallback;
-		let remaining = excessNodes;
+		let needsRerun = false;
 
 		const hasNodesToUnload = excessNodes > 0 && unused > 0 || itemList.length > maxSize;
 		const hasBytesToUnload = unused && this.cachedBytes > minBytesSize || this.cachedBytes > maxBytesSize;
@@ -252,18 +252,16 @@ class LRUCache {
 				// base while condition
 				const doContinue =
 					removedNodes < nodesToUnload
-					|| removedBytes < bytesToUnload;
-
-				// comment out since these can cause tiles to be removed resulting in tile gaps
-				//	|| this.cachedBytes - removedBytes - bytes > maxBytesSize
-				//	|| itemList.length - removedNodes > maxSize;
+					|| removedBytes < bytesToUnload
+					|| this.cachedBytes - removedBytes - bytes > maxBytesSize
+					|| itemList.length - removedNodes > maxSize;
 
 				// don't unload any used tiles unless we're above our size cap
 				if (
 					! doContinue
 					|| removedNodes >= unused
-					// && this.cachedBytes - removedBytes - bytes <= maxBytesSize
-					// && itemList.length - removedNodes <= maxSize
+					&& this.cachedBytes - removedBytes - bytes <= maxBytesSize
+					&& itemList.length - removedNodes <= maxSize
 				) {
 
 					break;
@@ -285,11 +283,12 @@ class LRUCache {
 
 			// if we didn't remove enough nodes or we still have excess bytes and there are nodes to removed
 			// then we want to fire another round of unloading
-			remaining = removedNodes < excessNodes || removedBytes < excessBytes && removedNodes < unused;
+			needsRerun = removedNodes < excessNodes || removedBytes < excessBytes && removedNodes < unused;
+			needsRerun = needsRerun && removedNodes > 0;
 
 		}
 
-		if ( remaining ) {
+		if ( needsRerun ) {
 
 			this.unloadingHandle = requestAnimationFrame( () => this.scheduleUnload() );
 


Test patch:
diff --git a/test/LRUCache.test.js b/test/LRUCache.test.js
index 9aa0bd3..4170584 100644
--- a/test/LRUCache.test.js
+++ b/test/LRUCache.test.js
@@ -95,7 +95,7 @@ describe( 'LRUCache', () => {
 
 	} );
 
-	it.skip( 'should evict items if they are the max item length even if they are used.', () => {
+	it( 'should evict items if they are the max item length even if they are used.', () => {
 
 		const cache = new LRUCache();
 		cache.unloadPriorityCallback = ( itemA, itemB ) => itemA.priority - itemB.priority;
@@ -143,7 +143,7 @@ describe( 'LRUCache', () => {
 
 	} );
 
-	it.skip( 'should update memory usage when the items are triggers.', () => {
+	it( 'should update memory usage when the items are triggers.', () => {
 
 		const cache = new LRUCache();
 		cache.minBytesSize = 10;