fix(Outline, SelectiveBloom): stop unnecessary effect reconstruction and re-runs

src/effects/Outline.tsx

@@ -1,12 +1,10 @@
-import { useThree } from '@react-three/fiber'
 import { OutlineEffect } from 'postprocessing'
-import { Ref, RefObject, useContext, useEffect, useMemo } from 'react'
+import { Ref, RefObject, use, useMemo } from 'react'
 import { Object3D } from 'three'
 import { EffectComposerContext } from '../EffectComposer'
-import { selectionContext } from '../Selection'
-import { resolveRef, useDispose } from '../util'
+import { EMPTY_ARRAY, useDispose, useSelectionSync } from '../util'
 
-type ObjectRef = RefObject<Object3D>
+type ObjectRef = RefObject<Object3D | null>
 
 export type OutlineProps = ConstructorParameters<typeof OutlineEffect>[2] &
   Partial<{
@@ -16,95 +14,71 @@ export type OutlineProps = ConstructorParameters<typeof OutlineEffect>[2] &
   }>
 
 export function Outline({
-  selection = [],
+  selection = EMPTY_ARRAY,
   selectionLayer = 10,
   blendFunction,
   patternTexture,
+  patternScale,
   edgeStrength,
   pulseSpeed,
   visibleEdgeColor,
   hiddenEdgeColor,
+  multisampling,
+  resolutionScale,
+  resolutionX,
+  resolutionY,
   width,
   height,
   kernelSize,
   blur,
   xRay,
   ref,
-  ...props
 }: OutlineProps) {
-  const invalidate = useThree((state) => state.invalidate)
-  const { scene, camera } = useContext(EffectComposerContext)
+  const { scene, camera } = use(EffectComposerContext)
 
   const effect = useMemo(
     () =>
       new OutlineEffect(scene, camera, {
         blendFunction,
         patternTexture,
+        patternScale,
         edgeStrength,
         pulseSpeed,
         visibleEdgeColor,
         hiddenEdgeColor,
+        multisampling,
+        resolutionScale,
+        resolutionX,
+        resolutionY,
         width,
         height,
         kernelSize,
         blur,
         xRay,
-        ...props,
       }),
-    // NOTE: `props` is an unstable reference, so we can't memoize it
-    // eslint-disable-next-line react-hooks/exhaustive-deps
     [
       blendFunction,
-      blur,
-      camera,
-      edgeStrength,
-      height,
-      hiddenEdgeColor,
-      kernelSize,
       patternTexture,
+      patternScale,
+      edgeStrength,
       pulseSpeed,
-      scene,
       visibleEdgeColor,
+      hiddenEdgeColor,
+      multisampling,
+      resolutionScale,
+      resolutionX,
+      resolutionY,
       width,
+      height,
+      kernelSize,
+      blur,
       xRay,
+      camera,
+      scene,
     ]
   )
 
-  const api = useContext(selectionContext)
-
-  useEffect(() => {
-    // Do not allow array selection if declarative selection is active
-    // TODO: array selection should probably be deprecated altogether
-    if (!api && selection) {
-      effect.selection.set(
-        Array.isArray(selection) ? (selection as Object3D[]).map(resolveRef) : [resolveRef(selection) as Object3D]
-      )
-      invalidate()
-      return () => {
-        effect.selection.clear()
-        invalidate()
-      }
-    }
-  }, [effect, selection, api, invalidate])
-
-  useEffect(() => {
-    effect.selectionLayer = selectionLayer
-    invalidate()
-  }, [effect, invalidate, selectionLayer])
-
-  useEffect(() => {
-    if (api && api.enabled) {
-      if (api.selected?.length) {
-        effect.selection.set(api.selected)
-        invalidate()
-        return () => {
-          effect.selection.clear()
-          invalidate()
-        }
-      }
-    }
-  }, [api, effect.selection, invalidate])
-
+  useSelectionSync(effect, selection, selectionLayer)
   useDispose(effect)
 
   return <primitive ref={ref} object={effect} />

src/effects/SelectiveBloom.tsx

@@ -1,13 +1,12 @@
 import { useThree } from '@react-three/fiber'
 import type { BloomEffectOptions } from 'postprocessing'
 import { BlendFunction, SelectiveBloomEffect } from 'postprocessing'
-import { Ref, RefObject, useContext, useEffect, useMemo } from 'react'
+import { Ref, RefObject, use, useEffect, useMemo } from 'react'
 import { Object3D } from 'three'
 import { EffectComposerContext } from '../EffectComposer'
-import { selectionContext } from '../Selection'
-import { resolveRef, useDispose } from '../util'
+import { EMPTY_ARRAY, resolveRef, useDispose, useSelectionSync } from '../util'
 
-type ObjectRef = RefObject<Object3D>
+type ObjectRef = RefObject<Object3D | null>
 
 export type SelectiveBloomProps = BloomEffectOptions &
   Partial<{
@@ -23,103 +22,94 @@ const addLight = (light: Object3D, effect: SelectiveBloomEffect) => light.layers
 const removeLight = (light: Object3D, effect: SelectiveBloomEffect) => light.layers.disable(effect.selection.layer)
 
 export function SelectiveBloom({
-  selection = [],
+  selection = EMPTY_ARRAY,
   selectionLayer = 10,
-  lights = [],
+  lights = EMPTY_ARRAY,
   inverted = false,
   ignoreBackground = false,
   luminanceThreshold,
   luminanceSmoothing,
+  mipmapBlur,
   intensity,
+  radius,
+  levels,
+  kernelSize,
+  resolutionScale,
   width,
   height,
-  kernelSize,
-  mipmapBlur,
+  resolutionX,
+  resolutionY,
   ref,
-  ...props
 }: SelectiveBloomProps) {
-  if (lights.length === 0) {
-    console.warn('SelectiveBloom requires lights to work.')
-  }
+  const { scene, camera } = use(EffectComposerContext)
 
   const invalidate = useThree((state) => state.invalidate)
-  const { scene, camera } = useContext(EffectComposerContext)
+
   const effect = useMemo(() => {
-    const effect = new SelectiveBloomEffect(scene, camera, {
+    const instance = new SelectiveBloomEffect(scene, camera, {
       blendFunction: BlendFunction.ADD,
       luminanceThreshold,
       luminanceSmoothing,
+      mipmapBlur,
       intensity,
+      radius,
+      levels,
+      kernelSize,
+      resolutionScale,
       width,
       height,
-      kernelSize,
-      mipmapBlur,
-      ...props,
+      resolutionX,
+      resolutionY,
     })
-    effect.inverted = inverted
-    effect.ignoreBackground = ignoreBackground
-    return effect
+    instance.inverted = inverted
+    instance.ignoreBackground = ignoreBackground
+    return instance
   }, [
     scene,
     camera,
     luminanceThreshold,
     luminanceSmoothing,
+    mipmapBlur,
     intensity,
+    radius,
+    levels,
+    kernelSize,
+    resolutionScale,
     width,
     height,
-    kernelSize,
-    mipmapBlur,
+    resolutionX,
+    resolutionY,
     inverted,
     ignoreBackground,
-    props,
   ])
 
-  const api = useContext(selectionContext)
+  // Must run before the lights effect below: addLight/removeLight read
+  // effect.selection.layer live, so it needs to already reflect the
+  // latest selectionLayer by the time lights get (re-)assigned to it.
+  useSelectionSync(effect, selection, selectionLayer)
 
   useEffect(() => {
-    // Do not allow array selection if declarative selection is active
-    // TODO: array selection should probably be deprecated altogether
-    if (!api && selection) {
-      effect.selection.set(
-        Array.isArray(selection) ? (selection as Object3D[]).map(resolveRef) : [resolveRef(selection) as Object3D]
-      )
-      invalidate()
-      return () => {
-        effect.selection.clear()
-        invalidate()
-      }
+    if (lights.length === 0) {
+      console.warn('SelectiveBloom requires lights to work.')
+      return
     }
-  }, [effect, selection, api, invalidate])
 
-  useEffect(() => {
-    effect.selection.layer = selectionLayer
+    // Refs may not have attached yet - resolve and drop nullish entries
+    // rather than crashing addLight/removeLight on a null object.
+    const resolvedLights = lights.map((light) => resolveRef(light)).filter((light): light is Object3D => light != null)
+    if (resolvedLights.length === 0) return
+
+    resolvedLights.forEach((light) => addLight(light, effect))
+
     invalidate()
-  }, [effect, invalidate, selectionLayer])
 
-  useEffect(() => {
-    if (lights && lights.length > 0) {
-      lights.forEach((light) => addLight(resolveRef(light), effect))
+    return () => {
+      resolvedLights.forEach((light) => removeLight(light, effect))
+
       invalidate()
-      return () => {
-        lights.forEach((light) => removeLight(resolveRef(light), effect))
-        invalidate()
-      }
     }
   }, [effect, invalidate, lights, selectionLayer])
 
-  useEffect(() => {
-    if (api && api.enabled) {
-      if (api.selected?.length) {
-        effect.selection.set(api.selected)
-        invalidate()
-        return () => {
-          effect.selection.clear()
-          invalidate()
-        }
-      }
-    }
-  }, [api, effect.selection, invalidate])
-
   useDispose(effect)
 
   return <primitive ref={ref} object={effect} />

src/tests/Outline.test.tsx

@@ -0,0 +1,89 @@
+import { EffectComposer as EffectComposerImpl, OutlineEffect, Selection as PPSelection } from 'postprocessing'
+import * as React from 'react'
+import { Mesh, Object3D } from 'three'
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { EffectComposer } from '../EffectComposer'
+import { Outline } from '../effects/Outline'
+import { Select, Selection } from '../Selection'
+import { flush, root, waitForComposer } from './test-utils'
+
+afterEach(async () => {
+  await React.act(async () => {
+    root.render(null)
+  })
+})
+
+describe('Outline', () => {
+  it('does not re-set its (empty, declarative-mode) selection on unrelated re-renders', async () => {
+    const setSpy = vi.spyOn(PPSelection.prototype, 'set')
+    const composerRef = React.createRef<EffectComposerImpl>()
+
+    const render = (tick: number) =>
+      root.render(
+        <EffectComposer ref={composerRef}>
+          <Outline />
+          <group name={`tick-${tick}`} />
+        </EffectComposer>
+      )
+
+    await React.act(async () => render(0))
+    await waitForComposer(composerRef)
+    await flush()
+    setSpy.mockClear()
+
+    for (let t = 1; t <= 5; t++) {
+      await React.act(async () => render(t))
+      await flush()
+    }
+
+    expect(setSpy).not.toHaveBeenCalled()
+    setSpy.mockRestore()
+  })
+
+  it('sets its selection from the Selection/Select API and clears it when the object deselects', async () => {
+    const effectRef = React.createRef<OutlineEffect>()
+    const meshRef = React.createRef<Mesh>()
+
+    const render = (enabled: boolean) =>
+      root.render(
+        <EffectComposer>
+          <Selection>
+            <Select enabled={enabled}>
+              <mesh ref={meshRef}>
+                <boxGeometry />
+                <meshBasicMaterial />
+              </mesh>
+            </Select>
+            <Outline ref={effectRef} />
+          </Selection>
+        </EffectComposer>
+      )
+
+    await React.act(async () => render(true))
+    await flush()
+    await flush()
+
+    expect(Array.from(effectRef.current!.selection)).toContain(meshRef.current)
+
+    await React.act(async () => render(false))
+    await flush()
+    await flush()
+
+    expect(Array.from(effectRef.current!.selection)).not.toContain(meshRef.current)
+  })
+
+  it('does not throw when a selection ref has not attached yet', async () => {
+    const composerRef = React.createRef<EffectComposerImpl>()
+    const unattachedRef = React.createRef<Object3D>()
+
+    await React.act(async () =>
+      root.render(
+        <EffectComposer ref={composerRef}>
+          <Outline selection={[unattachedRef]} />
+        </EffectComposer>
+      )
+    )
+    await waitForComposer(composerRef)
+    await expect(flush()).resolves.not.toThrow()
+  })
+})