ImageOverlayPlugin: Add failure handling (#1594)

* Add failure handling

* Clean up

* Update

* Updates

* Final fix

* Update signatures

Author: gkjohnson

src/three/plugins/API.md

@@ -1195,6 +1195,17 @@ deleteOverlay( overlay: ImageOverlay ): void
 Removes the given overlay from the plugin.
 
 
+### .resetFailedOverlays
+
+```js
+resetFailedOverlays(): void
+```
+
+Retries any overlay texture fetches that previously failed. Successfully loaded textures
+are applied to their tiles without requiring a geometry reload. Pairs with the `load-error`
+event, which fires on the `TilesRenderer` when an overlay texture fetch fails.
+
+
 ## LoadRegionPlugin
 
 Plugin that restricts tile loading and traversal to one or more geometric regions

src/three/plugins/images/ImageOverlayPlugin.d.ts

@@ -15,6 +15,7 @@ export class ImageOverlayPlugin {
 	addOverlay( overlay: ImageOverlay, order?: number ): void;
 	setOverlayOrder( overlay: ImageOverlay, order?: number ): void;
 	deleteOverlay( overlay: ImageOverlay ): void;
+	resetFailedOverlays(): void;
 
 }
 

src/three/plugins/images/ImageOverlayPlugin.js

@@ -1005,6 +1005,7 @@ export class ImageOverlayPlugin {
 			range: null,
 			target: null,
 			meshInfo: new Map(),
+			failed: false,
 		};
 
 		overlayInfo
@@ -1046,7 +1047,7 @@ export class ImageOverlayPlugin {
 
 		}
 
-		const { tiles, overlayInfo, tileControllers, processQueue } = this;
+		const { tiles, overlayInfo, tileControllers } = this;
 		const { ellipsoid } = tiles;
 		const { controller, tileInfo } = overlayInfo.get( overlay );
 		const tileController = tileControllers.get( tile );
@@ -1133,51 +1134,117 @@ export class ImageOverlayPlugin {
 
 		// if the image projection is outside the 0, 1 uvw range or there are no textures to draw in
 		// the tiled image set the don't allocate a texture for it.
-		let target = null;
 		if ( heightInRange && overlay.hasContent( range ) ) {
 
-			target = await processQueue
-				.add( { tile, overlay }, async () => {
+			await this._fetchTileOverlayTexture( tile, overlay, info );
 
-					// check if the overlay has been disposed since starting this function
-					if ( controller.signal.aborted || tileController.signal.aborted ) {
+		}
 
-						return null;
+		meshes.forEach( ( mesh, i ) => {
 
-					}
+			const array = new Float32Array( uvs[ i ] );
+			const attribute = new BufferAttribute( array, 3 );
+			info.meshInfo.set( mesh, { attribute } );
 
-					// Get the texture from the overlay
-					const regionTarget = await overlay.getTexture( range );
+		} );
 
-					// check if the overlay has been disposed since starting this function
-					if ( controller.signal.aborted || tileController.signal.aborted ) {
+	}
 
-						return null;
+	// Queues an overlay texture fetch for the given tile, writing the result into info.target.
+	// Never throws — failures mark info.failed and dispatch a load-error event instead.
+	async _fetchTileOverlayTexture( tile, overlay, info ) {
 
-					}
+		const { tiles, overlayInfo, tileControllers, processQueue } = this;
+		const { controller } = overlayInfo.get( overlay );
+		const tileController = tileControllers.get( tile );
+		const { range } = info;
 
-					return regionTarget;
+		info.target = await processQueue
+			.add( { tile, overlay }, async () => {
 
-				} )
-				.catch( err => {
+				// check if the overlay has been disposed since starting this function
+				if ( controller.signal.aborted || tileController.signal.aborted ) {
 
-					if ( ! ( err instanceof PriorityQueueItemRemovedError ) ) {
+					return null;
 
-						throw err;
+				}
 
-					}
+				// Get the texture from the overlay
+				const regionTarget = await overlay.getTexture( range );
 
-				} );
+				// check if the overlay has been disposed since starting this function
+				if ( controller.signal.aborted || tileController.signal.aborted ) {
 
-		}
+					return null;
 
-		info.target = target;
+				}
 
-		meshes.forEach( ( mesh, i ) => {
+				return regionTarget;
 
-			const array = new Float32Array( uvs[ i ] );
-			const attribute = new BufferAttribute( array, 3 );
-			info.meshInfo.set( mesh, { attribute } );
+			} )
+			.catch( err => {
+
+				if ( err instanceof PriorityQueueItemRemovedError ) {
+
+					return null;
+
+				}
+
+				info.failed = true;
+				tiles.dispatchEvent( { type: 'load-error', tile, overlay, error: err } );
+				return null;
+
+			} );
+
+	}
+
+	/**
+	 * Retries any overlay texture fetches that previously failed. Successfully loaded textures
+	 * are applied to their tiles without requiring a geometry reload. Pairs with the `load-error`
+	 * event, which fires on the `TilesRenderer` when an overlay texture fetch fails.
+	 */
+	resetFailedOverlays() {
+
+		const { processedTiles, overlayInfo, overlays } = this;
+		const failed = [];
+
+		// Release all failed entries synchronously so their DataCache disposal
+		// microtasks are queued before we re-lock below.
+		processedTiles.forEach( tile => {
+
+			overlays.forEach( overlay => {
+
+				const { tileInfo } = overlayInfo.get( overlay );
+				const info = tileInfo.get( tile );
+				if ( ! info.failed ) {
+
+					return;
+
+				}
+
+				info.failed = false;
+				overlay.releaseTexture( info.range );
+				failed.push( { tile, overlay, info } );
+
+			} );
+
+		} );
+
+		// Defer to the next frame so all disposal microtasks — including nested sub-cache
+		// cleanup — have fully drained before re-locking.
+		requestAnimationFrame( () => {
+
+			failed.forEach( ( { tile, overlay, info } ) => {
+
+				overlay.lockTexture( info.range );
+				this._fetchTileOverlayTexture( tile, overlay, info )
+					.then( () => {
+
+						this._updateLayers( tile );
+
+					} );
+
+			} );
 
 		} );
 

src/three/plugins/images/sources/GeoJSONImageSource.js

@@ -118,7 +118,11 @@ export class GeoJSONImageSource extends RegionImageSource {
 
 	disposeItem( texture ) {
 
-		texture.dispose();
+		if ( texture ) {
+
+			texture.dispose();
+
+		}
 
 	}
 

src/three/plugins/images/sources/MVTImageSource.js

@@ -206,21 +206,23 @@ export class MVTImageSource extends RegionImageSource {
 		tex.colorSpace = SRGBColorSpace;
 		tex.generateMipmaps = false;
 		tex.needsUpdate = true;
-		tex._regionArgs = [ minX, minY, maxX, maxY, level ];
 		return tex;
 
 	}
 
-	disposeItem( texture ) {
+	disposeItem( texture, [ minX, minY, maxX, maxY, level ] ) {
 
-		const [ minX, minY, maxX, maxY, level ] = texture._regionArgs;
 		forEachTileInBounds( [ minX, minY, maxX, maxY ], level, this._contentCache.tiling, ( tx, ty, tl ) => {
 
 			this._contentCache.release( tx, ty, tl );
 
 		} );
 
-		texture.dispose();
+		if ( texture ) {
+
+			texture.dispose();
+
+		}
 
 	}
 

src/three/plugins/images/sources/RegionImageSource.js

@@ -47,8 +47,6 @@ export class TiledRegionImageSource extends RegionImageSource {
 		this.tiledImageSource = tiledImageSource;
 		this.tileComposer = new TiledTextureComposer();
 		this.resolution = 256;
-		this.IS_DIRECT_TILE = Symbol( 'IS_DIRECT_TILE' );
-		this.LOCK_TOKENS = Symbol( 'LOCK_TOKENS' );
 
 	}
 
@@ -68,10 +66,9 @@ export class TiledRegionImageSource extends RegionImageSource {
 
 	async fetchItem( [ minX, minY, maxX, maxY, level ], signal ) {
 
-		const { tiledImageSource, tileComposer, IS_DIRECT_TILE, LOCK_TOKENS } = this;
+		const { tiledImageSource, tileComposer } = this;
 		const range = [ minX, minY, maxX, maxY ];
 		const tiling = tiledImageSource.tiling;
-		const tokens = [ ...range, level ];
 
 		// lock tiles for the requested level
 		await this._markImages( range, level, false );
@@ -95,14 +92,11 @@ export class TiledRegionImageSource extends RegionImageSource {
 		if ( singleTileBounds !== null ) {
 
 			// Clone rather than returning the texture directly so each region cache entry owns
-			// a distinct object. Returning the shared texture would cause symbol properties
-			// to be overwritten or deleted by concurrent cache entries during race conditions,
-			// (create, delete, create) leading to errors on disposal.
+			// a distinct object. Returning the shared texture would cause concurrent cache entries
+			// to alias the same object, leading to errors on disposal.
 			// Cloning shares the underlying Source so no extra GPU memory is used.
 			const [ tx, ty, tl ] = singleTileBounds;
 			const clone = tiledImageSource.get( tx, ty, tl ).clone();
-			clone[ IS_DIRECT_TILE ] = true;
-			clone[ LOCK_TOKENS ] = tokens;
 
 			return clone;
 
@@ -118,7 +112,6 @@ export class TiledRegionImageSource extends RegionImageSource {
 		const target = new CanvasTexture( canvas );
 		target.colorSpace = SRGBColorSpace;
 		target.generateMipmaps = false;
-		target[ LOCK_TOKENS ] = tokens;
 
 		// TODO: we could draw the parent tile data here if it's available just to make sure we
 		// have something to display but the texture is not usable until it returns. Though it
@@ -142,14 +135,13 @@ export class TiledRegionImageSource extends RegionImageSource {
 
 	}
 
-	disposeItem( target ) {
+	disposeItem( target, [ minX, minY, maxX, maxY, level ] ) {
 
-		const { IS_DIRECT_TILE, LOCK_TOKENS } = this;
-		const [ minX, minY, maxX, maxY, level ] = target[ LOCK_TOKENS ];
+		if ( target ) {
 
-		target.dispose();
-		delete target[ IS_DIRECT_TILE ];
-		delete target[ LOCK_TOKENS ];
+			target.dispose();
+
+		}
 
 		// Unlock the component tiles using the stored tokens
 		this._markImages( [ minX, minY, maxX, maxY ], level, true );

src/three/plugins/images/sources/TiledImageSource.js

@@ -69,6 +69,12 @@ export class TiledImageSource extends DataCache {
 	// dispose of the item that was fetched
 	disposeItem( texture ) {
 
+		if ( ! texture ) {
+
+			return;
+
+		}
+
 		texture.dispose();
 		if ( texture.image instanceof ImageBitmap ) {
 

src/three/plugins/images/utils/DataCache.js

@@ -18,8 +18,9 @@ export class DataCache {
 	}
 
 	// overridable
-	fetchItem() {}
-	disposeItem() {}
+	fetchItem( keys, signal ) {}
+	// called with null if the fetch failed
+	disposeItem( item, keys ) {}
 	getMemoryUsage( item ) {
 
 		return 0;
@@ -207,17 +208,27 @@ export class DataCache {
 					if ( result instanceof Promise ) {
 
 						// "disposeItem" will throw potentially if fetch, etc are cancelled using the abort signal
-						result.then( item => {
+						result
+							.then( item => {
 
-							this.disposeItem( item );
-							this.count --;
-							this.cachedBytes -= info.bytes;
+								this.disposeItem( item, info.args );
 
-						} ).catch( () => {} );
+							} )
+							.catch( () => {
+
+								this.disposeItem( null, info.args );
+
+							} )
+							.finally( () => {
+
+								this.count --;
+								this.cachedBytes -= info.bytes;
+
+							} );
 
 					} else {
 
-						this.disposeItem( result );
+						this.disposeItem( result, info.args );
 						this.count --;
 						this.cachedBytes -= info.bytes;