react-postprocessing icon indicating copy to clipboard operation
react-postprocessing copied to clipboard

Dynamically changing props (e.g. visibleEdgeColor) of Outline breaks it / no longer renders

Open Glavin001 opened this issue 1 year ago • 2 comments

Problem

Demo of bug and workaround: https://codesandbox.io/s/bug-change-outline-color-dmu52j?file=/src/App.js

✅ Constant/fixed visibleEdgeColor

<EffectComposer multisampling={8} autoClear={false}>
  <Outline
    blur edgeStrength={100} width={500}
    visibleEdgeColor="red"
  />
</EffectComposer>
image

🔥 Dynamically changing visibleEdgeColor breaks Outline, stops rendering

+ // `color` variable is dynamically changing
  <EffectComposer multisampling={8} autoClear={false}>
    <Outline
      blur edgeStrength={100} width={500}
+     visibleEdgeColor={color}
    />
  </EffectComposer>
image

Workaround

Changing the visibleEdgeColor of Outline element breaks it. Workaround is to change the key of EffectComposer so everything re-mounts and re-renders.

  <EffectComposer
   multisampling={8} autoClear={false}
+  key={color}
  >
    <Outline
      blur edgeStrength={100} width={500}
-     visibleEdgeColor={"red"}
+     visibleEdgeColor={color}
    />
  </EffectComposer>

To see this in action, change the APPLY_WORKAROUND in the demo sandbox.


Is this expected behaviour? Thanks!

Glavin001 avatar Aug 06 '22 03:08 Glavin001

I think the behavior is expected, though it's good you have a workaround. Typically it's good practice to mutate the effect itself as opposed to updating it via passing new props.

const outline = useRef()
const color1 = new THREE.Color("red")
const color2 = new THREE.Color("green")

useFrame(() => {
  outline.current.visibleEdgeColor.set(hover ? color1 : color2) // option 1: mutate the effect if it provides setters
  color1.set("green") // option 2: mutate the color itself, not sure if that works though.
})

<Outline ref={outline} visibleEdgeColor={color1} />

here's a minimal example based on yours: https://codesandbox.io/s/bug-change-outline-color-forked-y3uvy4?file=/src/App.js

the postprocessing docs shows which setters are public, and thus can be mutated with a ref: https://pmndrs.github.io/postprocessing/public/docs/class/src/effects/OutlineEffect.js~OutlineEffect.html#instance-set-visibleEdgeColor

chasedavis avatar Aug 06 '22 21:08 chasedavis

Thank you! Mutating the effect makes sense as the solution.

Typically it's good practice to mutate the effect itself as opposed to updating it via passing new props.

Would it be valuable to have prop changes throw a warning message, since they are actually only initial values? Or to allow the props to update as expected, similar to selectionLayer, mutating the effect within the Outline component?

Glavin001 avatar Aug 08 '22 15:08 Glavin001