three-to-ammo
three-to-ammo copied to clipboard
Handle arrays of vertices implemented as interleaved buffer attributes
This fixes creating collision shapes for indexed models.
Before this change, as computing bounding box and sphere was assuming vertices to be always 3 by 3 in the array, one could either incur in out of bound errors or have their shape incorrectly calculated.
I have double checked: indeed for certain indexed glb models we cannot assume that the vertices for a particular geometries will be divisible by 3, which the code assumes right now.
This seems to be the case for full-body avatar models models generated by e.g. https://readyplayer.me/avatar.
The root cause seems the use of InterleavedGeometries on the model. According to https://threejs.org/docs/#api/en/core/InterleavedBuffer, this means that the array we are extracting will not only contain information for the specific attribute (e.g. position), but potentially data from multiple attributes.
By using the index, we ensure the data we are extracting is exclusively relevant to the shape of the item.
This may still not be sufficient... In fact, to obtain the position from an interleaved buffered attribute one should also include stride and offset, see https://github.com/mrdoob/three.js/blob/dev/src/core/InterleavedBufferAttribute.js#L129
In theory, computing bounding box and sphere could be completely delegated to the THREEJS api. In many cases this comes already pre-computed and an attribute of the geometry itself. One further change could be to provide an optional bounding box to the apis that are supposed to be used in the web worker...
I have reworked and retitled the pull request: in fact the problem was dealing with arrays of vertices implemented via interleaved buffer attributes.
My prosed solution comes at a cost of some data duplication, as for this kind of arrays I perform a translation into plain arrays. The alternative would be to push the interleaving logics down into the rest of the api, e.g. passing offset and stride together with the shared array and rework all of the computations involving vertices. This approach is less invasive and touches only the "entry point" api.
I have improved my change so that only one array is used.
All the best