computeBonesUsingShaders on -> off -> on is causing a major bug

Bugs
1

If I do the following

  1. computeBonesUsingShaders = true
  2. computeBonesUsingShaders = false
  3. computeBonesUsingShaders = true

Technically a dev would expect nothing to change right?
But the problem is that it does…

This block of code during initialization is causing the mesh to break if I try to do something extra with it

mesh.computeBonesUsingShaders = !mesh.computeBonesUsingShaders
await sleep(200)
mesh.computeBonesUsingShaders = !mesh.computeBonesUsingShaders
await sleep(200)

Here is the PG https://playground.babylonjs.com/#SYQW69#1014

2

It looks like the first time CPU skinning is used the vertex positions and normals are cached, then each time you set computeBonesUsingShaders to true it resets them from the cache, overwriting the changes that you made to them manually.

I think one way to fix it is to manually set the vertex positions and normals again after you set computeBonesUsingShaders to true, like on lines 55-56 below.

1 Like
3

Thanks Blake, this is helpful :smile:

So it seems that everytime the computeBonesUsingShaders flag changes, we need to set the vertices data again

1 Like
4

Only if you’ve changed the vertex data manually yourself I think, since CPU skinning changes the vertex data each frame and then when you turn off CPU skinning it resets it from cache it looks like…

2 Likes
5

The thing that is odd is that if you toggle computeBonesUsingShaders = true/false again after your PG line, the mesh collapses again
Whereas if you do the same toggle during the very start of the code, the mesh doesn’t seem to be affected

Why is this though? Maybe it’s due to mesh.skeleton = null

6

Yes at the start of the code you haven’t removed the skeleton and set the vertices with the skeleton applied yet, so toggling CPU mode is fine at that point. But then any time later when you set computeBonesUsingShaders to true to stop using CPU skinning, then the vertex positions and normals get reset from the values that were cached before you set the vertices, undoing your changes to them. So then they need to be set again.

1 Like
7

Ok, then how would I ultimately remove the skeleton while keeping the same absolute vertex positions?

Setting mesh.skeleton = null
then, doing a setVerticesData everytime computeBoneUsingShaders changes seems to be a rather ‘hacky’ solution

Is there a more permanent solution to this?

8

Maybe you could set the vertex positions and normals first thing, before you start using CPU skinning the 1st time, so that the cached values will be those that you want and that’s what will be reset to each time you stop using CPU skinning. But maybe in the morning others will have better ways (maybe that cache can be reset manually for instance). :slight_smile:

2 Likes
9

woaa that’s some black magic right there :star_struck:

So that means I just set it the first time, and I don’t have to worry about side effects of computeBoneUsingShaders etc? That’ll be awesome if that’s the case!
Do you know what are the other possible side effects of doing it this way? Bearing in mind using this for gltf exports etc

10

Well it seems you only want those particular vertex values when the skeleton is null so I don’t think my last approach/idea is really a full solution, depending on the needs. I think resetting the cache after you change the values manually would maybe be a good way to go, but IDK how to do that with the public API… Let’s see what solutions others will have thou. :slight_smile:

1 Like
11

Just for example thou, here’s a test where I invalidate the cache by just adding line 46 to your original PG, right after you set the new vertex data. So maybe there’s a public, cleaner way to do below. :slight_smile:

mesh._internalMeshDataInfo._sourcePositions = null;
1 Like
12

Actually, taking this one step further from your PG, if I try to bind the skeleton back
mesh.skeleton = skeleton
The mesh blows up in size

So not a full solution to bind/unbind skeletons yet
I’m wondering if there’s a better way to do all this :thinking:

PG https://playground.babylonjs.com/#SYQW69#1020

13

Hmm, I think the new issue there is that since you brought the skeleton back you need the original vertex positions and normals back too. Here I set them back to their original values and wiped the cash again after setting the skeleton back and seems to be working again… LOL it is getting harry thou. :wink:

1 Like
14

Yea I guess ‘wiping’ the cache seems to have done the trick ; )
And certainly it’s getting very hairy lol

Wondering if @Deltakosh may have ideas on how to improve the API?

1 Like
15 
Uh-oh, the cycle doesn't work after unbind + bind + unbind + bind
PG: https://playground.babylonjs.com/#SYQW69#1027

EDIT: my bad, didn’t know the original verts data in setVerticesData was mutable
PG: https://playground.babylonjs.com/#SYQW69#1030

16

Yeah I agree setting up a new skeleton should reset the internal cache.

Please create an issue for me on the repo and I will make sure you do not need to call the _internalMeshDataInfo when reseting the skeleton

2 Likes
17

I think in the Mesh class’s computeBonesUsingShaders setter function, when it’s set to true the cache should be reset too. That way after turning CPU skinning on and then off again, you could afterwards change the vertex positions and normals, and then turn it on and off again and it would reset to your new values. Otherwise they’ll be overwritten without resetting the cache so that the new values can be cached and reset to…

EDIT: oops I meant when setting to true to reset the cache…

18

Here you go _internalMeshDataInfo not being cleared during skeleton change · Issue #12364 · BabylonJS/Babylon.js · GitHub
Also added Blake’s last comment in the github issue

2 Likes
19

Okay here’s a PR for fixing the caching issue that fixes the PG’s in the thread that needed to reset it manually. :slight_smile:

3 Likes
20

Merged! thanks a lot

2 Likes