From 144687c071bfc88b326a4d10e1606e9d9bc5117f Mon Sep 17 00:00:00 2001 From: kvvasuu Date: Fri, 31 Jul 2026 22:29:40 +0200 Subject: [PATCH] fix(EffectComposer): restore renderer.autoClear/toneMapping on unmount postprocessing forces these on mount and never restores them. Ref-counted per renderer so multiple composers sharing one don't stomp on each other, and skips restoring if something else changed the value since. --- src/EffectComposer.tsx | 47 +++++++- src/tests/EffectComposer.test.tsx | 191 ++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 3 deletions(-) diff --git a/src/EffectComposer.tsx b/src/EffectComposer.tsx index 7547b80f..7b0fef88 100644 --- a/src/EffectComposer.tsx +++ b/src/EffectComposer.tsx @@ -21,7 +21,7 @@ import { type ReactNode, type Ref, } from 'react' -import type { Camera, Group, Scene, TextureDataType } from 'three' +import type { Camera, Group, Scene, TextureDataType, WebGLRenderer } from 'three' import { HalfFloatType, NoToneMapping } from 'three' export const EffectComposerContext = /* @__PURE__ */ createContext<{ @@ -59,6 +59,44 @@ type ComposerState = { const isConvolution = (effect: Effect): boolean => (effect.getAttributes() & EffectAttribute.CONVOLUTION) === EffectAttribute.CONVOLUTION +/** + * autoClear/toneMapping get force-set and never restored by whoever sets + * them. Ref-counted per (renderer, property) since composers can share a + * renderer; skips restoring if the value already changed since acquire. + */ +function createRendererPropertyGuard(property: K) { + const refs = new WeakMap< + WebGLRenderer, + { count: number; original: WebGLRenderer[K]; forcedValue: WebGLRenderer[K] } + >() + + return { + acquire(gl: WebGLRenderer, forcedValue: WebGLRenderer[K]): void { + const existing = refs.get(gl) + if (existing) { + existing.count++ + existing.forcedValue = forcedValue + } else { + refs.set(gl, { count: 1, original: gl[property], forcedValue }) + } + }, + release(gl: WebGLRenderer): void { + const entry = refs.get(gl) + if (!entry) return + + if (--entry.count <= 0) { + if (gl[property] === entry.forcedValue) { + gl[property] = entry.original + } + refs.delete(gl) + } + }, + } +} + +const autoClearGuard = /* @__PURE__ */ createRendererPropertyGuard('autoClear') +const toneMappingGuard = /* @__PURE__ */ createRendererPropertyGuard('toneMapping') + /** * Groups a flat, ordered list of Effect/Pass instances into actual composer * passes, merging consecutive non-convolution Effects into a single @@ -116,6 +154,8 @@ export const EffectComposer = /* @__PURE__ */ memo(function EffectComposer({ const [composerState, setComposerState] = useState(null) useEffect(() => { + autoClearGuard.acquire(gl, false) + const effectComposer = new EffectComposerImpl(gl, { depthBuffer, stencilBuffer, multisampling, frameBufferType }) effectComposer.addPass(new RenderPass(scene, camera)) @@ -140,6 +180,7 @@ export const EffectComposer = /* @__PURE__ */ memo(function EffectComposer({ return () => { effectComposer.dispose() + autoClearGuard.release(gl) } // `size` intentionally excluded: it's applied via the composer.setSize // effect below, and shouldn't tear down/recreate the whole composer. @@ -199,10 +240,10 @@ export const EffectComposer = /* @__PURE__ */ memo(function EffectComposer({ // Disable tone mapping because threejs disallows tonemapping on render targets useEffect(() => { - const currentTonemapping = gl.toneMapping + toneMappingGuard.acquire(gl, NoToneMapping) gl.toneMapping = NoToneMapping return () => { - gl.toneMapping = currentTonemapping + toneMappingGuard.release(gl) } }, [gl]) diff --git a/src/tests/EffectComposer.test.tsx b/src/tests/EffectComposer.test.tsx index 60c28e8a..709c515d 100644 --- a/src/tests/EffectComposer.test.tsx +++ b/src/tests/EffectComposer.test.tsx @@ -485,6 +485,197 @@ describe('EffectComposer', () => { }) }) + describe('renderer state restoration', () => { + it('restores autoClear to its previous value once the composer is fully unmounted', async () => { + const gl = root.render(null).getState().gl + gl.autoClear = true // known baseline, independent of whatever earlier tests in this file left behind + + const ref = React.createRef() + + await React.act(async () => + root.render( + + + + ) + ) + + await waitForComposer(ref) + expect(gl.autoClear).toBe(false) + + await React.act(async () => root.render(null)) + + expect(gl.autoClear).toBe(true) + }) + + it('keeps autoClear disabled while a sibling composer on the same renderer is still mounted, regardless of unmount order', async () => { + const gl = root.render(null).getState().gl + gl.autoClear = true + + const refA = React.createRef() + const refB = React.createRef() + + await React.act(async () => + root.render( + <> + + + + + + + + ) + ) + + await waitForComposer(refA) + await waitForComposer(refB) + expect(gl.autoClear).toBe(false) + + // Unmount the first composer only (key "a" drops out of the tree) - + // the second is still relying on autoClear staying off, so this must + // not restore it yet. Keying both is essential here: without it, + // React would match by position and reuse "a"'s instance in place + // (just updating its props to "b"'s), unmounting the wrong one. + await React.act(async () => + root.render( + + + + ) + ) + + expect(gl.autoClear).toBe(false) + + // Now the last one goes too - only now should it actually restore. + await React.act(async () => root.render(null)) + + expect(gl.autoClear).toBe(true) + }) + + it('restores toneMapping to its previous value once the composer is fully unmounted', async () => { + const gl = root.render(null).getState().gl + gl.toneMapping = THREE.ACESFilmicToneMapping // known baseline, distinct from NoToneMapping + + const ref = React.createRef() + + await React.act(async () => + root.render( + + + + ) + ) + + await waitForComposer(ref) + expect(gl.toneMapping).toBe(THREE.NoToneMapping) + + await React.act(async () => root.render(null)) + + expect(gl.toneMapping).toBe(THREE.ACESFilmicToneMapping) + }) + + it('keeps toneMapping disabled while a sibling composer on the same renderer is still mounted, regardless of unmount order', async () => { + const gl = root.render(null).getState().gl + gl.toneMapping = THREE.ACESFilmicToneMapping + + const refA = React.createRef() + const refB = React.createRef() + + await React.act(async () => + root.render( + <> + + + + + + + + ) + ) + + await waitForComposer(refA) + await waitForComposer(refB) + expect(gl.toneMapping).toBe(THREE.NoToneMapping) + + // Unmount the first composer only - the second still relies on + // toneMapping staying off, so this must not restore it yet. + await React.act(async () => + root.render( + + + + ) + ) + + expect(gl.toneMapping).toBe(THREE.NoToneMapping) + + // Now the last one goes too - only now should it actually restore. + await React.act(async () => root.render(null)) + + expect(gl.toneMapping).toBe(THREE.ACESFilmicToneMapping) + }) + + it('does not clobber a manual autoClear change made while the composer was mounted', async () => { + const gl = root.render(null).getState().gl + // Pre-mount baseline is false (not the usual true) specifically so + // it differs from the manual override below - autoClear only has two + // states, so this is the only way to make an unconditional restore- + // to-original and a "preserve the manual change" outcome observably + // different from each other. + gl.autoClear = false + + const ref = React.createRef() + + await React.act(async () => + root.render( + + + + ) + ) + + await waitForComposer(ref) + expect(gl.autoClear).toBe(false) + + // Something outside this component takes manual control while the + // composer happens to still be mounted - a deliberate, more current + // intent than whatever we captured before mount. + gl.autoClear = true + + await React.act(async () => root.render(null)) + + // Must stay what the manual override set it to, not get silently + // reset back to the pre-mount value we originally captured (false). + expect(gl.autoClear).toBe(true) + }) + + it('does not clobber a manual toneMapping change made while the composer was mounted', async () => { + const gl = root.render(null).getState().gl + gl.toneMapping = THREE.ACESFilmicToneMapping + + const ref = React.createRef() + + await React.act(async () => + root.render( + + + + ) + ) + + await waitForComposer(ref) + expect(gl.toneMapping).toBe(THREE.NoToneMapping) + + gl.toneMapping = THREE.ReinhardToneMapping + + await React.act(async () => root.render(null)) + + expect(gl.toneMapping).toBe(THREE.ReinhardToneMapping) + }) + }) + describe('StrictMode', () => { it('preserves effects after the StrictMode fake-unmount/remount cycle', async () => { const ref = React.createRef()