brianzinn / react-babylonjs

React for Babylon 3D engine
https://brianzinn.github.io/react-babylonjs/
818 stars 105 forks source link

diffuse and specular props not re rendering light in scene #140

Closed hupeky closed 3 years ago

hupeky commented 3 years ago

Hi brian,

Im having an issue where the specular and diffuse of light components are not getting updated when new prop values are being passed

I know from the console.log that data.color is being changed, but the light does not reflect the change

any help would be appreciated thanks.

const PointLightComponent = ({ id, onClick }: InteractableProps): ReactElement => {
  const dragRef = useRef<Nullable<AbstractMesh>>(null);
  const data = useInteractable(id, dragRef.current) as PointLight;
  console.log(data.color);
  return (
    <>
      {data && (
        <sphere
          ref={dragRef}
          onCreated={(e) => onClick(e)}
          name={id}
          updatable
          segments={7}
          receiveShadows={false}
          diameter={0.3}
          position={data.position}
        >
          {data.color && <HandleMat color={data.color} />}
          <pointLight
            specular={data.color}
            diffuse={data.color}
            range={data.range}
            name={id}
            position={new Vector3(0, 0, 0)}
            intensity={data.intensity}
          />
          <pointerDragBehavior />
        </sphere>
      )}
    </>
  );
};
brianzinn commented 3 years ago

Thanks @kyehuelin for reporting. I'll get back to you tomorrow on this. Haven't come across that one before. Sounds like you are logging the color change and the pointLight doesn't update.

hupeky commented 3 years ago

@brianzinn

yes, so ive got some sliders changing various props (intensity, range, color). the console.log shows that the data is changing including color.

the intensity and range are updating fine. color does not.

its interesting that intensity and range are raw values and color is a class. is it possible that there is an incorrect comparison being made on the reference rather than the data of the class. (r,g,b)

brianzinn commented 3 years ago

How are you setting data.color? This works for example:

const red = useRef(0);
const [pointLightDiffuse, setPointLightDiffuse] = useState(new Color3(red.current, 0.5, 0.5));

useEffect(() => {
  const handle = setInterval(() => {
    red.current = (red.current + 0.1) % 1;
    setPointLightDiffuse(new Color3(red.current, 0.5, 0.5));
  }, 1000);
  return () => clearInterval(handle);
}, []);

return (
  <Engine antialias adaptToDeviceRatio engineOptions={{ preserveDrawingBuffer: true, stencil: true }} canvasId='babylonJS'>
    <Scene clearColor={new Color3(0, 0, 0)}>
      <pointLight name='omni' position={new Vector3(0, 50, 0)} diffuse={pointLightDiffuse} />
     ...
     </Scene>
  </Engine
);

The reconciler is calling Color3 equals method, which checks r, g and b. Are you maintaining the same reference - if so I can update my example that maintains Color3 ref.

hupeky commented 3 years ago

ah, when i use

  const newColor = new Color3(data.color.r, data.color.g, data.color.b);
...
          <spotLight
            specular={newColor}
            diffuse={newColor}

it works. I guess the i was saving the ref of the color in the store and not a copy of the data. so Ill update my store to copy values in rather than the ref of the object

thank you

brianzinn commented 3 years ago

It should work though without you creating a new Color3. I am calling equals on the instance: https://github.com/BabylonJS/Babylon.js/blob/master/src/Maths/math.color.ts#L141

It should be checking r/g/b values. Here is the comparison I do: https://github.com/brianzinn/react-babylonjs/blob/master/src/PropsHandler.ts#L245

If you could share a repro then hopefully I would be able to figure it out. It's a performance penalty, but I avoided reference checks to allow people to re-use same instance, which helps with GC pressure.

brianzinn commented 3 years ago

going to close, since you found a solution - despite the behaviour working not as I was expecting. Please re-open if you'd like to work on references updating the color, which was how I would expect - there may be a bug there on my side. Cheers.