Keep the render loop marked running after a frame Retry could race

An error Scene catches itself marks Cesium's render loop stopped while the
loop's next animation frame is still queued. A Retry, or a tab shown again,
before that frame ran started a second loop, and the globe drew every frame
twice. The render and resize wrapper now keeps the loop marked running after
a call made while it ran; the queued frame stops it if nothing restarted it.
This also covers the inline panel's timer frames, so its own guard goes.

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.

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",

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);

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;
+  }
+});

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];
   };
 }

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();
+  }
+});