GLTFLoader - De-duplicate BufferGeometry

[mattdesl]

This feature adds simple BufferGeometry caching to GLTFParser, based on the primitives object (looking at attributes and indices to see if all keys line up).

If GLTF is exported or post-processed to de-duplicate matching geometries, you might end up with a meshes definition like this:

  ...
  "meshes": [
    {
        "name" : "Cube.001", 
        "primitives" : [
            {
                "attributes" : {
                    "NORMAL" : 2, 
                    "POSITION" : 1
                }, 
                "indices" : 0,
                "material": 1
            }
        ]
    }, 
    {
        "name" : "Cube", 
        "primitives" : [
            {
                "attributes" : {
                    "NORMAL" : 2, 
                    "POSITION" : 1
                }, 
                "indices" : 0,
                "material": 0
            }
        ]
    }
  ]
  ...

Both of these cubes have different materials, but point to the same index & vertex data. With this PR, because all the attributes/indices line up between the two primitives, they will end up using a single BufferGeometry. In large scenes with a lot of duplicated geometry (e.g. a city with tons of repeated buildings and trees of different scales), you can end up with huge loading/rendering benefits because you are dramatically cutting down the total geometries loaded onto the GPU.

This PR is really only useful if you post-process your GLTF script — I have a node script below, but perhaps one day this sort of feature will end up in Blender's exporter, or in more standard tools like gltf-pipeline.

GLTF de-duplication Node.js script

By default, I've enabled this feature, but the user can turn it off by passing an options object into the second argument of GLTFLoader constructor. I am not sure whether you would prefer it on or off by default.

[donmccurdy]

This looks great to me, thanks. I'm happy with having this on by default, and frankly not even sure the option of turning it off is necessary. Is there a complete sample model you could share? Just as an attachment in a PR comment is fine.

[takahirox]

Probably, if you check if ( keysA.length !== keysB.length ) return false; you could remove for ( i = 0; i < keysB.length; i++ ) { ... }?

[mattdesl]

I think both would be needed, in case one object has extra attributes that don't appear in the other. In that case they should be treated as two separate BufferGeometries.

[takahirox]

in case one object has extra attributes that don't appear in the other.

I think keysA.length doesn't equal to keysB.length in that case.

[mattdesl]

Ah yes , I misread your comment. 👍

[takahirox]

I vote for default (and always on).

[takahirox]

Can we use hash, rather than array, for the faster search?

For example, key can be like this (it can be more optimized)

var key = '';
key += 'indices_' + a.indices;

var attribA = a.attributes || {};
var keysA = Object.keys( attribA );

for ( var i = 0; i < keysA.length; i++ ) {

    var k = keysA[ i ];
    key += '_' + k + '_' + keysA[ k ];

}

Also, getCachedGeometry code would be simpler, just return cache[ key ] || null;.

[mattdesl]

The hash would also have to take into account the sorting of the keys. I'm not sure the code is really much simpler and I can't imagine the performance differences would be significant. But I can explore that if you want.

Here's a test case, a bunch of cubes that get de-duplicated down to a single one.

Link to ZIP of blend and GLTF files

Note: When turning particles into meshes, Blender is smart enough to use the same Object Data for all of them, and de-duplication isn't necessary after exporting GLTF. I had to force them all as unique geometries in this test. However, I've run into other situations where deduplication is important—certain workflows and editing approaches (e.g. using Duplicate Object, or exporting from Maya) can lead to a lot of redundant geometry.

[takahirox]

I still think hash is better than array because that each geometry searches in array isn't scalable against distinct geometries#, O(N^2). (I assume this option is always on. And yeah sorting would be sort of problem for performance tho...).

But such optimization could have done later. Array first would be ok for now.

[mrdoob]

This looks great to me, thanks. I'm happy with having this on by default, and frankly not even sure the option of turning it off is necessary. Is there a complete sample model you could share? Just as an attachment in a PR comment is fine.

Agreed. No need to have an options parameter that way 👍

[mattdesl]

Ok, thanks for the feedback everybody. I've enabled this by default (no need for options anymore) and cleaned up the isPrimitiveEqual function.

[mrdoob]

Ops! Sorry, seems like another PR caused some conflicts. Do you mind resolving them?

[mattdesl]

Ok, I've merged with latest dev and I'm using if/else now instead of early breaking/continuing the loop.

[mrdoob]

Thanks @mattdesl!

@donmccurdy looks good then?

[donmccurdy]

This looks good to me. 🙂

For my own curiosity (and this doesn't need to block merging) is there a difference between reusing an entire BufferGeometry vs using multiple geometries with the same BufferAttributes?

[mrdoob]

Hmm... yes. Currently, I think the renderer avoids reuploading buffer attributes if the geometry and material are the same as the previous object rendered.

[mrdoob]

Thanks!

[takahirox]

Let me share the evaluation about performance dependency on distinct primitives#. (distinct primitives means the ones that can't be reused because of different attributes)

The conclusion is it doesn't matter in general.
But it can be performance issue where distinct primitive# is very very huge (>2000).
So this impl is ok so far, but it'd be better to optimize some day just in case.

#primitives	w/o this change [ms] (*1)	w/ this change [ms] (*2)
100	50	50
500	120	130
1,000	250	250
2,000	350	450
3,000	500	700
4,000	700	1,100
5,000	1,000	1,700
6,000	1,300	2,300
7,000	1,800	3,200
8,000	2,100	4,300
9,000	2,500	5,200
10,000	3,200	7,000

*1, *2: Elapsed time of GLTFLoader.load. gltf file is based on many-cubes-optimized.gltf @mattdesl posted

*1: To simulate 'w/o this change', I added return null; here L1214

three.js/examples/js/loaders/GLTFLoader.js

Lines 1213 to 1215 in 8732d13

	function getCachedGeometry ( cache, newPrimitive ) {
	for ( var i = 0; i < cache.length; i++ ) {

*2: To simulate 'w/ this change' with distinct primitives, I added return false; here L1192

three.js/examples/js/loaders/GLTFLoader.js

Lines 1191 to 1193 in 8732d13

	var keysB = Object.keys(attribB);
	if ( keysA.length !== keysB.length ) {

[takahirox]

The complexity of the part I mentioned above is O(n^2 * m) where n is distinct pritmitives# and m is keys# in an attributes while the one of hash with sorting keys is O(n * m * logm). So probably hash key approach would be better in terms of scalability, I haven't evaluated yet tho.

[donmccurdy]

Nice writeup, thanks @takahirox!

But it can be performance issue where distinct primitive# is very very huge (>2000).

The other condition is that the reused primitive be simple; for complex primitives this might skew the other way. And the loading time is a secondary concern to runtime performance here, where this PR does improve things nicely.

So probably hash key approach would be better in terms of scalability, I haven't evaluated yet tho.

Open to trying other versions of isPrimitiveEqual in another PR if you'd like to have a go at it. 🙂

[takahirox]

Yup I will but feeling like trying to find better solutions because I know that sorting idea isn't the best solution.

[takahirox]

I'm thinking of updating glTF spec (in next version) to allow shared primitives among meshes rather than trying algorithm optimization.

KhronosGroup/glTF#1250 would be a good thread?

[donmccurdy]

Worth discussing, maybe in KhronosGroup/glTF#1249 would be better 👍

[takahirox]

Oops, I've just posted a wrong thread by my misoperation. Thanks for the right thread.

[takahirox]

I just realized that more precisely we should also check targets, extensions, and extras to know if geometry is reusable. Checking logic will be heavy and messy. So I wanna further encourage glTF spec update to enable reusable geometry(primitive) specification.