diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 221a56271..e8ec7a60f 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2106,10 +2106,15 @@ An inline conversation panel never reloads or navigates its host's page, so it offers Retry after a render error only, and nothing after a lost context, where the panel runtime's own notice applies. The panel's timer (`keepPanelRendering()`) draws only while Cesium's render loop is on, so it -stops after a render error and draws again after Retry. An error in a frame -the timer draws leaves the loop's next animation frame queued, and the timer -keeps Cesium's loop marked running, so Retry does not start a second loop that -would draw every frame twice once the host resumes frames. +stops after a render error and draws again after Retry. + +A render error marks Cesium's loop stopped, but a frame drawn while the loop +ran leaves its next animation frame queued: an error Scene catches itself +returns to the loop, which queues the frame, and a frame the panel's timer +draws leaves it queued. The `render()` and `resize()` wrapper keeps the loop +marked running after such a call, so a Retry before that frame, or a tab shown +again (`src/app/tools.js`), does not start a second loop that would draw every +frame twice. The queued frame stops the loop itself if nothing restarted it. The `webglcontextlost` listener does not `preventDefault()` the context loss or restore the context; it only reports it. diff --git a/scripts/format-scope.json b/scripts/format-scope.json index 309159f5d..d81419796 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -560,6 +560,8 @@ "src/ui/renderFailureLoopErrors.test.mjs", "src/ui/renderFailureEventRetry.test.mjs", "src/app/renderFailureInlineRetry.test.mjs", + "src/ui/renderFailureSecondLoop.test.mjs", + "src/app/renderFailureInlineSecondLoop.test.mjs", "src/layers/cctv/lifecycle.js", "src/layers/cctv/controls.js", "src/layers/cctv/geometry.js", diff --git a/src/app/embed.js b/src/app/embed.js index b7ceb465c..dba6e9f16 100644 --- a/src/app/embed.js +++ b/src/app/embed.js @@ -103,20 +103,12 @@ export function keepPanelRendering( // Cesium stops its render loop after a render error, and the timer stops // with it; Retry restarts both. Report the error in full: workers report // plain objects, which Cesium prints as [object Object]. - let drawing = false; const removeErrorListener = viewer.scene?.renderError?.addEventListener( - (_scene, error) => { - // Cesium marks its loop stopped, but in a frame the timer draws, the - // loop's next animation frame is still queued and resumes when frames - // do. Restarting the loop now would start a second one beside it, and - // the globe would draw twice a frame. - if (drawing && viewer.cesiumWidget) - viewer.cesiumWidget._renderLoopRunning = true; + (_scene, error) => console.error( "[God's Eye View panel] render error:", describeError(error), - ); - }, + ), ); const timer = windowRef.setInterval(() => { if ( @@ -125,13 +117,8 @@ export function keepPanelRendering( viewer.isDestroyed?.() ) return; - drawing = true; - try { - viewer.resize(); - viewer.render(); - } finally { - drawing = false; - } + viewer.resize(); + viewer.render(); }, PANEL_FRAME_MS); return () => { windowRef.clearInterval(timer); diff --git a/src/app/renderFailureInlineSecondLoop.test.mjs b/src/app/renderFailureInlineSecondLoop.test.mjs new file mode 100644 index 000000000..6c450960b --- /dev/null +++ b/src/app/renderFailureInlineSecondLoop.test.mjs @@ -0,0 +1,109 @@ +// Retry in an inline panel after an error Scene catches itself in an +// animation frame, when the host stops frames right after it. The loop's next +// animation frame is still queued, so Retry must not start a second loop. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import * as Cesium from 'cesium'; +import { keepPanelRendering } from './embed.js'; +import { installRenderFailureNotice } from '../ui/renderFailure.js'; + +test('Retry in a panel whose host stopped frames right after an error Scene caught in an animation frame keeps one render loop', () => { + const frames = []; + let tick = null; + let clock = 0; + const windowRef = { + requestAnimationFrame: () => 1, + cancelAnimationFrame() {}, + setInterval: (fn) => ((tick = fn), 1), + clearInterval() {}, + }; + const previousRaf = globalThis.requestAnimationFrame; + globalThis.requestAnimationFrame = (fn) => frames.push(fn); + const previousError = console.error; + console.error = () => {}; + try { + let broken = false; + let drawn = 0; + const scene = { + renderError: new Cesium.Event(), + preUpdate: new Cesium.Event(), + postRender: new Cesium.Event(), + requestRender() {}, + initializeFrame() {}, + render() { + drawn += 1; + this.preUpdate.raiseEvent(this); + if (broken) this.renderError.raiseEvent(this, new Error('boom')); + this.postRender.raiseEvent(this); + }, + }; + const widget = Object.create(Cesium.CesiumWidget.prototype); + Object.assign(widget, { + _scene: scene, + _clock: { tick() {} }, + _canRender: true, + _useDefaultRenderLoop: false, + _renderLoopRunning: false, + _showRenderLoopErrors: false, + _targetFrameRate: undefined, + isDestroyed: () => false, + resize() {}, + }); + // CesiumWidget's own renderError handler, which marks the loop stopped. + scene.renderError.addEventListener(() => { + widget._useDefaultRenderLoop = false; + widget._renderLoopRunning = false; + }); + const viewer = { + scene, + cesiumWidget: widget, + canvas: { addEventListener() {}, removeEventListener() {} }, + get useDefaultRenderLoop() { + return widget.useDefaultRenderLoop; + }, + set useDefaultRenderLoop(value) { + widget.useDefaultRenderLoop = value; + }, + isDestroyed: () => false, + render: () => widget.render(), + resize: () => widget.resize(), + }; + const store = { + renderFailure: null, + publishRenderFailure(next) { + this.renderFailure = next; + }, + }; + const uninstall = installRenderFailureNotice(viewer, { + store, + allowReload: false, + locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, + logError() {}, + }); + const stop = keepPanelRendering(viewer, { windowRef, now: () => clock }); + viewer.useDefaultRenderLoop = true; + // The error comes in an animation frame; Scene catches it, and the loop + // still queues its next frame. Then the host stops animation frames. + broken = true; + for (const render of frames.splice(0)) render(16); + assert.equal(store.renderFailure?.reason, 'render-error'); + assert.equal(frames.length, 1); + broken = false; + clock = 400; + store.renderFailure.retry(); + clock += 33; + tick(); + assert.equal(store.renderFailure, null); + assert.equal(frames.length, 1, 'Retry starts no second loop'); + // The host resumes frames: the globe draws once a frame. + drawn = 0; + for (let frame = 1; frame <= 3; frame += 1) + for (const render of frames.splice(0)) render(frame * 16); + assert.equal(drawn, 3); + stop(); + uninstall(); + } finally { + globalThis.requestAnimationFrame = previousRaf; + console.error = previousError; + } +}); diff --git a/src/ui/renderFailure.js b/src/ui/renderFailure.js index 192dfc2cc..6a4359bd9 100644 --- a/src/ui/renderFailure.js +++ b/src/ui/renderFailure.js @@ -162,25 +162,37 @@ function finishRaise(event) { } /** - * Wrap `target[name]` so an error it throws is reported and thrown on, for - * the caller to handle as before. Returns a function that unwraps it. + * Wrap the widget's `name` method so an error it throws is reported and + * thrown on, for the caller to handle as before. Returns a function that + * unwraps it. + * + * A render error marks Cesium's render loop stopped, but a call made while + * the loop ran leaves the loop's next animation frame queued: the loop queues + * it after a call that returns, and a frame an inline panel's timer draws + * leaves it queued. That frame stops the loop itself, and a Retry or a shown + * tab that comes first must not start a second loop beside it, so the loop + * stays marked running. A call from the loop that throws is still marked + * stopped by the loop's own catch. */ -function reportThrown(target, name, report) { - const original = target?.[name]; +function reportThrown(widget, name, report) { + const original = widget?.[name]; if (typeof original !== 'function') return () => {}; - const own = Object.hasOwn(target, name); + const own = Object.hasOwn(widget, name); const guarded = function (...args) { + const looping = widget._renderLoopRunning; try { return original.apply(this, args); } catch (error) { report(error); throw error; + } finally { + if (looping) widget._renderLoopRunning = true; } }; - target[name] = guarded; + widget[name] = guarded; return () => { - if (target[name] !== guarded) return; - if (own) target[name] = original; - else delete target[name]; + if (widget[name] !== guarded) return; + if (own) widget[name] = original; + else delete widget[name]; }; } diff --git a/src/ui/renderFailureSecondLoop.test.mjs b/src/ui/renderFailureSecondLoop.test.mjs new file mode 100644 index 000000000..3ced898f4 --- /dev/null +++ b/src/ui/renderFailureSecondLoop.test.mjs @@ -0,0 +1,151 @@ +// Retry after an error Scene catches itself (Scene.tryAndCatchError raises +// renderError and returns). CesiumWidget's renderError handler marks the loop +// stopped, but startRenderLoop still queues the loop's next animation frame. +// A Retry that lands before that frame runs must not start a second loop. +// Drives Cesium's real startRenderLoop and useDefaultRenderLoop setter +// (CesiumWidget.prototype). +import test from 'node:test'; +import assert from 'node:assert/strict'; +import * as Cesium from 'cesium'; +import { installRenderFailureNotice } from './renderFailure.js'; + +function setup({ targetFrameRate } = {}) { + const frames = []; + const previousRaf = globalThis.requestAnimationFrame; + globalThis.requestAnimationFrame = (fn) => frames.push(fn); + const scene = { + renderError: new Cesium.Event(), + preUpdate: new Cesium.Event(), + preRender: new Cesium.Event(), + postRender: new Cesium.Event(), + broken: false, + drawn: 0, + requestRender() {}, + initializeFrame() {}, + // Scene.render (Scene.js:4369): an update error is caught by + // tryAndCatchError, which raises renderError and returns normally. + render(time) { + this.drawn += 1; + this.preUpdate.raiseEvent(this, time); + if (this.broken) this.renderError.raiseEvent(this, new Error('boom')); + this.preRender.raiseEvent(this, time); + this.postRender.raiseEvent(this, time); + }, + }; + const widget = Object.create(Cesium.CesiumWidget.prototype); + Object.assign(widget, { + _scene: scene, + _clock: { tick() {} }, + _canRender: true, + _useDefaultRenderLoop: false, + _renderLoopRunning: false, + _showRenderLoopErrors: false, + _targetFrameRate: targetFrameRate, + isDestroyed: () => false, + resize() {}, + }); + // CesiumWidget's own renderError handler (CesiumWidget.js:414). + scene.renderError.addEventListener(() => { + widget._useDefaultRenderLoop = false; + widget._renderLoopRunning = false; + }); + const viewer = { + scene, + cesiumWidget: widget, + canvas: { addEventListener() {}, removeEventListener() {} }, + get useDefaultRenderLoop() { + return widget.useDefaultRenderLoop; + }, + set useDefaultRenderLoop(value) { + widget.useDefaultRenderLoop = value; + }, + }; + const store = { + renderFailure: null, + publishRenderFailure(next) { + this.renderFailure = next; + }, + }; + const uninstall = installRenderFailureNotice(viewer, { + store, + locationRef: { href: 'https://gev.example/', reload() {}, assign() {} }, + logError() {}, + }); + let time = 0; + const runFrames = (n) => { + for (let i = 0; i < n; i += 1) { + time += 17; + for (const fn of frames.splice(0)) fn(time); + } + }; + const restore = () => { + uninstall(); + globalThis.requestAnimationFrame = previousRaf; + }; + return { frames, scene, viewer, store, runFrames, restore }; +} + +for (const targetFrameRate of [undefined, 60]) { + test(`targetFrameRate ${targetFrameRate}: Retry before the next frame after an error Scene caught keeps one render loop`, () => { + const { frames, scene, viewer, store, runFrames, restore } = setup({ + targetFrameRate, + }); + try { + viewer.useDefaultRenderLoop = true; + runFrames(2); + scene.broken = true; + runFrames(1); + assert.equal(store.renderFailure?.reason, 'render-error'); + // The cause is gone; Retry runs (a queued Enter on the focused Retry) + // before the loop's next animation frame. + scene.broken = false; + store.renderFailure.retry(); + assert.equal(frames.length, 1, 'one animation frame queued after Retry'); + scene.drawn = 0; + runFrames(3); + assert.equal(store.renderFailure, null); + assert.equal(scene.drawn, 3, 'the globe draws once a frame'); + } finally { + restore(); + } + }); +} + +test('control: Retry after the next frame ran keeps one render loop', () => { + const { frames, scene, viewer, store, runFrames, restore } = setup(); + try { + viewer.useDefaultRenderLoop = true; + runFrames(2); + scene.broken = true; + runFrames(1); + scene.broken = false; + runFrames(1); + assert.equal(frames.length, 0, 'the stopped loop queued nothing'); + store.renderFailure.retry(); + scene.drawn = 0; + runFrames(3); + assert.equal(scene.drawn, 3); + } finally { + restore(); + } +}); + +test('a tab hidden and shown again before the next frame after an error Scene caught keeps one render loop', () => { + const { scene, viewer, runFrames, restore } = setup(); + try { + viewer.useDefaultRenderLoop = true; + runFrames(2); + scene.broken = true; + runFrames(1); + scene.broken = false; + // src/app/tools.js turns the loop off while the tab is hidden and back on + // when it is shown, before the queued frame runs. + viewer.useDefaultRenderLoop = false; + viewer.useDefaultRenderLoop = true; + scene.drawn = 0; + runFrames(3); + assert.equal(scene.drawn, 3, 'the globe draws once a frame'); + } finally { + restore(); + } +});