Dynamic overlay fixes (#1402)

* Remove lingering data

* Fix incorrect level reference

* Fix priority queue

* Add more specific tile deletion

* Handle errors more gracefully

Author: gkjohnson

src/core/renderer/index.js

@@ -8,6 +8,6 @@ export * from './loaders/CMPTLoaderBase.js';
 export * from './constants.js';
 
 export { LRUCache } from './utilities/LRUCache.js';
-export { PriorityQueue } from './utilities/PriorityQueue.js';
+export * from './utilities/PriorityQueue.js';
 export * as TraversalUtils from './utilities/TraversalUtils.js';
 export * as LoaderUtils from './utilities/LoaderUtils.js';

src/core/renderer/utilities/PriorityQueue.js

@@ -1,4 +1,15 @@
-class PriorityQueue {
+export class PriorityQueueItemRemovedError extends Error {
+
+	constructor() {
+
+		super( 'PriorityQueue: Item removed' );
+		this.name = 'PriorityQueueItemRemovedError';
+
+	}
+
+}
+
+export class PriorityQueue {
 
 	// returns whether tasks are queued or actively running
 	get running() {
@@ -98,8 +109,16 @@ class PriorityQueue {
 			// catch here to handle the case where the promise was never used anywhere
 			// else.
 			const info = callbacks.get( item );
-			info.promise.catch( () => {} );
-			info.reject( new Error( 'PriorityQueue: Item removed.' ) );
+			info.promise.catch( err => {
+
+				if ( ! ( err instanceof PriorityQueueItemRemovedError ) ) {
+
+					throw err;
+
+				}
+
+			} );
+			info.reject( new PriorityQueueItemRemovedError() );
 
 			items.splice( index, 1 );
 			callbacks.delete( item );
@@ -117,6 +136,7 @@ class PriorityQueue {
 			if ( filter( item ) ) {
 
 				this.remove( item );
+				i --;
 
 			}
 
@@ -196,5 +216,3 @@ class PriorityQueue {
 	}
 
 }
-
-export { PriorityQueue };

src/three/plugins/images/ImageOverlayPlugin.js

@@ -1,5 +1,5 @@
 import { WebGLRenderTarget, Color, SRGBColorSpace, BufferAttribute, Matrix4, Vector3, Box3, Triangle, CanvasTexture } from 'three';
-import { PriorityQueue } from '3d-tiles-renderer/core';
+import { PriorityQueue, PriorityQueueItemRemovedError } from '3d-tiles-renderer/core';
 import { CesiumIonAuth, GoogleCloudAuth } from '3d-tiles-renderer/core/plugins';
 import { TiledTextureComposer } from './overlays/TiledTextureComposer.js';
 import { XYZImageSource } from './sources/XYZImageSource.js';
@@ -168,6 +168,12 @@ export class ImageOverlayPlugin {
 	// plugin functions
 	init( tiles ) {
 
+		if ( ! this.renderer ) {
+
+			throw new Error( 'ImageOverlayPlugin: "renderer" instance must be provided.' );
+
+		}
+
 		const tileComposer = new TiledTextureComposer( this.renderer );
 		const processQueue = new PriorityQueue();
 		processQueue.maxJobs = 10;
@@ -219,7 +225,7 @@ export class ImageOverlayPlugin {
 				) {
 
 					const order = info.order;
-					this.deleteOverlay( overlay, false );
+					this.deleteOverlay( overlay );
 					this.addOverlay( overlay, order );
 
 					overlayChanged = true;
@@ -879,21 +885,37 @@ export class ImageOverlayPlugin {
 
 	}
 
-	deleteOverlay( overlay, forceDispose = true ) {
+	deleteOverlay( overlay ) {
 
 		const { overlays, overlayInfo, processQueue } = this;
 		const index = overlays.indexOf( overlay );
 		if ( index !== - 1 ) {
 
+			// delete tile info explicitly instead of blindly dispose of the full overlay
 			const { tileInfo, controller } = overlayInfo.get( overlay );
-			tileInfo.forEach( ( { meshInfo, target } ) => {
+			tileInfo.forEach( ( { meshInfo, range, meshRange, level, target, meshRangeMarked, rangeMarked }, tile ) => {
+
+				// release the ranges
+				if ( meshRange !== null && meshRangeMarked ) {
+
+					markOverlayImages( meshRange, level, overlay, true );
+
+				}
+
+				if ( range !== null && rangeMarked ) {
+
+					markOverlayImages( range, level, overlay, true );
+
+				}
 
 				if ( target !== null ) {
 
+					// release the render targets
 					target.dispose();
 
 				}
 
+				tileInfo.delete( tile );
 				meshInfo.clear();
 
 			} );
@@ -910,11 +932,6 @@ export class ImageOverlayPlugin {
 			} );
 
 			overlays.splice( index, 1 );
-			if ( forceDispose ) {
-
-				overlay.dispose();
-
-			}
 
 			this._markNeedsUpdate();
 
@@ -1051,7 +1068,6 @@ export class ImageOverlayPlugin {
 
 		}
 
-		const level = tile.__depthFromRenderedParent - 1;
 		const info = {
 			range: null,
 			meshRange: null,
@@ -1086,12 +1102,16 @@ export class ImageOverlayPlugin {
 					.add( { tile, overlay }, () => {
 
 						info.rangeMarked = true;
-						return markOverlayImages( range, level, overlay, false );
+						return markOverlayImages( range, info.level, overlay, false );
 
 					} )
-					.catch( () => {
+					.catch( err => {
+
+						if ( ! ( err instanceof PriorityQueueItemRemovedError ) ) {
 
-						// the queue throws an error if a task is removed early
+							throw err;
+
+						}
 
 					} );
 
@@ -1284,9 +1304,13 @@ export class ImageOverlayPlugin {
 					} );
 
 				} )
-				.catch( () => {
+				.catch( err => {
 
-					// the queue throws an error if a task is removed early
+					if ( ! ( err instanceof PriorityQueueItemRemovedError ) ) {
+
+						throw err;
+
+					}
 
 				} );
 

test/core/PriorityQueue.test.js

@@ -259,4 +259,21 @@ describe( 'PriorityQueue', () => {
 
 	} );
 
+	it( 'should be able to successfully remove all items by filter.', async () => {
+
+		const queue = new PriorityQueue();
+		queue.priorityCallback = () => 0;
+		queue.autoUpdate = false;
+		queue.maxJobs = 1;
+
+		queue.add( 1, () => {} );
+		queue.add( 2, () => {} );
+		queue.add( 3, () => {} );
+
+		expect( queue.items ).toHaveLength( 3 );
+		queue.removeByFilter( i => true );
+		expect( queue.items ).toHaveLength( 0 );
+
+	} );
+
 } );