Collada Loader does not set BufferGeometry.index

[gkjohnson]

The ColladaLoader doesn't ever set the triangle indices of a BufferGeometry, even though they're provided by the Collada format, causing the resultant geometry to be less memory efficient than it could be.

Here is where the attribute arrays are expanded.

Possible Solution

I imagine the reason this is being done is because the the format can describe 3 or 4 (or n?) sided polygons. However, I think a more efficient approach might be to expand the quads in the index array and leave the attribute buffers provided as they are in the file. It could happen in this this function, for example.

Thoughts? I'm less familiar with this file, so there may be a slew of other reasons why this is here that I'm unaware of. I don't know if joint weights effects would be affected, for example.

[Mugen87]

or n?

Yes, that's possible.

Thoughts?

The loader always produces a non-indexed geometry. But I don't know the reason for this design decision. Maybe simplicity? 😇

[mrdoob]

Yeah... I think it's the same as with the OBJLoader. The format supports multiple uvs and colors per index and WebGL does not.

[mrdoob]

Actually, it may not be the case. But it sure makes things much simpler and maintainable.

[gkjohnson]

The format supports multiple uvs and colors per index and WebGL does not.

Actually, it may not be the case.

OBJ seems to, but I wasn't under the impression that Collada did. Maybe I'm wrong?

it sure makes things much simpler and maintainable.

I definitely agree, but wouldn't you be able to expand the face indices here rather than expanding the attribute arrays without sacrificing maintainability and achieve the denser representation?

My only hesitation is whether or not joint and animation information would be affected, in which case the change might not be worth it. I might take a crack at it at some point and see if it works out.

Are example pages typically how these loader changes are tested?

Thanks!

[Mugen87]

Are example pages typically how these loader changes are tested?

Yes. We do not have unit test for these loaders.