pmndrs / react-three-fiber

🇨🇭 A React renderer for Three.js
https://docs.pmnd.rs/react-three-fiber
MIT License
27.62k stars 1.6k forks source link

Instance switching / constructor argument changes for Object3Ds do not work properly #3317

Open Omxjep3434 opened 3 months ago

Omxjep3434 commented 3 months ago

Description

There is a bug in the switchInstance() method in renderer.ts when invoked on a class deriving from Object3D. Not all children will be preserved during the switch due to the nature of the for..of loop, and removing children of the instance within that loop. The loop needs to be modified to support instance.children having elements removed during iteration.

Code example:

In the following code example, you'll see that the second mesh gets lost when the Group is reconstructed.

function InstanceSwitchingIssue() {
  const [constructorArg, setConstructorArg] = useState(0);

  return (
    <>
      <button onClick={() => setConstructorArg(constructorArg + 1)}>Reconstruct Group</button>

      <Canvas>
        <group args={[constructorArg]}>
          <mesh width={1} geometry={new BoxGeometry()} position={[-1, 0, 0]} material-color="red">
          </mesh>

          <mesh width={1} geometry={new BoxGeometry()} position={[1, 0, 0]} material-color="green">
          </mesh>
        </group>
      </Canvas>
    </>
  );
}

Problematic code (renderer.ts)

    // https://github.com/pmndrs/react-three-fiber/issues/1348
    // When args change the instance has to be re-constructed, which then
    // forces r3f to re-parent the children and non-scene objects
    if (instance.children) {
      for (const child of instance.children) {
        // MY NOTE: Appending to the new instance will also remove from the old instance, causing the iteration problem.
        if (child.__r3f) appendChild(newInstance, child)
      }
      instance.children = instance.children.filter((child) => !child.__r3f)
    }