brianzinn / react-babylonjs

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

colorPicker value not changing #131

Closed hupeky closed 3 years ago

hupeky commented 3 years ago

currently running

    "@babylonjs/core": "^4.2.0",
     "react-babylonjs": "^3.0.14",

Hi Brian, when i set the value to a given color the colorpicker is not updating to that color, it just stays red, which i guess is the default color.

const green = new Color3(0, 1, 0);
return (
...
      <colorPicker value={green} ... />
...
brianzinn commented 3 years ago

thanks for reporting @kyehuelin . I will investigate - have never actually used that control before :)

brianzinn commented 3 years ago

It seems like a bug in babylonjs where it's not invalidating the control to redraw. Here is a codesandbox to reproduce: https://codesandbox.io/s/falling-voice-p4sjs?file=/src/App.js

Notice that the color is changing in the inspector, but not in the UI: image

image

I cannot explain it right now :( I tried it on a regular babylonjs playground and it works... I haven't come across before where something in the Explorer wasn't reflected in the canvas.

brianzinn commented 3 years ago

Actually I found the reason. It's because I do a copyFrom.

colorPicker.value.copyFrom(Color3.Green());

That won't trigger the code needed to update the GUI (https://github.com/BabylonJS/Babylon.js/blob/master/gui/src/2D/controls/colorpicker.ts#L49). I think there will be no adverse side effects if I change that to assignment.

brianzinn commented 3 years ago

Good find - took me a bit of digging, because I was looking in the wrong place!! Fixed in version 3.0.15. please re-open if it's not working.

hupeky commented 3 years ago

Nice find. thank you, Ive realised that this maybe a wider issue with all Intrinsicelements values being passed as props.

for example, all the lights have a diffuse and specular, which have the same problem as colorPicker. once the value are set, there is no changing them again. mayb this has already been fixed for items with your update. as ive not tested it.

Edit: also for example standardMaterial.emissiveColor

Im having to esacape with ref every time i have to change values like this


const HandleMat = ({ name = "handleMat", color }: Props) => {
  const mat = useCallback(
    (node: StandardMaterial) => {
      if (node !== null) {
        node.emissiveColor = color;
      }
      // eslint-disable-next-line react-hooks/exhaustive-deps
    },
    [color]
  );
  return (
    <standardMaterial
      ref={mat}
      name={name}
      maxSimultaneousLights={8}
      disableLighting
      emissiveColor={color}
    />
  );
};
hupeky commented 3 years ago

@brianzinn added a bit more :)