From d9e34f10c6932c29a5a5ccea2fc4e79fd4a06e45 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Tue, 10 Nov 2026 14:28:01 +0400 Subject: [PATCH] Finish an interrupted frame event as soon as its error surfaces Retry used to apply the queued listener changes of a Cesium event a throwing listener left mid-raise, adds first and removes second. A layer turned off and on while the notice was up landed in both queues and was removed. The render and resize wrapper now finishes those events before it raises the render error, so later adds and removes apply at once and in order. --- docs/CURRENT-STATE.md | 9 ++-- src/ui/renderFailure.js | 38 +++++++------- src/ui/renderFailureEventRetry.test.mjs | 70 +++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 22 deletions(-) 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(); + } +});