diff --git a/src/scenes/director.js b/src/scenes/director.js index cb9c0b55c..98364dfff 100644 --- a/src/scenes/director.js +++ b/src/scenes/director.js @@ -234,7 +234,7 @@ export class SceneDirector { destroy() { if (this._destroyPromise) return this._destroyPromise; this._destroyed = true; - this._sceneSeekGeneration++; + this._bumpSceneSeekGeneration(); this._visibilityUnsubscribe?.(); this._cameraHandoffUnsubscribe?.(); this._removeCameraInput?.(); @@ -1071,7 +1071,7 @@ export class SceneDirector { loadShot(sceneId, shotId, options) { if (this._destroyed) return Promise.resolve({ started: false, reason: 'destroyed' }); - this._sceneSeekGeneration++; + this._bumpSceneSeekGeneration(); this._interactionTransitions = 0; const previousGeneration = this._loadGeneration; const work = this._loadShot(sceneId, shotId, options); @@ -1521,7 +1521,17 @@ export class SceneDirector { * @returns {Promise} Null when the shortcut does not apply and * LOAD must rebuild the shot; otherwise whether the seek settled. */ - async _seekLoadedShot(scene, seekState) { + _bumpSceneSeekGeneration() { + this._loadAbort?.abort(); + this._loadAbort = null; + return ++this._sceneSeekGeneration; + } + + async _seekLoadedShot( + scene, + seekState, + seekGeneration = this._sceneSeekGeneration, + ) { if (this._destroyed) return null; const shot = seekState?.shot; if ( @@ -1533,6 +1543,8 @@ export class SceneDirector { this._selectedShotId !== shot.id ) return null; + if (this._destroyed || seekGeneration !== this._sceneSeekGeneration) + return false; if (!this._claimCameraOwnership()) return false; this._cancelActiveSceneTravel(); this._loadAbort?.abort(); @@ -1547,29 +1559,57 @@ export class SceneDirector { // settling, leaves declared layers off the shot's stack. Reconcile only // those through the cancellable admission path. const staleStates = Object.fromEntries( - Object.entries(states).filter(([id, state]) => { - const wantEnabled = !!state?.enabled; - const lifecycle = this.dataManager.getLayerLifecycleState(id); - return ( - lifecycle && - (lifecycle.enabled !== wantEnabled || - lifecycle.lifecycleState === 'enabling' || - lifecycle.lifecycleState === 'disabling' || - lifecycle.uncertain || - this.dataManager.isEffectivelyEnabled(id) !== wantEnabled) - ); - }), + Object.entries(states) + .filter(([id, state]) => { + const wantEnabled = !!state?.enabled; + const lifecycle = this.dataManager.getLayerLifecycleState(id); + return ( + lifecycle && + (lifecycle.enabled !== wantEnabled || + lifecycle.lifecycleState === 'enabling' || + lifecycle.lifecycleState === 'disabling' || + lifecycle.uncertain || + this.dataManager.isEffectivelyEnabled(id) !== wantEnabled) + ); + }) + .map(([id, state]) => [id, { enabled: !!state?.enabled }]), ); if (Object.keys(staleStates).length) { + const previousEnabled = new Map( + Object.keys(staleStates).map((id) => [ + id, + Boolean(this.dataManager.getLayerLifecycleState(id)?.enabled), + ]), + ); const controller = new AbortController(); this._loadAbort = controller; const layerResult = await this._applyLayerStates( staleStates, - this._loadToken(generation, controller.signal), + this._loadToken(generation, controller.signal, seekGeneration), + { stopOnRefusal: true, exitContextMode: false }, ); if (this._loadAbort === controller) this._loadAbort = null; - if (layerResult.cancelled || layerResult.refused.length) return false; + if ( + layerResult.cancelled || + layerResult.refused.length || + this._destroyed || + seekGeneration !== this._sceneSeekGeneration + ) { + if (!this._destroyed && !this._loadAbort && !this._running) { + for (const id of [...layerResult.applied].reverse()) { + if (this._destroyed || this._loadAbort || this._running) break; + await this.dataManager.setEnabled( + id, + previousEnabled.get(id) ?? false, + { origin: 'scene' }, + ); + } + } + return false; + } } + if (this._destroyed || seekGeneration !== this._sceneSeekGeneration) + return false; let applied = true; for (const [layerId, state] of Object.entries(states)) { if (!state?.enabled || !state.params) continue; @@ -1582,6 +1622,8 @@ export class SceneDirector { } } if (!applied) return null; + if (this._destroyed || seekGeneration !== this._sceneSeekGeneration) + return false; this._setCameraView( seekState.camera || resolveCameraPose(scene, shot.camera), ); @@ -1603,7 +1645,7 @@ export class SceneDirector { seekScene(sceneId, progress) { if (this._destroyed) return Promise.resolve(false); this.stopScene(msg('Seeking scene clock')); - const generation = ++this._sceneSeekGeneration; + const generation = this._bumpSceneSeekGeneration(); return this._trackWork(this._seekScene(sceneId, progress, generation)); } @@ -1617,11 +1659,10 @@ export class SceneDirector { return false; const seekState = this._sceneSeekState(scene, progress); if (!seekState) return false; - const seeked = await this._seekLoadedShot(scene, seekState); - if (seeked !== null) - return ( - seeked && !this._destroyed && generation === this._sceneSeekGeneration - ); + const seeked = await this._seekLoadedShot(scene, seekState, generation); + if (this._destroyed || generation !== this._sceneSeekGeneration) + return false; + if (seeked !== null) return seeked; const result = await this._loadShot(scene.id, seekState.shot.id, { sceneSeek: seekState, }); @@ -1641,15 +1682,17 @@ export class SceneDirector { * @param {AbortSignal} [signal] - Abort signal for this LOAD's manager calls * @returns {{ cancelled: boolean, signal: AbortSignal|undefined }} */ - _loadToken(generation, signal = undefined) { + _loadToken(generation, signal = undefined, seekGeneration = null) { const director = this; return { signal, get cancelled() { return ( director._destroyed || - signal?.aborted || - director._loadGeneration !== generation + Boolean(signal?.aborted) || + director._loadGeneration !== generation || + (seekGeneration !== null && + director._sceneSeekGeneration !== seekGeneration) ); }, }; @@ -1788,7 +1831,7 @@ export class SceneDirector { startScene(sceneId, options) { if (this._destroyed) return Promise.resolve({ started: false, reason: 'destroyed' }); - this._sceneSeekGeneration++; + this._bumpSceneSeekGeneration(); return this._trackWork(this._startScene(sceneId, options)); } @@ -1954,9 +1997,7 @@ export class SceneDirector { this._setSceneMediaPlayback(); this._interactions?.clear(); this._dataPacks?.clear(); - this._sceneSeekGeneration++; - this._loadAbort?.abort(); - this._loadAbort = null; + this._bumpSceneSeekGeneration(); this._loadGeneration++; this._clock.stopShot(); this._cancelActiveSceneTravel(); @@ -2200,7 +2241,11 @@ export class SceneDirector { * the transition in flight. * @returns {Promise<{ applied: string[], refused: string[], cancelled: boolean }>} */ - async _applyLayerStates(targetStates, token = null) { + async _applyLayerStates( + targetStates, + token = null, + { stopOnRefusal = false, exitContextMode = true } = {}, + ) { const applied = []; const refused = []; const abort = () => ({ applied, refused, cancelled: true }); @@ -2210,8 +2255,10 @@ export class SceneDirector { // the operator's pre-mode state, which is exactly where a stopped scene // should come to rest. Tearing that transaction in half would strand // Context, so it completes and cancellation is honoured immediately after. - await this._exitIsolatingContextMode(); - if (token?.cancelled) return abort(); + if (exitContextMode) { + await this._exitIsolatingContextMode(); + if (token?.cancelled) return abort(); + } const signal = token?.signal; const registered = new Set( @@ -2241,6 +2288,7 @@ export class SceneDirector { console.warn( `[Scenes] Layer refused: ${id} → ${enabled ? 'on' : 'off'}`, ); + if (stopOnRefusal) break; continue; } if (params && typeof this.dataManager.restoreLayerState !== 'function') { diff --git a/src/scenes/director.test.mjs b/src/scenes/director.test.mjs index b874612ae..b2981a68b 100644 --- a/src/scenes/director.test.mjs +++ b/src/scenes/director.test.mjs @@ -302,6 +302,132 @@ test('same-shot seeking restores a layer whose superseded LOAD transition rolls } }); +test('same-shot seeking rolls back earlier layers and preserves Context, clock and camera when a later layer is refused', async () => { + let refuse = () => false; + const { director, dataManager, styleManager, viewer, restore } = makeDirector({ + data: { refuse: (id, on) => refuse(id, on) }, + }); + try { + const shot = director._project.scenes[0].shots[0]; + shot.layers = { + flights: { enabled: true }, + traffic: { enabled: true }, + satellites: { enabled: true }, + }; + await director.loadShot('scene-1', 'shot-a', { flyDuration: 0 }); + assert.equal(await director.seekScene('scene-1', 0.1), true); + await dataManager.setEnabled('flights', false); + await dataManager.setEnabled('traffic', false); + await dataManager.setEnabled('satellites', false); + styleManager.contextMode = 'space-missions'; + styleManager.contextExits.length = 0; + refuse = (id) => id === 'traffic'; + dataManager.setEnabledCalls.length = 0; + const viewsBefore = viewer.views.length; + const visualCalls = styleManager.visualCalls.length; + + assert.equal(await director.seekScene('scene-1', 0.25), false); + assert.equal( + dataManager.setEnabledCalls.some(({ id }) => id === 'satellites'), + false, + 'stop the plan before later ids are written', + ); + const enabled = new Map(dataManager.getAll().map(({ id, enabled }) => [id, enabled])); + assert.equal(enabled.get('flights'), false, 'first stale layer stays disabled'); + assert.equal(enabled.get('traffic'), false, 'refused second layer stays disabled'); + assert.equal(enabled.get('satellites'), false, 'later stale layer is never written'); + assert.equal(styleManager.contextMode, 'space-missions'); + assert.deepEqual(styleManager.contextExits, []); + assert.equal(viewer.views.length, viewsBefore, 'camera stays at the pre-seek pose'); + assert.equal(styleManager.visualCalls.length, visualCalls, 'do not rebuild the shot'); + assert.ok( + Math.abs(director.getPlaybackTimingState().snapshot.sceneProgress - 0.1) < 1e-9, + 'clock stays at the last settled seek position', + ); + } finally { + await director.destroy(); + restore(); + } +}); + +test('same-shot seeking aborts in-flight admission when superseded by an early-return load or start', async () => { + const { styleManager, viewer, restore } = makeDirector(); + let holdEnable = false; + let releaseEnable; + const enableGate = new Promise((resolve) => { + releaseEnable = resolve; + }); + const lifecycle = new LayerLifecycle(viewer); + lifecycle.register({ + id: 'flights', + name: 'flights', + init() {}, + async enable() { + if (holdEnable) await enableGate; + }, + disable() {}, + update() {}, + }); + const director = new SceneDirector(viewer, styleManager, lifecycle); + try { + await director.loadShot('scene-1', 'shot-a', { flyDuration: 0 }); + assert.equal(await director.seekScene('scene-1', 0.1), true); + await lifecycle.setEnabled('flights', false); + holdEnable = true; + const viewsBefore = viewer.views.length; + + const seek = director.seekScene('scene-1', 0.25); + for ( + let i = 0; + i < 20 && lifecycle.getLayerLifecycleState('flights').lifecycleState !== 'enabling'; + i++ + ) { + await Promise.resolve(); + } + assert.equal(lifecycle.getLayerLifecycleState('flights').lifecycleState, 'enabling'); + const admissionSignal = director._loadAbort?.signal; + assert.ok(admissionSignal, 'same-shot seek installs an admission controller'); + + const missingLoad = director.loadShot('missing', 'nope'); + assert.equal( + admissionSignal.aborted, + true, + 'missing load aborts the admission controller before returning', + ); + assert.equal((await missingLoad).reason, 'shot-not-found'); + assert.equal(viewer.views.length, viewsBefore, 'shortcut has not committed before abort'); + + releaseEnable(); + assert.equal(await seek, false); + await settle(); + assert.equal(lifecycle.isEnabled('flights'), false, 'layer is not restored'); + assert.equal(viewer.views.length, viewsBefore, 'setView is not called'); + assert.ok( + Math.abs(director.getPlaybackTimingState().snapshot.sceneProgress - 0.1) < 1e-9, + 'sceneProgress stays at the last settled value', + ); + + let loadFallbackCalls = 0; + const origLoadShot = director._loadShot.bind(director); + director._loadShot = (...args) => { + loadFallbackCalls += 1; + return origLoadShot(...args); + }; + const origSeekLoadedShot = director._seekLoadedShot.bind(director); + director._seekLoadedShot = async () => { + await director.startScene('missing'); + return null; + }; + assert.equal(await director.seekScene('scene-1', 0.25), false); + assert.equal(loadFallbackCalls, 0, 'stale seek returning null must not start _loadShot'); + director._seekLoadedShot = origSeekLoadedShot; + } finally { + releaseEnable?.(); + await director.destroy(); + restore(); + } +}); + test('same-shot seeking keeps the shortcut when declared visibility is unchanged', async () => { const { director, dataManager, styleManager, restore } = makeDirector(); try { @@ -867,15 +993,20 @@ function fakeStyleManager({ contextMode = null, exitFails = false } = {}) { /** Cesium viewer double whose flights complete on the next microtask turn. */ function fakeViewer() { const flights = []; + const views = []; let cancelled = 0; return { flights, + views, get cancelledFlights() { return cancelled; }, camera: { flyTo(options) { flights.push(options); Promise.resolve().then(() => options.complete?.()); }, + setView(options) { + views.push(options); + }, cancelFlight() { cancelled++; }, }, };