GLTFExporter: Fix export of multi-material meshes.

[Mugen87]

Fixed #16263

[takahirox]

I remember that .uuid has memory consumption problem and we discussed if we should introduce lazy .uuid generation (or other solutions). We haven't concluded whether we should but I think just in case I prefer less using uuid as much as possible.

And cachedData.attributes is also used for geometry.attributes and geometry.morphAttributes. Cache key is attribute (BufferAttribute instance) for them. 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 think even if it's a bit complicated but making utility functions like below may be less problematic. What do you think?

// geometry.attributes/morphAttributes

if ( hasAttributeCache( attribute ) ) {

    result = getAttributeCache( attribute );

} else {

    result = foo;
    setAttributeCache( result, attribute );

}


// geometry.index

if ( hasAttributeCache( attribute, start, count ) ) {

    result = getAttributeCache( attribute, start, count );

} else {

    result = foo;
    setAttributeCache( result, attribute, start, count );

}

function hasAttributeCache( attribute, start, count ) {

    return cachedData.attributes.has( attribute ) &&
        cachedData.attributes.get( attribute ).has( start ) &&
        cachedData.attributes.get( attribute ).get( start ).has( count );

}

function getAttributeCache( attribute, start, count ) {

    return cachedData.attributes.get( attribute ).get( start ).get( count );

}

function setAttributeCache( data, attribute, start, count ) {

    if ( ! cachedData.attributes.has( attribute ) ) {

        cachedData.attributes.set( attribute, new Map() );

    }

    if ( ! cachedData.attributes.get( attribute ).has( start ) ) {

        cachedData.attributes.get( attribute ).set( start, new Map() );

    }

    cachedData.attributes.get( attribute ).get( start ).set( count, data );

}

[takahirox]

var cacheKey = geometry.uuid + ':' + groups[ i ].start + ':' + groups[ i ].count;

Plus, using geometry.uuid for cache key. So even if another geometry.index refers to the same BufferAttribute instance, it's distinguished. This is a regression.

[takahirox]

#16265 (comment)

[Mugen87]

Let's merge this PR for now so the user in the forum has a fixed version of GLTFExporter. Further improvements can be done in a separate PR. It feels weird to copy/paste all of your code so it's better when you make the next PR 😊

[takahirox]

TBH, I don't think we should merge because this PR has a regression even though it solves an issue. If they need a fixed version right now, I can make another PR now.

[takahirox]

Ah, you already merged. OK I'll make PR.

[donmccurdy]

I remember that .uuid has memory consumption problem and we discussed if we should introduce lazy .uuid generation (or other solutions). We haven't concluded whether we should but I think just in case I prefer less using uuid as much as possible.

I don't think I agree with this... export is an expensive operation, and a very reasonable time to use .uuid even if it has overhead. That overhead is small compared to serializing the entire scene. Avoiding UUID within the render loop is reasonable of course.

[takahirox]

My concern is memory consumption can keep big even after exporting. If we introduce lazy uuid generation and if exporter refers objects uuid which are not generated yet, uuids are generated at that time. Memory consumption increases. And the objects don't release their uuids. If the objects are kept using after exporting, memory consumption keeps big.

If we don't have any solutions except for using uuid, it may be ok to use. But if we have another solution I prefer not using uuid as much as possible.

Of course, we don't have lazy uuid generation now. And not really sure whether we will do. So just my preference. Not strong opinion.

[mrdoob]

Thanks!