Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<mesh> element, fromInstance not updating properly #205

Closed
DefaultV opened this issue Feb 25, 2022 · 9 comments
Closed

<mesh> element, fromInstance not updating properly #205

DefaultV opened this issue Feb 25, 2022 · 9 comments

Comments

@DefaultV
Copy link

Bug repo:
https://github.com/DefaultV/create-react-app-typescript-babylonjs/tree/Bug-MeshFromInstanceChange

Hello! Currently when using the <mesh fromInstance={...}> property, it doesn't seem to dispose of the old mesh when updating the value, nor update the reference to the new mesh. Is this intended behaviour or am I missing something?

  1. In the following repo, I'm firstly creating a box within a useState(box).
  2. Then I pass this to a <mesh fromInstance={box}> component.
  3. Then I replace the state with a new sphere mesh, setState(MeshBuilder.CreateSphere(...)

Following screenshot is the result, I expected the box to disappear on step 3

image

@brianzinn
Copy link
Owner

that is by design, but certainly I can consider changing that. the fromInstance is equivalent currently to instantiation (or in this case replacement for instantiation), which happens only at time of object creation. the reconciler understands that - have you considered using the key prop to signal to the reconciler that it is a different object. I'm trying to work out if this should be part of core functionality and maybe you can make that case?

@DefaultV
Copy link
Author

In my other project I use the key prop for multiple objects which all seem to have the same behaviour. What would be a workaround for this? Storing refs to the meshes I need to replace and then disposing re-parenting the new ones?

Maybe my approach is just very niché, one of the functionalities of my project is to generate and replace meshes at runtime.

I don't know if it should be core functionality, but if I were supposed to override the instance I would expect the old mesh to be disposed somehow, maybe have a separate prop for disposeMeshOnOverride. Just another name than that 😄

@brianzinn
Copy link
Owner

hi @DefaultV I think I understand your issue and hope this is a solution. There exists that functionality already and it is an "opt-in" mechanism. Whenever fromInstance prop is used I am assuming that the developer wants to manage the lifetime of the object. Imagine, for example, that you want to re-use a Texture and pass it through in different places, etc.

It's a bit buried in the code, but I am redoing the docs right now and will make sure to have an example that uses this property. Also, there will be a webpage dedicated to all of these Custom Props. Anyway, this ought to fix it for you:??

   <mesh fromInstance={box} disposeInstanceOnUnmount />

https://github.com/brianzinn/react-babylonjs/blob/master/packages/react-babylonjs/src/CustomProps.ts#L24

@DefaultV
Copy link
Author

DefaultV commented Mar 1, 2022

That sounds great! I'd love to see an example, I'm currently having a hard time to get it to work on my end. See my PR for more:
https://github.com/DefaultV/create-react-app-typescript-babylonjs/tree/Bug-MeshFromInstanceChange

@brianzinn
Copy link
Owner

sorry - I totally didn't see your comment. i will look tonight.

@brianzinn
Copy link
Owner

brianzinn commented Mar 8, 2022

I made a couple of small changes to your Box.tsx to make it clearer and isolate the issue. You can put back the useRef if you want.

import { Mesh, MeshBuilder } from "@babylonjs/core";
import React from "react";
import { FunctionComponent, useState } from "react";
import { useScene } from "react-babylonjs";

const Boxed: FunctionComponent = () => {
  const scene = useScene();
  const [mesh, setNewMesh] = useState<Mesh>(() => {
    const boxMesh = MeshBuilder.CreateBox(
      "box",
      {
        size: 2
      },
      scene
    );
    boxMesh.position.y = 1;
    boxMesh.onDisposeObservable.add(() => console.log("disposing"));
    console.log("initial box created");
    return boxMesh;
  });

  const handleOnCreate = (mesh: Mesh) => {
    if (mesh.name === "box") {
      setTimeout(() => {
        const sphereMesh = MeshBuilder.CreateSphere("sphere", {}, scene);
        sphereMesh.position.y = 1;
        console.log("created sphere");
        setNewMesh(sphereMesh);
      }, 2500);
    }
  };

  console.log(mesh.name, mesh.uniqueId, mesh.isDisposed());

  return (
    <mesh
      key={mesh.name}
      name="boxed"
      fromInstance={mesh}
      onCreated={handleOnCreate}
      disposeInstanceOnUnmount
    />
  );
};

export { Boxed };

output:

Babylon.js v4.2.1 - WebGL2 - Parallel shader compilation 
initial box created 
box, 3. false
boxed, 3, false
disposing 
created sphere 
sphere, 17, false

Notes:

  1. the name "boxed" is applied to the mesh. that is because there is a props handler. you can see it's the same mesh from the uniqueId
  2. the onCreated callback will be fired even when you assign a new one, since the "key" changes and it will call those lifecycle events.
  3. useState with the init function is only called once - not on each render.

You can always use a useEffect hook and call dispose yourself, but what I have shown above is how to get the reconciler to work with you.

If you have a tricker example then feel free to share, but otherwise I hope that makes it clear. 😄

@DefaultV
Copy link
Author

DefaultV commented Mar 8, 2022

Thanks a ton for the example! I am still quite curious.
On your example, changing line 8 to just:

const [mesh, setNewMesh] = useState<Mesh>(
    MeshBuilder.CreateBox(
      "box",
      {
        size: 2,
      },
      scene
    )
  );

Seems to break the functionality somehow, why is that?

@brianzinn
Copy link
Owner

It will be evaluated evey render, whereas a function () => ... is only called the first time useState is called. The best way to imagine it is that javascript will pass in a function without evaluating the contents, but if you pass in code then it will be run by javascript and passed in (and ignored after the first render, but "ignoring" a MeshBuilder.CreateBox, will still create a box).

There is a good explanation here - better than the React docs:
https://blog.logrocket.com/a-guide-to-usestate-in-react-ecb9952e406c/

@brianzinn
Copy link
Owner

Please re-open if you have more questions or anything is unclear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants