Decal Normal cannot be updated correctly

[rjhllr]

I can't really wrap my head around this so far, but I'm also not as deep in the whole reconciler topic.

I'm creating decals on click (akin to the official example of the Cat Playground / Docs. To do this, I store the current decalNormal in the state of a function component. Write the hit info into the state:

    const onScenePointerDown = (evt: PointerInfo, scene: BabylonScene) => {
        if (evt.pickInfo?.pickedMesh) {
            setDecalNormal(evt.pickInfo.getNormal(true) ?? new Vector3())
            setDecalPosition(evt.pickInfo.pickedPoint ?? new Vector3())
            setSourceMesh(evt.pickInfo.pickedMesh)
            setHasDecal(true)
        } else {
            console.warn('click: no mesh hit')
        }
    }

Then render the decal:

    {hasDecal && (<decal 
    ref={decalRef} disposeInstanceOnUnmount={true}
    sourceMesh={getSourceMesh}
    name='positioningDecal'
    size={new Vector3(10, 10, 10)}
    position={getDecalPosition}
    normal={getDecalNormal}>
        <standardMaterial name="decalMat" zOffset={-5.7} useObjectSpaceNormalMap={true} >
            <texture url='/static/decal_marker.png' hasAlpha={true} />
        </standardMaterial>
    </decal>)}

This works fine for the first run. For the second click, the rotation of the placed decal mesh does not change, even though the state value behind getDecalNormal changes correctly. I assume the normal prop is used to calculate the needed rotation based on the viewing angle. Normally I'd assume my code being at fault, but when I add the following:

<decal key={getDecalNormal} ref={decalRef} disposeInstanceOnUnmount={true} 

to provoke a component unmount on update everything works as intended and demonstrated in the non-React example linked above.

Sorry for the slightly messy code - pulled it out of a bigger project and slightly reformatted it.

Is this working as intended and I just don't understand some mechanic or does this sound like a bug? If it's indeed a bug I'd be more than happy to further isolate + provide a reproducible example.

[rjhllr]

Reproducible Example (didn't really care about minifying it, removing unuseds and such - just to illustrate the point for now): https://codesandbox.io/s/clever-elion-6rt42m?file=/src/App.js If you remove the slashes in line 96 (forcing the rerender by key), everything works. The broken behavior once again illustrated: Place the first decal by click:

-> Works, you can see the eye contours are followed by the decal texture.

If you now place a second decal, you'll see that the contour + rotation of the decal placed on the eye (first click) is still kept. Only the position updates correctly:

[brianzinn]

It is working as expected and it's definitely something that will be explained in further detail in the new docs. And also I am open to suggestions even breaking changes - I have considered newing up instances when a construction property changes that isn't on the object being reconciled against, but that can lead to other issues. Things like 'box' have a 'size' and other properties are also unchangeable and lead to confusion.

The only props that update for builders are ones that are properties on a mesh, so the "constructor" properties are only applied once. This is intentional, since we don't want to keep re-creating - it would be a lot more intuitive if behind the scenes the constructor props were part of a memoization. You can see there is a lot going on in the factory builder: https://github.com/BabylonJS/Babylon.js/blob/master/packages/dev/core/src/Meshes/Builders/decalBuilder.ts#L30

Whereas the reconciler is a bit dumb and is only able to apply "Mesh" props like position.

When you set the "key" you were, as you know, forcing the mesh to dispose and a new one to be created - which called all that above code. I would suggest using an index on the key.

const [decalKey, setDecalKey] = useState(0);

const onScenePointerDown = (...) => {
  if (...) {
   setDecalKey(key => key + 1); 
  }
}

{(decalKey > 0) &&
  <decal key={decalKey} ...>
     ...
  </decal>
}

It's a bit innefficient as it will reload the texture, but you could always pre-load a texture and assign.

<texture ref={textureRef} .... />

<texture fromInstance={textureRef.current} ... />

Does that make sense? Sorry it's a bit unclear - it can be deduced from essentially separating the "constructor" parameters from the "properties" of underlying Class (or in the case of builders what the builder returns).

[brianzinn]

also, disposeInstanceOnUnmount prop doesn't need to be provided for a mesh - it will default to true. It's there primarily for an opt-out mechanism, if you want it not disposed during reconciler unmount. So, you would want to use it above on the texture, although I would need to double-check, but when you use fromInstance the default behaviour to not dispose I believe, since then it's likely like you are managing your state on your own elsewhere. That's another very important thing to document - these new docs will be great as they link out to code sandbox and can walk through these often confusing parts of this library. Most of these custom props have come through feature requests and different use cases, but never landed properly in documentation. Now finally there is a way to make it clearer with the new docs that hopefully will be out in next month or so.

[brianzinn]

Please re-open if you have more questions. Closing from inactivity.