GLTFExporter generates unnecessary attributes

[cx20]

Description of the problem

I tried exporting the mesh using the latest version of GLTFExporter.
However, the size of the output model is larger than I thought.
The size when exporting scene.gltf with the following sample is 928 KB.

three.js + GLTFExporter result:

In this model, vertex colors are not used, but it seems that attributes of white vertex colors are generated in the output glTF file. Perhaps by excluding unused attributes I think that the size can be made even smaller.

Three.js version

• Dev

Browser

• Chrome

OS

• Windows

Hardware Requirements (graphics card, VR Device, ...)

ThinkPad X260 + Intel HD Graphics 520

Modified: Link of GLTFExporter was GLTFLoader, it was fixed.

[looeee]

Make sure to export as binary for smaller size. It's 693kb with the binary option enabled.

In this model, vertex colors are not used, but it seems that attributes of white vertex colors are generated in the output glTF file

This is something I've observed as well. It would be useful to avoid creating unused attributes.

[cx20]

Make sure to export as binary for smaller size. It's 693kb with the binary option enabled.

It is exactly the case that the size decreases by adding binary option. However, we can further reduce the size by optimizing the output attributes.

I noticed the following comment in GLTFExporter.js.

three.js/examples/js/exporters/GLTFExporter.js

Lines 1095 to 1097 in 02b5ba0

	// @QUESTION Detect if .vertexColors = THREE.VertexColors?
	// For every attribute create an accessor
	for ( var attributeName in geometry.attributes ) {

Perhaps by adding conditions here, I think that it can be solved.

[takahirox]

Can you try to replace CubeGeometry with BoxBufferGeometry in the example? I think color attribute is unnecessarily added in BufferGeometry#fromGeometry in the exporter. Geometry will be deprecated sooner or later and we recommend using BufferGeometry instead.

[cx20]

@takahirox Thank you for the advice.
I would like to try replacing CubeGeometry with BoxBufferGeometry.
However, Simply replacing it, the model was not displayed.
http://jsdo.it/cx20/ULNB
It is probably because THREE.BufferGeometry.merge is not functioning properly. I do not understand its usage properly, so I need to investigate.

[donmccurdy]

GLTFExporter has to convert from Geometry to BufferGeometry to get binary data suited for export, and currently BufferGeometry.fromGeometry (1) adds empty vertex colors, and (2) significantly increases the number of vertices.

As @takahirox suggests it would be best to use BufferGeometry instead. To merge buffer geometries, use BufferGeometryUtils.mergeBufferGeometries().

Possible changes:

• Fix conversion to not add vertex colors.
• Change GLTFExporter to reject Geometry with a clear warning, rather than the more subtle issue of writing an inefficient file.
• Rename BufferGeometry.merge(), and/or clarify docs. I think people expect it to append like Geometry did, when it overwrites vertices instead.

EDIT: ^List above is out of date. Updated version below.

I’m not sure the duplicate vertices can be fixed automatically, but the new mergeVertices function can be used to clean that up manually:
#14116

[fernandojsg]

I'd go for the first option, fixing the vertex colors and I'd keep a warning when using Geometry instead of BufferGeometry just for the sake of compatibility with old code and let the exporter just works even if the file is bigger or you are duplicating vertices or so.

[takahirox]

Fix conversion to not add vertex colors.

+1 but I'm no longer familiar with Geometry so question, how do we detect geometry doesn't has vertex color? Geometry has faces(Face3 array), and Face3 has vertexColors and color which corresponds to color attribute in BufferGeometry. Do we regard as vertex color isn't set if all faces' vertexColors in Geometry are empty and color's rgb are all 1? (I'm not really sure if I understand Geometry and Face3 correctly. Sorry if I'm missing something.)

Change GLTFExporter to reject Geometry with a clear warning, rather than the more subtle issue of writing an inefficient file.

Warning sounds good. Personally I also like rejecting because probably Geometry will be deprecated. But it may be too early, as Fernando mentioned we don't need to break compatibility now.

Rename BufferGeometry.merge(), and/or clarify docs. I think people expect it to append like Geometry did, when it overwrites vertices instead.

I think so too. The current behavior seems like copying for me.

BTW, noticed that BufferAttribute.count isn't updated in BufferGeometry.merge() although new attributes array can be bigger. I think it's a bug, correct? Or is it intentional?

[donmccurdy]

Do we regard as vertex color isn't set if all faces' vertexColors in Geometry are empty and color's rgb are all 1?

I don't necessarily know that this bug is easy or worthwhile to fix; I haven't looked into it. There may be a reason the vertex colors are added based on the Face3 representation as you say. In a PR for a converter (#15552) I did something like this, which seemed to work:

var hasVertexColors = geometry.colors.length > 0;
geometry = new THREE.BufferGeometry().fromGeometry( geometry );
if ( ! hasVertexColors ) geometry.removeAttribute( 'color' );

BTW, noticed that BufferAttribute.count isn't updated in BufferGeometry.merge() although new attributes array can be bigger.

The count will not change when you merge - the attributes array is not resized, just overwritten (see code).

[takahirox]

According to the doc, Geometry.colors is for Points and Lines. Mesh uses Face3.vertexColors. So I think checking only geometry.colors isn't good enough, but also we need to check .faces.

https://threejs.org/docs/#api/en/core/Geometry.colors

The count will not change when you merge - the attributes array is not resized, just overwritten (see code).

Ah, BufferAttrubute.array is typed array so it won't be resized. (For example even I set array[array.length] = value array won't be bigger). But I think better to have clearer loop ending condition. I'll make a PR. Update: made PR #15827.

[cx20]

@takahirox I was able to create a sample using BoxBufferGeometry instead of CubeGeometry.
http://jsdo.it/cx20/EB4w

Test Case	glTF format	glb format
CubeGeometry	928 KB	694 KB
BoxBufferGeometry	502 KB	370 KB

I confirmed that color attributes will not be generated using BufferGeometry.
However, the exported file also appears to output attributes other than POSITION.
If possible, I think it would be nice to have an option to remove attributes other than POSITION at export.

"attributes": {
    "POSITION": 8,
    "NORMAL": 9,
    "TEXCOORD_0": 10
},

[takahirox]

If you don't need them, can't you use geometry.removeAttribute( 'normal' ).removeAttribute( 'uv' );?

https://threejs.org/docs/#api/en/core/BufferGeometry.removeAttribute

[cx20]

@takahirox Thanks! I was able to use your suggestion method.
The model will no longer be displayed on the screen, but the exported glTF file can be made even smaller.

Test Case	glTF format	glb format
CubeGeometry	928 KB	694 KB
BoxBufferGeometry	502 KB	370 KB
BoxBufferGeometry + removeAttribute	221 KB	160 KB

The output glTF file can be displayed with no error in glTF Viewer, so I think that it is a practical range.

[cx20]

BTW, I tried to Draco compression using gltf-pipeline for reference.
It was able to shrink significantly compared with the initial size of 928 KB.
Although it is a future feature proposal, Draco compression feature is also desired for glTF Exporter.

Test Case	glTF	glb	glTF + Draco	glb + Draco
CubeGeometry	928 KB	694 KB	928 KB	694 KB
BoxBufferGeometry	502 KB	370 KB	29 KB	15 KB
BoxBufferGeometry + removeAttribute	221 KB	160 KB	19 KB	8 KB

In the model exported from CubeGeometry somehow Draco compression could not be done.
I think this is a problem of gltf-pipeline . Should I report the issue to gltf-pipeline?

[donmccurdy]

In the model exported from CubeGeometry somehow Draco compression could not be done.
I think this is a problem of gltf-pipeline . Should I report the issue to gltf-pipeline?

it requires indices, see CesiumGS/gltf-pipeline#420. Using BufferGeometryUtils.mergeVertices() before exporting will reduce the number of vertices and create indices for Draco, so I'd recommend that generally.