Crash when reading vertex data from a Geometry that has been updated in-place with less vertex data

Bugs
1

Repro: Babylon.js Playground

There seems to be an issue reading vertex data from a Geometry if it has been updated at any point with fewer vertices than any previous vertex data.
When querying for vertex data there is a constructor call to Float32Array which tries to fetch a view into the underlying data buffer with the same length of the original vertex data’s size. However, our newly updated vertex data has a smaller size than our original, causing an error to be thrown from the Float32Array constructor.

The (bad) size that we’re trying to read is provided by the function caller to VertexBuffer.getFloatData, in this case it is Geometry._totalVertices.

The _totalVertices field is automatically set when calling Geometry.setVerticesBuffer with the VertexData.PositionKind, which is in turn called by Geometry.setVerticesData. However, this value is never updated when Vertex Data is updated through Geometry.updateVerticesData. I guess one solution to this would be to update the field in the Geometry.updateVerticesData function.

You could also argue that what the Geometry class is asking of the VertexBuffer is valid, as its actual GPU buffer still has the capacity of the original vertex data size (or the largest in the lifetime of the VertexBuffer if you update multiple times). But it doesn’t work like this currently either, as the CPU-side buffer of the data is set to the user-given array when updating the VertexBuffer which is smaller in this repro-case.

Unsure which way to fix is best!

Unfortunately I don’t have the time to set aside for a PR at the moment as I don’t have Babylon running locally currently, so any assistance would be greatly appreciated!

Thanks,
-Tore

2

Checking!

3

Ok this is expected. You cannot update a vertex buffer with less data. The vertex size is locked and readonly.

So this is expected. Rendering still work as the indices keep the GPU data read in check but everything else will fail

4

To extend on that, we assume the vertex buffer stays at the same size. I’m not super sure if I want to change such a fundamental case so close to our 8.0 release.

I’ll give it a shot and if it ends up being controllable, I’d apply the fix and keep you posted

5

ok it SEEMS to be fine
I’ll keep an eye on the PR to see how the tests react

Fix Geometry update with smaller buffer by deltakosh · Pull Request #16212 · BabylonJS/Babylon.js

6

If this is a bit of a risky change, perhaps it could also be fixed by allowing the user to set the _totalVertices field? That is possible today through the setVerticesData function in Geometry, but there is no such parameter for the updateVerticesData.

ninja edit:
Locally in our project I was able to get it working by simply setting the _totalVertices field through a ts-ignore. So for us a parameter, like in the setVerticesData function, or a separate function to set the value, is probably enough at least for our issue.

7

I feel better right now. I feel this is the right fix :smiley:

1 Like