diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 583e6d6a6..c5dd54a73 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1600,10 +1600,11 @@ render error before Cesium's loop handles it as before. until a frame starts and ends without a render error (Cesium raises `postRender` after a failed frame too), and a loop that fails again keeps it. A listener that throws leaves its Cesium event mid-raise, with listeners - added or removed since only queued, so Retry first applies the queue of the - frame and clock events (`finishRaise()`): the notice's frame watchers start - with the next frame, and a listener removed since, for a layer turned off, - is not called again. + added or removed since only queued, so the `render()` and `resize()` wrapper + finishes the frame and clock events it cut short (`finishRaise()`) before it + raises the render error: the notice's frame watchers start with the next + frame, a listener removed while the loop is stopped, for a layer turned off, + is not called again, and one turned off and on again keeps running. - **Reload** after a lost context reloads the page: Cesium cannot draw into a context it has lost, and the address keeps the view. - **Reset view** opens the app's root address (`appRootUrl()`), without the diff --git a/src/ui/renderFailure.js b/src/ui/renderFailure.js index b5d5b2b55..192dfc2cc 100644 --- a/src/ui/renderFailure.js +++ b/src/ui/renderFailure.js @@ -42,17 +42,6 @@ export function installRenderFailureNotice( locationRef.reload(); return; } - [ - scene.preUpdate, - scene.postUpdate, - scene.preRender, - scene.postRender, - scene.camera?.changed, - scene.camera?.moveStart, - scene.camera?.moveEnd, - viewer.clock?.onTick, - viewer.clock?.onStop, - ].forEach(finishRaise); viewer.useDefaultRenderLoop = true; scene.requestRender?.(); } @@ -120,11 +109,25 @@ export function installRenderFailureNotice( // postRender listener, the clock's onTick, a resize) also stops Cesium's // render loop, but raises no renderError, and Cesium reports it only in the // panel the app turns off. Raise it as a render error, so Cesium's handler, - // an inline panel's timer and this notice all see it. + // an inline panel's timer and this notice all see it. Such an error passes + // here before anything else runs, so finish the raises it cut short first: + // listeners added or removed while the loop is stopped then apply at once, + // in order. const unguard = ['resize', 'render'].map((name) => - reportThrown(viewer.cesiumWidget, name, (error) => - scene.renderError.raiseEvent(scene, error), - ), + reportThrown(viewer.cesiumWidget, name, (error) => { + [ + scene.preUpdate, + scene.postUpdate, + scene.preRender, + scene.postRender, + scene.camera?.changed, + scene.camera?.moveStart, + scene.camera?.moveEnd, + viewer.clock?.onTick, + viewer.clock?.onStop, + ].forEach(finishRaise); + scene.renderError.raiseEvent(scene, error); + }), ); canvas?.addEventListener?.('webglcontextlost', onContextLost); return () => { @@ -138,9 +141,8 @@ export function installRenderFailureNotice( /** * Finish the raise a throwing listener cut short. Cesium's `Event.raiseEvent` * marks the event as raising until every listener returns, and until then it - * only queues the listeners added or removed: the frame watchers the notice - * adds, or the throwing listener of a layer turned off since. Apply the queue - * as the raise would have. + * only queues the listeners added or removed. Apply the queue as the raise + * would have, so listeners added or removed from here on take effect at once. */ function finishRaise(event) { if (!event?._invokingListeners) return; diff --git a/src/ui/renderFailureEventRetry.test.mjs b/src/ui/renderFailureEventRetry.test.mjs index 19614ff53..1ed20b684 100644 --- a/src/ui/renderFailureEventRetry.test.mjs +++ b/src/ui/renderFailureEventRetry.test.mjs @@ -132,3 +132,73 @@ test('a throwing postRender listener removed before Retry is not called again', restore(); } }); + +const listenerCount = (event) => + [...event._listeners.values()].reduce((sum, scopes) => sum + scopes.size, 0); + +test('a listener turned off and on while the loop is stopped runs after Retry', () => { + const { scene, viewer, store, runFrames, restore } = setup(); + try { + let broken = true; + scene.postRender.addEventListener(() => { + if (broken) throw new TypeError('layer listener failed'); + }); + let calls = 0; + const frame = () => { + calls += 1; + }; + let removeFrame = scene.postRender.addEventListener(frame); + scene.requestRender(); + viewer.useDefaultRenderLoop = true; + runFrames(1); + assert.equal(store.renderFailure?.reason, 'render-error'); + removeFrame(); // another layer is turned off + removeFrame = scene.postRender.addEventListener(frame); // and on again + broken = false; + store.renderFailure.retry(); + calls = 0; + scene.requestRender(); + runFrames(3); + assert.ok(calls > 0, 'the re-added listener runs after Retry'); + assert.ok(scene.postRender._listeners.has(frame)); + assert.equal( + scene.postRender.numberOfListeners, + listenerCount(scene.postRender), + ); + } finally { + restore(); + } +}); + +test('a listener added, removed and added again while the loop is stopped runs after Retry', () => { + const { scene, viewer, store, runFrames, restore } = setup(); + try { + let broken = true; + scene.postRender.addEventListener(() => { + if (broken) throw new TypeError('layer listener failed'); + }); + scene.requestRender(); + viewer.useDefaultRenderLoop = true; + runFrames(1); + assert.equal(store.renderFailure?.reason, 'render-error'); + let calls = 0; + const onGroundFrame = () => { + calls += 1; + }; + // worldAnnotationRenderer: a ground task starts, times out, the next starts + scene.postRender.addEventListener(onGroundFrame); + scene.postRender.removeEventListener(onGroundFrame); + scene.postRender.addEventListener(onGroundFrame); + broken = false; + store.renderFailure.retry(); + scene.requestRender(); + runFrames(3); + assert.ok(calls > 0, 'the re-added listener runs after Retry'); + assert.equal( + scene.postRender.numberOfListeners, + listenerCount(scene.postRender), + ); + } finally { + restore(); + } +});