fix(shaders): request a frame when an Effect registers or unregisters

.changeset/effect-registration-frame.md

@@ -0,0 +1,5 @@
+---
+'@camp-dev/shaders': patch
+---
+
+A parked scene now redraws when an Effect mounts, unmounts, or re-registers on a prop change, such as Dither's `pattern` or Grain's `blend`. Before, the screen kept the old image until something else asked for a frame.

docs/agents/tsl.md

@@ -34,7 +34,7 @@ The rebuild on a new `build` reference is deliberate, and a test asserts it. Mem
 
 ### The bare-uniform-write gotcha: a bare uniform write repaints nothing
 
-The scene renders on demand. Once every component votes static, the frame loop parks after one flush, and a value written straight into `uniform.value` waits on the GPU until something else asks for a frame. After any raw write, including `Vector2.set` on a vec uniform, call `scheduler.requestRender()`. `useAnimatableUniform` already calls it after each write.
+The scene renders on demand. Once every component votes static, the frame loop parks after one flush, and a value written straight into `uniform.value` waits on the GPU until something else asks for a frame. After any raw write, including `Vector2.set` on a vec uniform, call `scheduler.requestRender()`. `useAnimatableUniform` already calls it after each write. An Effect's registration rebuilds the output material and has the same problem, so `useSceneRegistration` calls it each time an Effect registers or unregisters.
 
 For the aspect ratio, use `useAspectUniform`. It exists because twelve components wired `useResize` to an aspect uniform by hand without the call. Back then, `useResize` returned a stub on a component's first effect pass and filled in one render later, after the flush frame, so on a static scene such as the Components banner the 12:1 canvas rendered at the 16:9 fallback until a resize. `useResize` now hands over the scene's own size signal on the first render, and a visible scene redraws straight after each resize; a hidden or off-screen one draws at the new size when it resumes. A pixel-density change still updates the signal with no redraw of its own, so the call still matters.
 

packages/shaders/src/components/palette-map/shader.tsx

@@ -205,18 +205,5 @@ export function PaletteMapShader({ colors, pixelSize, spread }: PaletteMapShader
     [colorsKey, pixelSizeUniform, spreadUniform, dprUniform],
   );
 
-  // A new palette re-registers the resample above, and unmounting removes it.
-  // Either rebuilds the output material but draws nothing on a static scene,
-  // so ask for a frame. React runs this effect and its cleanup after
-  // useResample's, which was declared first, so each request follows the
-  // rebuild.
-  useEffect(() => {
-    shaderContext?.scheduler.requestRender();
-
-    return () => {
-      shaderContext?.scheduler.requestRender();
-    };
-  }, [colorsKey, shaderContext]);
-
   return null;
 }

packages/shaders/src/react/components/shader-scene/shader-scene.test.tsx

@@ -13,6 +13,7 @@ import {
   type Vector2,
 } from '../../../engine.js';
 import { ShadersError } from '../../errors/shaders-error.js';
+import { usePostProcessPass } from '../../hooks/use-overlay-pass/use-overlay-pass.js';
 import { useResize } from '../../hooks/use-resize/use-resize.js';
 import { useShaderContext } from '../../hooks/use-shader-context/use-shader-context.js';
 import { useStaticSceneHint } from '../../hooks/use-static-hint/use-static-hint.js';
@@ -119,6 +120,40 @@ describe('ShaderScene', () => {
     expect(() => unmount()).not.toThrow();
   });
 
+  // An Effect's cleanup asks for a frame, and on a parked scene that request
+  // would queue one. Once the scene has disposed its scheduler, it must not.
+  // The Effect votes last, so its vote still holds when that cleanup runs.
+  it('queues no frame when a parked scene unmounts with an Effect mounted', async () => {
+    const frames: FrameRequestCallback[] = [];
+
+    vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => {
+      frames.push(callback);
+
+      return frames.length;
+    });
+
+    function Effect() {
+      usePostProcessPass((color) => color, []);
+      useStaticSceneHint(true);
+
+      return null;
+    }
+
+    const { unmount } = render(
+      <ShaderScene>
+        <Effect />
+      </ShaderScene>,
+    );
+
+    await waitFor(() => expect(frames.length).toBeGreaterThan(0));
+    for (let now = 16; frames.length > 0; now += 16) {
+      for (const frame of frames.splice(0)) frame(now);
+    }
+
+    expect(() => unmount()).not.toThrow();
+    expect(frames).toEqual([]);
+  });
+
   it('fires onError with a renderer-init ShadersError when init fails', async () => {
     vi.spyOn(console, 'error').mockImplementation(() => {});
     const cause = new Error('no gpu backend');

packages/shaders/src/react/hooks/use-base-pass-uv/use-base-pass-uv.test.tsx

@@ -22,7 +22,7 @@ function makeCtx(): {
     renderer: {} as ShaderContextValue['renderer'],
     scene: {} as ShaderContextValue['scene'],
     camera: {} as ShaderContextValue['camera'],
-    scheduler: {} as ShaderContextValue['scheduler'],
+    scheduler: { requestRender: () => false } as ShaderContextValue['scheduler'],
     registerOverlay: () => () => undefined,
     registerResample: (resample: Resample) => {
       registered.push(resample);

packages/shaders/src/react/hooks/use-overlay-pass/use-overlay-pass.test.tsx

@@ -21,7 +21,7 @@ function makeCtx(): {
     renderer: {} as ShaderContextValue['renderer'],
     scene: {} as ShaderContextValue['scene'],
     camera: {} as ShaderContextValue['camera'],
-    scheduler: {} as ShaderContextValue['scheduler'],
+    scheduler: { requestRender: () => false } as ShaderContextValue['scheduler'],
     registerOverlay: (transform: PostProcessTransform) => {
       registered.push(transform);
 
@@ -72,7 +72,7 @@ describe('usePostProcessPass', () => {
       renderer: {} as ShaderContextValue['renderer'],
       scene: {} as ShaderContextValue['scene'],
       camera: {} as ShaderContextValue['camera'],
-      scheduler: {} as ShaderContextValue['scheduler'],
+      scheduler: { requestRender: () => false } as ShaderContextValue['scheduler'],
       registerOverlay: () => cleanupFn,
     } as unknown as ShaderContextValue;
 

packages/shaders/src/react/hooks/use-resample/use-resample.test.tsx

@@ -19,7 +19,7 @@ function makeContext(): {
     renderer: {} as ShaderContextValue['renderer'],
     scene: {} as ShaderContextValue['scene'],
     camera: {} as ShaderContextValue['camera'],
-    scheduler: {} as ShaderContextValue['scheduler'],
+    scheduler: { requestRender: () => false } as ShaderContextValue['scheduler'],
     registerOverlay: () => () => undefined,
     registerResample: (resample: Resample) => {
       registered.push(resample);

packages/shaders/src/react/hooks/use-scene-registration/use-scene-registration.test.tsx

@@ -6,11 +6,26 @@ import { afterEach, describe, expect, it } from 'vitest';
 import { ShaderContext, type ShaderContextValue, type Slot } from '../../context/shader-context.js';
 import { useSceneRegistration } from './use-scene-registration.js';
 
-afterEach(cleanup);
+// What happened, in order, so a test can check that a frame request follows
+// the registration change it has to draw.
+const events: string[] = [];
 
-// The hook hands the context to `register` and reads nothing from it, so an
-// empty object stands in for a mounted scene.
-const context = {} as ShaderContextValue;
+afterEach(() => {
+  cleanup();
+  events.length = 0;
+});
+
+// The hook hands the context to `register` and asks its scheduler for frames,
+// so a scheduler that logs each request stands in for a mounted scene.
+const context = {
+  scheduler: {
+    requestRender: () => {
+      events.push('request');
+
+      return true;
+    },
+  },
+} as ShaderContextValue;
 
 function Wrapper({ children }: { children: ReactNode }) {
   return <ShaderContext.Provider value={context}>{children}</ShaderContext.Provider>;
@@ -23,8 +38,11 @@ function makeProbe() {
     useSceneRegistration(
       (_shaderContext, slot) => {
         slots.push(slot);
+        events.push('register');
 
-        return () => undefined;
+        return () => {
+          events.push('unregister');
+        };
       },
       [mode],
     );
@@ -36,6 +54,38 @@ function makeProbe() {
 }
 
 describe('useSceneRegistration', () => {
+  // A registration rebuilds the output material and draws nothing, so a scene
+  // that has parked needs a frame to show it.
+  it('asks for a frame after registering', () => {
+    const { Probe } = makeProbe();
+
+    render(<Probe />, { wrapper: Wrapper });
+
+    expect(events).toEqual(['register', 'request']);
+  });
+
+  // Unregistering rebuilds the material too, and a parked scene would keep
+  // showing the Effect's last frame.
+  it('asks for a frame after unregistering', () => {
+    const { Probe } = makeProbe();
+    const { unmount } = render(<Probe />, { wrapper: Wrapper });
+
+    events.length = 0;
+    unmount();
+
+    expect(events).toEqual(['unregister', 'request']);
+  });
+
+  it('asks for a frame after each half of a swap when deps change', () => {
+    const { Probe } = makeProbe();
+    const { rerender } = render(<Probe mode="a" />, { wrapper: Wrapper });
+
+    events.length = 0;
+    rerender(<Probe mode="b" />);
+
+    expect(events).toEqual(['unregister', 'request', 'register', 'request']);
+  });
+
   // The scene puts a registration under a slot it has seen back in that
   // slot's place, so an Effect whose deps change keeps its place in the
   // stack.

packages/shaders/src/react/hooks/use-scene-registration/use-scene-registration.ts

@@ -2,8 +2,9 @@
 
 // How an Effect joins its <ShaderScene> and leaves it again. usePostProcessPass
 // and useResample each pick which list the Effect joins, and this hook owns
-// the rest: register on mount, unregister on unmount, and swap registrations
-// when a dependency changes. A change to that lifecycle lands here once.
+// the rest: register on mount, unregister on unmount, swap registrations when
+// a dependency changes, and ask for a frame after each so a parked scene shows
+// the change. A change to that lifecycle lands here once.
 import { type DependencyList, useEffect, useState } from 'react';
 
 import type { ShaderContextValue, Slot } from '../../context/shader-context.js';
@@ -13,8 +14,8 @@ import { useShaderContext } from '../use-shader-context/use-shader-context.js';
  * Run `register` against the enclosing scene, and call the unregister
  * function it returns on unmount and before each re-register. It registers
  * again whenever a value in `deps` changes, under the same slot, so the
- * Effect keeps its place in the stack. Outside a mounted <ShaderScene> it
- * does nothing.
+ * Effect keeps its place in the stack. Each change asks the scene for a frame.
+ * Outside a mounted <ShaderScene> it does nothing.
  */
 export function useSceneRegistration(
   register: (shaderContext: ShaderContextValue, slot: Slot) => () => void,
@@ -28,7 +29,20 @@ export function useSceneRegistration(
   useEffect(() => {
     if (!shaderContext) return;
 
-    return register(shaderContext, slot);
+    const { scheduler } = shaderContext;
+    const unregister = register(shaderContext, slot);
+
+    // Registering and unregistering each rebuild the output material, the
+    // shader that draws the finished image to the canvas, and draw nothing. A
+    // parked scene would keep showing the old image: the bare-uniform-write
+    // gotcha in `docs/agents/tsl.md`, set off by a rebuild. requestRender
+    // returns at once unless the scene is idle.
+    scheduler.requestRender();
+
+    return () => {
+      unregister();
+      scheduler.requestRender();
+    };
     // `register` is a new closure on every render, so listing it would
     // re-register every render. `deps` decides when the Effect registers again.
     // eslint-disable-next-line react-hooks/exhaustive-deps

packages/shaders/src/runtime/frame-scheduler/frame-scheduler.test.ts

@@ -529,6 +529,22 @@ describe('dispose invariants', () => {
     rafCallbacks.forEach((cb) => cb(performance.now()));
     expect(client).not.toHaveBeenCalled();
   });
+
+  // An Effect's cleanup asks for a frame, and when a whole scene unmounts it
+  // can run after the scene has disposed its scheduler.
+  it('queues no frame for a requestRender after dispose', () => {
+    const scheduler = new FrameScheduler();
+
+    scheduler.add(vi.fn());
+    scheduler.start();
+    scheduler.setIdle(true);
+    scheduler.dispose();
+    rafCallbacks = [];
+
+    scheduler.requestRender();
+
+    expect(rafCallbacks).toEqual([]);
+  });
 });
 
 describe('FrameScheduler phase reset', () => {