Replacing a mesh with another in a component

[mariosanchez]

Hi, I'm facing an issue trying to replace a mesh in a component, but not sure if I'm doing it with the best approach. Also, I'm using an asset manager to load the meshes. Imagine something like change clothes from a 3D character (maybe easy to understand my with this example).

Some context, I have a component that recieves a Babylon mesh perform some actions (like add a material) on this mesh and renders it:

import React from 'react';

function Part({ partMesh, material, smooth }) {
  partMesh.setEnabled(true);

  if (material != null) {
    partMesh.material = material;
  }

  if (smooth) {
    partMesh.forceSharedVertices();
  }

  return <mesh fromInstance={partMesh} />;
}

export default Part;

I assumed that just passing a different mesh as prop it will be updated in the <mesh /> I'm returning, but instead it seems that it's not being changed, I check it myself by geting the refof the <mesh /> and comparing the uniqueId of both input mesh and current mesh from ref. Just set it the first time but it's not changing when I pass another mesh to the component.

Another approach I tried as a workaround is to have two different components (doing the same which feels weird to do it so) and conditionally render them with a ternary expression, and then I was able to get the effect I wanted, "replace" one part with another, but If I change the state twice consecutively and go back to the original state, the mesh disapeared. As I have already checked, the mesh is disposed after hide it with the conditional render.

So I decided to try by cloning the mesh but then I have issues by dinamically change the mesh material.

I have the feeling it could be a easier way to do what I'm trying to achieve, and would want to know I someone can put me in the right direction.

Thanks in advanced!

[brianzinn]

Good question - and I think there is room for improvement on both the implementation and documentation. Firstly the fromInstance is only applied at time on instantiation (only on first render and is ignored afterwards):

<mesh fromInstance={partMesh} />

One way to trick the reconciler would be to use a key:

<mesh key={partMesh.uniqueId} fromInstance={partMesh} />

That would force the reconciler to replace the mesh being tracked by the reconciler and dispose the original one. We could have the reconciler do a basic equality check like object equals instead on the fromInstance property. I think that makes sense from an expected behavior point of view, but the automatic actions would need to reflow (ie: if you attach a material declaratively or the other automatic things that happen like shadow casting and physics imposters, etc).

What do you think about those options?

[mariosanchez]

Hey @brianzinn, thanks for your fast response :)

The key solution, would be a useful workaround, but as a person more used to the React way of thinking, I have to admit that it would not feel very natural to use components this way, is not the behaviour I would expect in a component and would prefer it to be well documented. Is also true that in my case is not such a big deal, since the Part component also abstract this kind of things.

The second option, doing the equality check, seems the expected way to work with React, whether I change a prop the component reacts to this change. But not sure on how the need to reflow would affect the library internals or the use of it in lirabrary users code.

I think the second option fits better in the "React mindset", but if it's a lot of work, maybe documenting the first option could be a step further until there is a chance to develope the second.

Tell me if you think I can help in any way.

Thanks!

[mariosanchez]

Just tried the solution you suggested, and put the uniqueId as key, now it happens that the mesh is disposed after the first replacement and want to get back the original mesh, and remains in this state. It's the same effect I found in the second aproach on mi first message.

As far I could understand it has to do with this function logic https://github.com/brianzinn/react-babylonjs/blob/c46f8e39dd00c80ee3c6a9ea5bea6fe52bc8f927/src/ReactBabylonJSHostConfig.ts#L566

Not sure if there is a way to revert the babylon dispose state, didn't find much info about this, but I think the answer is "no" :_D

So far I could try to represent all posible meshes and control whether they are disabled or not, but would be a workaround.

What do you think about this?

[brianzinn]

The only way currently would be to set the mesh dispose to an empty/new function. You were correct with the code, because that function calls removeChild, which calls dispose: https://github.com/brianzinn/react-babylonjs/blob/c46f8e39dd00c80ee3c6a9ea5bea6fe52bc8f927/src/ReactBabylonJSHostConfig.ts#L104

One kind of hacky way to get around that would be to patch the mesh yourself.

const originalDispose = mesh.dispose; // pointer to function - not calling it
mesh.dispose = (doDispose) => {
  if (doDispose) {
    originalDispose();
  }

Then the reconciler would not do the dispose and when you are ready you can dispose it yourself with:

mesh.dispose(true);

In terms of doing things the React way - DOM elements don't require any cleanup, so it's better to rely on the reconciler to call dispose - I don't know if an escape hatch like "doNotDispose" would be a good thing, but maybe the best solution is that when fromInstance is used that we always skip calling dispose? I think the best way to work is the way you describe and when you change fromInstance that the underlying object is replaced and the declarative children/parents get notified (ie: attach materials, physics, etc.). You could be attaching a texture that is re-used and then we would for sure not want to call dispose.

[mariosanchez]

Ey @brianzinn I've been super bussy so I could not follow this thread earlier.

Thanks for clarifying, now I understand better how this part of the library works.

To be honest, I'm not sure if my case is the very common use case, I could imagen other people looking for similar behaviour. Also I would say yes, for the fromInstance it seems right to replace de underlying object declarativelly, but not sure, so it's okay for me to wait for more people with similar use cases or needs and meanwhile use the workaround you suggested :)

By the way, it worked, and I share my solution here if is usefull for anyone.

Since I don't like to have this behaviour in all my Part components I did a component that wraps it:

import React, { useRef } from 'react';
import Part from './Part';

export default function SwappablePart({ partMesh, ...props }) {
  const currentMeshRef = useRef(null);
  const currentMesh = currentMeshRef?.current?.hostInstance;
  partMesh = patchDispose(partMesh);

  if (currentMesh != null && currentMesh.uniqueId !== partMesh.uniqueId) {
    currentMesh.setEnabled(false);
  }

  return <Part ref={currentMeshRef} partMesh={partMesh} {...props} />;
}

function patchDispose(mesh) {
  const originalDispose = mesh.dispose;

  mesh.dispose = (doDispose) => {
    if (doDispose) {
      originalDispose();
    }
  };

  return mesh;
}

It's okay by the moment, if we have a better solution provided by the library in the future I could easyly migrate.

Thanks!!

[brianzinn]

I’ve got a fix started on v3 branch - going to add a new property called ‘disposeInstanceOnUnmount’ or similar and it will default to false, because the common use case is that the mesh would be reused and then that provides an opt-in mechanism. Does that sound good?

[mariosanchez]

Would work for me. Thanks!

Let me know if you need help in any way.

[alexhoma]

Hi @brianzinn! Has version 3 the disposeInstanceOnUnmount property ready or is there another workaround for this?

PS. Thank you for the v3 :smile:

[brianzinn]

@alexhoma Thanks for the reminder. I was going through the issues before mentioning there was a v3 out, so thanks for the reminder as I definitely want this breaking change in the 3.0 release. I'll add that property, but default it to false...

[brianzinn]

should be fixed in 3.0.3 NPM. I have updated the breaking changes documentation: https://github.com/brianzinn/react-babylonjs/blob/master/docs/breaking-changes-2.x-to-3.0.md#frominstance-declared-objects-to-not-automatically-dispose-but-you-can-opt-in

Please re-open issue if you have any questions/comments. Cheers.

[alexhoma]

Awesome @brianzinn :blush: