Repository navigation
fix(shaders): request a frame when an Effect registers or unregisters - #282
Merged
Merged
Conversation
Registering and unregistering rebuild the output material and draw nothing, so a parked scene kept the old image until something else asked for a frame. useSceneRegistration now asks after each change, which covers every Effect, and PaletteMap's local workaround goes.
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the shared Effect registration redraw change.
No open issues 🎉
What changed in this PR
Fixes stale images on parked shader scenes by moving Effect redraw requests into the shared registration hook, covering both resample and post-process Effects.
Changes:
- Requests a frame after Effect registration and unregistration, replacing PaletteMap’s local workaround.
- Adds lifecycle-order and disposal regression tests, and updates scheduler mocks.
- Documents the shared redraw behavior and adds a patch changeset.
| File | Description |
|---|---|
packages/shaders/src/runtime/frame-scheduler/frame-scheduler.test.ts |
Tests that render requests after disposal queue no frame. |
packages/shaders/src/react/hooks/use-scene-registration/use-scene-registration.ts |
Requests redraws after registration and cleanup. |
packages/shaders/src/react/hooks/use-scene-registration/use-scene-registration.test.tsx |
Tests redraw ordering on mount, unmount, and dependency changes. |
packages/shaders/src/react/hooks/use-resample/use-resample.test.tsx |
Adds requestRender to the scheduler mock. |
packages/shaders/src/react/hooks/use-overlay-pass/use-overlay-pass.test.tsx |
Updates scheduler mocks for shared redraw requests. |
packages/shaders/src/react/hooks/use-base-pass-uv/use-base-pass-uv.test.tsx |
Adds requestRender to the scheduler mock. |
packages/shaders/src/react/components/shader-scene/shader-scene.test.tsx |
Tests parked-scene teardown with an Effect mounted. |
packages/shaders/src/components/palette-map/shader.tsx |
Removes the redundant local redraw workaround. |
docs/agents/tsl.md |
Documents redraw requests during Effect registration changes. |
.changeset/effect-registration-frame.md |
Records the parked-scene fix as a patch release. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Registering or unregistering an Effect rebuilds the output material but draws nothing. On a parked scene, a changed Effect prop or a removed Effect left the old image on screen until something else asked for a frame.
What changes
useSceneRegistrationnow callsrequestRender()after each registration and each unregistration.useResampleandusePostProcessPassboth go through it, so this covers every Effect, and PaletteMap's local workaround is gone. A running scene pays nothing, becauserequestRender()returns at once unless the scene is idle.The new cleanup also runs when a whole scene unmounts, after the scheduler may be disposed. A parked-scene test and a
FrameSchedulertest confirm that this queues no frame.Closes SHA-311