Cannot merge meshes out of a MeshWriter as of alpha.61

Hi,

I believe I’ve found a bug. I used to be able to merge meshes out of a MeshWriter. But it now fails with “Positions are required” error. Having played with GLTF assets, I’m familiar with this error :sweat_smile:, however this now happens on code that used to work.

I’ve tested many versions and can confirm it worked up until alpha.60 and stopped working from alpha.61. Hopefully that’s useful info.

Repro PG: https://playground.babylonjs.com/#PL752W#647

Happy to help if there is anything more I can do.
Cheers.

Cc @ryantrem as I suspect it could come from this PR: Add a simple coroutine implementation and use it to make an async version of MergeMeshes. by ryantrem · Pull Request #11452 · BabylonJS/Babylon.js · GitHub.

I had a look and it boils down to this commit : Optimize updates of SPS attributes · BabylonJS/Babylon.js@2a4675c · GitHub

Basically in SPS used in the mesh writer there is a ultra heavy focus on perf. UpateDirectly is not keeping track of the original data hence the change. The created mesh are not meant to be mergeable.

Your merge function is not useful in the pg as there is only one mesh inside.

I am inclined to keep the current behavior but would like to ensure there is not more you are doing where it would break your code ???

Thanks for the reply @sebavan. Yes the PG is a basic repro which makes no sense, it’s only to get the error happening. In my app, I have multiple separate texts that are positioned separately and merged.

Is there an option to opt in the new behaviour? Or at least make the old one opt-in? This seems like I might not be the only person having a breaking change.

yup lets check what @Deltakosh is thinking here, I could add a flag to also allow the previous behavior ?

Yes a flag could be the good solution

2 Likes

I had another idea and a fix will be in the next nightly

1 Like

Fix MergeMesh on MeshWriter by sebavan · Pull Request #11871 · BabylonJS/Babylon.js · GitHub should be in the next nightly

2 Likes

Cool @sebavan. Unfortunately I’m afk the whole day and might not be able to test this until next week. Hope that’s ok and thanks for the work :).

1 Like

Hi, I’m trying to test this now. Could you push a new version to npm? Or what’s an alternative to test the latest unpublished version in my app easily? Thanks in advance.

@RaananW will publish a new version soon :slight_smile:

1 Like

Hi @sebavan, I just wanted to confirm that this is fixed now (tested beta.7). Thanks for the last minute strike of genius :wink: .

2 Likes