brianzinn / react-babylonjs

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

babylon-image source wants a string & mesh #123

Closed MikeMcMahon closed 3 years ago

MikeMcMahon commented 3 years ago

trying to specify the source of a babylon-image requires a type of string & mesh

        <babylon-image
          horizontalAlignment={Control.HORIZONTAL_ALIGNMENT_CENTER}
          verticalAlignment={Control.VERTICAL_ALIGNMENT_CENTER}
          source={"./img/icons_clipboard.png"} // This does not work
          height={0.8}
          width={0.8}
          stretch={Image.STRETCH_FILL}
        />

I end up having to specify my images with an initial URL. Unfortunately updating this URL after the fact doesn't seem to change the image (for toggle button purposes). We end up having to create imgRef's and modify the image.source attribute directly which works.

brianzinn commented 3 years ago

hi @MikeMcMahon Interesting question - the whole GUI is really I feel a strong point of declarative JSX. I just looked at the image constructor: https://doc.babylonjs.com/typedoc/classes/babylon.gui.image#constructor

It has a url in the constructor ONLY, so that is typically how image is set, but you cannot update via url prop. You are trying to set the source and the renderer will pass that through to the image object (I just checked the prop handler). You can also use a <babylon-image key={props.imageSource} url={props.imageSource}> as a hacky workaround to re-create and dispose the existing image. I think this may be due to the underlying image itself not updating the 'source'? Do you have more information or a babylonjs playground - are you saying that you do (just free-typed so may not be valid, but as an example):

const imageRef=useRef();

useEffect(() => {
  imageRef.current.source = props.newSource; // this is a string and it only works this way?
});

<babylon-image ref={imageRef} .../>

Would like to help work out the solution. I haven't used images that way just "image" fonts in the GUI, so just need a repro and I can work on a solution. Cheers.

edit: also "./img/icons_clipboard.png" is in your /public/img and that is relative to your current path (ie: you do not have a 404 in your network tab). I would suggest to use just '/img/icons_clipboard.png', so it works in nested paths - unless you use different images based on paths, which seems unlikely.

brianzinn commented 3 years ago

This works in a playground - are you saying you were setting the source and it wasn't working?

var advancedTexture = BABYLON.GUI.AdvancedDynamicTexture.CreateFullscreenUI("UI");
// https://doc.babylonjs.com/toolsAndResources/assetLibraries/availableTextures

var image = new BABYLON.GUI.Image("img", "textures/crate.png");

advancedTexture.addControl(image);

setTimeout(() => {
    image.source='textures/fire.png'
}, 2000)

Just want to confirm the nature of the issue. Thanks.

MikeMcMahon commented 3 years ago

Not sure how to do a babylonjs playground w/ react, but let me get you a better example of what I am talking about that describes it better than I can with words. :D

brianzinn commented 3 years ago

I think I found the issue - it's from the typings. Here is a working sandbox on the 'source' though - you can see the compiler complains, but it runs: https://codesandbox.io/s/frosty-grass-h41ol?file=/src/App.tsx

import "./styles.css";

import React, { useState } from "react";
import { Engine, Scene } from "react-babylonjs";
import { Vector3 } from "@babylonjs/core";

const imageList = ["/img/crate.png", "/img/fire.png"];

export default function App() {
  const [imageIndex, setImageIndex] = useState<number>(0);
  const buttonClick = () => {
    setImageIndex((current) => current + 1);
  };
  return (
    <>
      <div className="App">
        <h1>Hello Babylon</h1>
        <button onClick={buttonClick}>change image</button>
      </div>
      <div>
        <Engine antialias adaptToDeviceRatio canvasId="babylonCanvas">
          <Scene>
            <arcRotateCamera
              name="camera1"
              target={Vector3.Zero()}
              alpha={Math.PI / 2}
              beta={Math.PI / 4}
              radius={8}
            />
            <hemisphericLight
              name="light1"
              intensity={0.7}
              direction={Vector3.Up()}
            />
            <adtFullscreenUi name="ui">
              <rectangle name="rect" height="50px" width="150px">
                <babylon-image source={imageList[imageIndex % 2]} />
              </rectangle>
            </adtFullscreenUi>
          </Scene>
        </Engine>
      </div>
    </>
  );
}
brianzinn commented 3 years ago

I think you have found a bug! The typings problem is because I have "source" from InstancedMesh declared on all host elements. It's a constructor argument for InstancedMesh. I think it can be safely removed from the "CustomProps" type, which is being incorrectly applied here via that impossible "Intersection Type".

Type 'string' is not assignable to type '(string & Mesh) | undefined'.ts(2322)
... The expected type comes from property 'source'
MikeMcMahon commented 3 years ago

Wow, awesome! You beat me to providing an example. That's exactly what we are encountering :D

brianzinn commented 3 years ago

Thanks for reporting the issue. Should be fixed in 3.0.11 😃 image