Fix GLTFExporter bug

[takahirox]

This PR fixes a regression added to GLTFExporter by #16265 (comment). Simpler solution idea is welcome.

[donmccurdy]

What is the bug you're trying to fix? From your earlier comments:

So even if another geometry.index refers to the same BufferAttribute instance, it's distinguished.

Agreed, if geometry1.index === geometry2.index then it would be nice to not duplicate the attribute.

So, I know it's very rare case but, if geometry.attributes/morphAttributes and geometry.index refer to the same BufferAttribute instance, they're distinguished.

I don't agree here – reusing an index as a vertex attribute is too much of an edge case. If we can reduce the +50 lines of code in this change by omitting this case, we should do so.

[takahirox]

What is the bug you're trying to fix? From your earlier comments:

So even if another geometry.index refers to the same BufferAttribute instance, it's distinguished.

Agreed, if geometry1.index === geometry2.index then it would be nice to not duplicate the attribute.

Yes, that one. We didn't duplicate before (in case no multi materials). #16265 has regression.

So, I know it's very rare case but, if geometry.attributes/morphAttributes and geometry.index refer to the same BufferAttribute instance, they're distinguished.

I don't agree here – reusing an index as a vertex attribute is too much of an edge case. If we can reduce >the +50 lines of code in this change by omitting this case, we should do so.

I think I'm ok +50 lines for that. But no strong opinion. If we distinguish index attributes from vertex attributes, maybe adding cacheData.indices or something may be clear.

BTW, if we don't like this +50 lines functions and distinguish index from attributes, how simply we can write index cache? BufferAttribute doesn't have uuid. And we need to take account of group.start/count for cache key.

[takahirox]

BTW, if we don't like this +50 lines functions and distinguish index from attributes, how simply we can write index cache? BufferAttribute doesn't have uuid. And we need to take account of group.start/count for cache key.

A cheap idea is temporal uid.

var uids = new Map();
var uid = 0;

function getUID( object ) {

    if ( ! uids.has( object ) ) {

        uids.set( object, uid ++ );

    }

    return uids.get( object );

}

var key = getUID( geometry.index ) + ':' + start + ':' + count;

if ( cachedData.indices.has( key ) ) {
...

[donmccurdy]

The new version (with a Map and incrementing counter) looks good to me 👍

[mrdoob]

Thanks!