Geometry.fromBufferGeometry has an issue when reading InterleavedBuffer

[Julianouyang]

Geometry.fromBufferGeometry produces wrong data when working with InterleavedBuffer

Assume we have a bufferGeometry with interleavedBufferAttribute as its attribute type.

There is an issue in geometry.js if we call Geometry.fromBufferGeometry(bufferGeometry).
The current code assumes that the type of attribute is BufferAttribute, but it also can be InterleavedBufferAttribute, and they cannot be read in the same way.

For BufferAttribute, we read attribute like position, normal individually. But for InterleavedBufferAttribute, it returns a typed array that bundles all attributes together. So we need to read the data based on its offset and itemsize.

It might be better to support reading attribute individually in InterleavedBufferAttribute class, but at least we need a quick fix in the geometry method.

If you all think this is a bug and nobody is fixing it right now, I can open a PR. I can also create a jsfiddle if you think it's necessary to demonstrate the wrong data output.

Three.js version

• Dev
• r109
• ...

Browser

• All of them
• Chrome
• Firefox
• Internet Explorer

OS

• All of them
• Windows
• macOS
• Linux
• Android
• iOS

[Mugen87]

Geometry can be considered as deprecated so its unlikely that this will ever be supported. What is your use case for still relying on Geometry?

[Julianouyang]

@Mugen87 I tried to create an EdgesGeometry and it calls Geometry.fromBufferGeometry internally.

[Mugen87]

Oh dear, this is indeed an unfortunate dependency to Geometry. Thanks for pointing this out!

@mrdoob TBH, I was actually not aware of this detail. I guess we have to migrate EdgesGeometry to BufferGeometry in the near future.

[WestLangley]

Geometry.fromBufferGeometry is used internally in multiple places: SubdivisionModifier, SimplifyModifier, EdgesGeometry. The first two are examples, though.

[WestLangley]

I can open a PR.

Sure, why not? I think WireframeGeometry supports interleaved buffers...

[Julianouyang]

No problem, I am working on it.

[Julianouyang]

It's also worth mentioning that a few lines after, where addFace function gets called, don't support THREE.TriangleStripDrawMode of indices.

If we have indices in triangle strip mode, the resulting faces could go wrong...

[WestLangley]

If we have indices in triangle strip mode, the resulting faces could go wrong

Related #17369 (comment).

Also see the discussion in #9869.