diff --git a/src/createEffectComponent.tsx b/src/createEffectComponent.tsx new file mode 100644 index 0000000..370c83c --- /dev/null +++ b/src/createEffectComponent.tsx @@ -0,0 +1,92 @@ +import { extend, useThree } from '@react-three/fiber' +import type { BlendFunction, Effect, Pass } from 'postprocessing' +import type { ExoticComponent, JSX, Ref } from 'react' +import { useCallback, useRef } from 'react' +import { useLiveDefaults } from './util' + +export type EffectConstructor = new (...args: any[]) => Effect | Pass + +// The effect's own options type, straight off its constructor - postprocessing +// already types every effect's sole options object precisely (either inline or, +// like BloomEffect, as a named exported type); this just strips the `| undefined` +// that comes from the parameter being optional. +export type EffectOptions = NonNullable[0]> + +const components = new WeakMap | string>() +let i = 0 + +const BLEND_KEYS = ['blendMode-blendFunction', 'blendMode-opacity-value'] + +/** + * Registers `effect` as a JSX intrinsic once per class and returns a + * component that renders it. Everything else - construction from `args`, + * live prop application (with the same Color/Vector coercion and reset- + * to-default on removal any r3f element gets), disposal - is r3f's own + * reconciler, same rules as ``/``. Only fits + * effects whose constructor works with zero arguments (`new Effect()`) - + * r3f's own reset-on-removal falls back to `0` otherwise, which is wrong + * for anything non-numeric. Effects that require e.g. scene/camera stay + * hand-rolled (see Outline.tsx, SelectiveBloom.tsx, ShockWave.tsx). + * + * `blendFunction`/`opacity` are pierced through to `blendMode-*` - every + * `Effect` has them on a nested `blendMode`, not on the effect itself, so a + * plain top-level prop would silently land on a stray, unread property. + * Applied via useLiveDefaults, not as plain JSX props: BlendMode's own + * constructor requires `blendFunction` (no default), so its constructor + * length isn't 0 either, and r3f's native reset-on-removal falls back to + * `changedProps[prop] = 0` - which is BlendFunction.SKIP, not a merely + * "wrong" blend function but one that hides the effect entirely. + */ +export function createEffectComponent( + effect: T +): ( + props: P & { + blendFunction?: BlendFunction + opacity?: number + args?: ConstructorParameters + ref?: Ref> + } +) => JSX.Element { + return function EffectComponent({ blendFunction, opacity, ref, ...props }: any) { + let Component = components.get(effect) + + if (!Component) { + const key = `@react-three/postprocessing/${effect.name}-${i++}` + extend({ [key]: effect }) + components.set(effect, (Component = key)) + } + + const camera = useThree((state) => state.camera) + const localRef = useRef>(null) + + // Forwards ref's own return value: r3f's setFiberRef (React 19-style ref + // cleanup) calls the ref function again only if it *didn't* return one, + // otherwise it stores and calls that instead - never re-invoking this + // function with null. So localRef must be cleared from inside that same + // returned cleanup, not left for a null call that will never come. + const setRef = useCallback( + (instance: InstanceType | null) => { + localRef.current = instance + if (typeof ref !== 'function') { + if (ref) ref.current = instance + return + } + const cleanup = ref(instance) + if (typeof cleanup !== 'function') return + return () => { + localRef.current = null + cleanup() + } + }, + [ref] + ) + + useLiveDefaults( + localRef, + { 'blendMode-blendFunction': blendFunction, 'blendMode-opacity-value': opacity }, + BLEND_KEYS + ) + + return + } +} diff --git a/src/index.ts b/src/index.ts index 492432c..5defb53 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,3 +1,4 @@ +export * from './createEffectComponent' export * from './EffectComposer' export * from './Selection' export * from './util' diff --git a/src/tests/EffectComposer.test.tsx b/src/tests/EffectComposer.test.tsx index 709c515..7fb1705 100644 --- a/src/tests/EffectComposer.test.tsx +++ b/src/tests/EffectComposer.test.tsx @@ -453,8 +453,9 @@ describe('EffectComposer', () => { } }) - it('never disposes the same ColorAverage instance twice, even in StrictMode', async () => { + it('disposes every ColorAverage instance seen, even across StrictMode\'s mount/cleanup/mount cycle', async () => { const disposedNodes: ColorAverageEffect[] = [] + const seenInstances = new Set() const disposeSpy = vi.spyOn(ColorAverageEffect.prototype, 'dispose').mockImplementation(function ( this: ColorAverageEffect ) { @@ -474,11 +475,17 @@ describe('EffectComposer', () => { ) ) await flush() + if (ref.current) seenInstances.add(ref.current) } await React.act(async () => root.render(null)) - const uniqueDisposed = new Set(disposedNodes) - expect(uniqueDisposed.size).toBe(disposedNodes.length) + // dispose() is idempotent (just event-firing / shallow property + // disposal, no internal state), so StrictMode calling it more than + // once per instance is fine - this only checks nothing leaked. + const disposedSet = new Set(disposedNodes) + for (const instance of seenInstances) { + expect(disposedSet.has(instance)).toBe(true) + } } finally { disposeSpy.mockRestore() } diff --git a/src/tests/createEffectComponent.test.tsx b/src/tests/createEffectComponent.test.tsx new file mode 100644 index 0000000..a6b2d49 --- /dev/null +++ b/src/tests/createEffectComponent.test.tsx @@ -0,0 +1,286 @@ +import { Effect, EffectComposer as EffectComposerImpl } from 'postprocessing' +import * as React from 'react' +import { Uniform } from 'three' +import { describe, expect, it, vi } from 'vitest' +import { createEffectComponent } from '../createEffectComponent' +import { EffectComposer } from '../EffectComposer' +import { flush, root } from './test-utils' + +// Zero-arity, real accessor - the class of effect createEffectComponent +// targets. Matches BloomEffect's shape: `constructor({...} = {})`. +class FakeEffect extends Effect { + private _value: number + constructor({ value = 0 }: { value?: number } = {}) { + super('FakeEffect', 'mainImage() {}') + this._value = value + } + get value() { + return this._value + } + set value(v: number) { + this._value = v + } +} + +const FakeEffectComponent = /* @__PURE__ */ createEffectComponent(FakeEffect) + +// Options stored only in `uniforms` (mirrors WaterEffectImpl/RampEffect +// before this branch gave them real accessors) - `factor` here HAS a real +// accessor, proving that's what makes a plain JSX prop actually reach it. +class UniformFixtureEffect extends Effect { + constructor({ factor = 0 }: { factor?: number } = {}) { + super('UniformFixtureEffect', 'mainImage() {}', { uniforms: new Map([['factor', new Uniform(factor)]]) }) + } + get factor(): number { + return this.uniforms.get('factor')!.value + } + set factor(v: number) { + this.uniforms.get('factor')!.value = v + } +} + +const UniformFixtureComponent = /* @__PURE__ */ createEffectComponent( + UniformFixtureEffect +) + +describe('createEffectComponent', () => { + it('constructs the effect and passes props through to the instance', async () => { + const composerRef = React.createRef() + + await React.act(async () => + root.render( + + + + ) + ) + + await flush() + + // @ts-expect-error - `effects` isn't part of the public Pass typing + const effect = composerRef.current!.passes[1].effects[0] + + expect(effect).toBeInstanceOf(FakeEffect) + expect(effect.value).toBe(42) + + await React.act(async () => root.render(null)) + }) + + it('applies a live prop without reconstructing the instance (r3f-native, no args change)', async () => { + const composerRef = React.createRef() + const ref = React.createRef() + + const render = (value: number) => + root.render( + + + + ) + + await React.act(async () => render(1)) + await flush() + const first = ref.current + expect(first!.value).toBe(1) + + await React.act(async () => render(2)) + await flush() + + expect(ref.current).toBe(first) + expect(ref.current!.value).toBe(2) + + await React.act(async () => root.render(null)) + }) + + it('resets a live prop to its constructor default when removed (r3f-native diffProps)', async () => { + const composerRef = React.createRef() + const ref = React.createRef() + + await React.act(async () => + root.render( + + + + ) + ) + await flush() + expect(ref.current!.value).toBe(5) + + await React.act(async () => + root.render( + + + + ) + ) + await flush() + + expect(ref.current!.value).toBe(0) + + await React.act(async () => root.render(null)) + }) + + it('reconstructs when an explicit args prop changes, same as any other r3f element', async () => { + const composerRef = React.createRef() + const ref = React.createRef() + + const render = (value: number) => + root.render( + + + + ) + + await React.act(async () => render(1)) + await flush() + const first = ref.current + expect(first!.value).toBe(1) + + await React.act(async () => render(2)) + await flush() + + expect(ref.current).not.toBe(first) + expect(ref.current!.value).toBe(2) + + await React.act(async () => root.render(null)) + }) + + it('applies blendFunction/opacity through blendMode, not as a stray top-level property', async () => { + const composerRef = React.createRef() + const ref = React.createRef() + + await React.act(async () => + root.render( + + + + ) + ) + await flush() + + expect(ref.current!.blendMode.blendFunction).toBe(7) + expect(ref.current!.blendMode.opacity.value).toBe(0.5) + expect((ref.current as unknown as { blendFunction?: unknown }).blendFunction).toBeUndefined() + + await React.act(async () => root.render(null)) + }) + + it('resets blendFunction/opacity to blendMode\'s own defaults when the props are removed', async () => { + const composerRef = React.createRef() + const ref = React.createRef() + + await React.act(async () => + root.render( + + + + ) + ) + await flush() + const defaultBlendFunction = ref.current!.blendMode.blendFunction + const defaultOpacity = ref.current!.blendMode.opacity.value + + await React.act(async () => + root.render( + + + + ) + ) + await flush() + expect(ref.current!.blendMode.blendFunction).toBe(7) + expect(ref.current!.blendMode.opacity.value).toBe(0.5) + + await React.act(async () => + root.render( + + + + ) + ) + await flush() + + expect(ref.current!.blendMode.blendFunction).toBe(defaultBlendFunction) + expect(ref.current!.blendMode.opacity.value).toBe(defaultOpacity) + + await React.act(async () => root.render(null)) + }) + + it('updates a uniforms-Map-backed prop live via its accessor, without reconstructing', async () => { + const composerRef = React.createRef() + const ref = React.createRef() + + const render = (factor: number) => + root.render( + + + + ) + + await React.act(async () => render(1)) + await flush() + const first = ref.current + expect(first!.uniforms.get('factor')!.value).toBe(1) + + await React.act(async () => render(2)) + await flush() + + expect(ref.current).toBe(first) + expect(ref.current!.uniforms.get('factor')!.value).toBe(2) + + await React.act(async () => root.render(null)) + }) + + it('disposes the instance on unmount', async () => { + const disposeSpy = vi.spyOn(FakeEffect.prototype, 'dispose') + const composerRef = React.createRef() + + try { + await React.act(async () => + root.render( + + + + ) + ) + await flush() + + await React.act(async () => root.render(null)) + + expect(disposeSpy).toHaveBeenCalledTimes(1) + } finally { + disposeSpy.mockRestore() + } + }) + + it('forwards a callback ref\'s own returned cleanup (React 19 ref cleanup), instead of dropping it', async () => { + const composerRef = React.createRef() + const events: string[] = [] + const cleanup = vi.fn(() => { + events.push('cleanup') + }) + const callbackRef = vi.fn((instance: FakeEffect | null) => { + events.push(instance ? 'attach' : 'attach-null') + if (instance) return cleanup + }) + + await React.act(async () => + root.render( + + + + ) + ) + await flush() + + expect(callbackRef).toHaveBeenCalledTimes(1) + expect(cleanup).not.toHaveBeenCalled() + + await React.act(async () => root.render(null)) + + // React 19 ref-cleanup semantics: once a cleanup is returned, it's + // called directly - the callback itself is never re-invoked with null. + expect(cleanup).toHaveBeenCalledTimes(1) + expect(callbackRef).toHaveBeenCalledTimes(1) + expect(events).toEqual(['attach', 'cleanup']) + }) +}) diff --git a/src/util.tsx b/src/util.tsx index e45d9e6..59aaf4f 100644 --- a/src/util.tsx +++ b/src/util.tsx @@ -1,6 +1,6 @@ import { useThree, type ReactThreeFiber } from '@react-three/fiber' import type { Selection as PPSelection } from 'postprocessing' -import { use, useEffect, useMemo, useRef, type RefObject } from 'react' +import { use, useEffect, useLayoutEffect, useMemo, useRef, type RefObject } from 'react' import { Object3D, Vector2, type Vector2Tuple } from 'three' import { selectionContext } from './Selection' @@ -12,13 +12,8 @@ export const EMPTY_ARRAY: never[] = [] export const resolveRef = (ref: T | RefObject) => typeof ref === 'object' && ref != null && 'current' in ref ? ref.current : ref -/** - * Keeps a postprocessing effect's `selection` (and its render layer) in - * sync with either mode effects support: the manual - * `selection` prop (used only when there's no enclosing ), or - * the declarative Selection/Select API. The two are mutually exclusive - - * context wins when both are present. - */ +// Keeps `selection` synced with whichever mode is active: the manual +// `selection` prop, or the declarative Selection/Select API (context wins). export function useSelectionSync( effect: { selection: PPSelection }, selection: Object3D | Object3D[] | RefObject | RefObject[], @@ -59,26 +54,89 @@ export function useSelectionSync( }, [api, effect.selection, invalidate]) } -/** - * r3f never disposes objects (their state may be owned outside - * React), so effects rendered that way must dispose themselves. Guards - * against double-dispose across StrictMode's dev-only mount/cleanup/mount - * cycle, where the cleanup closure re-runs against the same instance. - */ +// r3f never disposes objects, so they must dispose themselves. export const useDispose = void }>(instance: T): void => { - const disposedRef = useRef>(new WeakSet()) - useEffect(() => { - const disposed = disposedRef.current return () => { - if (instance && typeof instance === 'object' && !disposed.has(instance)) { - disposed.add(instance) - instance.dispose?.() - } + instance?.dispose?.() } }, [instance]) } +// Reads a plain or r3f-pierced ("a-b-c") key off an object, matching what +// applyProps below can write - a single reader that works for both shapes. +export function readPierced(instance: object, key: string): unknown { + let target: unknown = instance + for (const part of key.split('-')) { + if (target == null) return undefined + target = (target as Record)[part] + } + return target +} + +// Writes a plain or r3f-pierced ("a-b-c") key, mirroring readPierced. +// Deliberately a plain assignment, not r3f's applyProps: these instances +// are built with `new`, not r3f's reconciler, so applyProps' Color/Vector +// coercion never applies to them. Callers wrap values that need coercion. +export function applyPierced(instance: object, key: string, value: unknown): void { + const parts = key.split('-') + let target: unknown = instance + for (let idx = 0; idx < parts.length - 1; idx++) { + if (target == null) return + target = (target as Record)[parts[idx]] + } + if (target == null) return + ;(target as Record)[parts[parts.length - 1]] = value +} + +// Applies live-mutable properties onto an instance built via `new` (not +// r3f's reconciler, so r3f's own prop diffing/reset never runs on it). +// Falls back to the constructor-time default when a value is `undefined`. +// Only calls `set` when the resolved value actually changed since the last +// apply - some setters have side effects beyond storing the value (e.g. +// OutlineEffect's `multisampling` disposes its render target on every set). +export function useLiveDefaults( + instance: T | RefObject | null, + values: Record, + keys: Iterable, + get: (instance: T, key: string) => unknown = readPierced, + set: (instance: T, key: string, value: unknown) => void = applyPierced +): void { + const snapshotRef = useRef<{ instance: T; defaults: Map; applied: Map } | null>(null) + const invalidate = useThree((state) => state.invalidate) + + useLayoutEffect(() => { + const resolved = resolveRef(instance) + if (!resolved) return + if (snapshotRef.current?.instance !== resolved) { + snapshotRef.current = { instance: resolved, defaults: new Map(), applied: new Map() } + } + const { defaults, applied } = snapshotRef.current + let changed = false + + for (const key of keys) { + if (!defaults.has(key)) { + // Seed `applied` too, not just `defaults`, so an unchanged key + // skips `set` even on this first pass (avoids re-triggering + // setters with side effects, e.g. multisampling's dispose). + const current = get(resolved, key) + defaults.set(key, current) + applied.set(key, current) + } + const next = values[key] !== undefined ? values[key] : defaults.get(key) + if (Object.is(applied.get(key), next)) continue + set(resolved, key, next) + applied.set(key, next) + changed = true + } + + // These instances are mutated directly (not via r3f's reconciler), so + // r3f never sees the change - without this, frameloop="demand" would + // never repaint after a live prop update. + if (changed) invalidate() + }) +} + export const useVector2 = (props: Record, key: string): Vector2 => { const value = props[key] as ReactThreeFiber.Vector2 | undefined diff --git a/src/wrapEffect.tsx b/src/wrapEffect.tsx index 6fc1f44..2f9933d 100644 --- a/src/wrapEffect.tsx +++ b/src/wrapEffect.tsx @@ -1,8 +1,9 @@ import { extend, useThree } from '@react-three/fiber' -import type { BlendFunction, Effect, Pass } from 'postprocessing' +import type { BlendFunction } from 'postprocessing' import { useMemo, type ExoticComponent, type JSX, type Ref } from 'react' +import type { EffectConstructor } from './createEffectComponent' -export type EffectConstructor = new (...args: any[]) => Effect | Pass +export type { EffectConstructor } // Handles three ConstructorParameters shapes: required first param // (P), optional first param (Partial

— some effects in postprocessing