-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GLTFLoader: Don't duplicate vertex buffers when loading triangle fans and strips #27926
Comments
I think it's nice if attribute (= WebGL buffer) can be shared among geometries. I don't really remember but like according to the codes below, currently attributes are not allowed to be shared among geometries, are they? three.js/src/renderers/webgl/WebGLAttributes.js Lines 130 to 144 in 07ec4e8
three.js/src/renderers/webgl/WebGLGeometries.js Lines 9 to 36 in 39305cf
In #17089 we were thinking of shared buffers with different parameters and the change became complex. But, I haven't deeply thought yet, can't just shared attributes (so same parameters) be implemented much easier with reference counter without API change and be good start? Perhaps I guess we don't really want to make an effort for WebGL right now because we want to spend more time for WebGPU (this is just my guess) but small changes would be acceptable? Any thoughts on WebGL shared buffer support? @mrdoob @Mugen87 @donmccurdy I'm willing to work on it and fix this issue if it sounds good. |
I've added some debugging output to glTF Transform, to calculate the effective "vertex count" in a few different ways: gltf-transform inspect oval.gltf --format md
See definitions for each vertex count method. The "upload" vertex count is what we upload to the GPU today, to my understanding. And "upload-optimistic" is what could be uploaded ideally, either by attribute or by interleaved buffer. Big difference on this model – 386,437,472 vertices if we upload attributes per-geometry (current), only 38,924 if we upload attributes individually or per by interleaved buffer. Lot of draw calls either way. :)
Perhaps! I'm not sure how simple this is compared to #17089, but I'm open to it. We'd need to think about what to do with interleaved buffers — both gltfpack and gltf-transform will interleave vertex data by default. I'm not sure how much binding and unbinding buffers in WebGL costs us in performance, but for models that don't reuse attributes in this way (probably more common) we don't want a performance regression there.
I'm fairly optimistic I can reduce the memory cost of optimizing this model in glTF Transform dramatically, if that helps anyone. Tracking this in donmccurdy/glTF-Transform#1315. |
I would like to try shared WebGL buffers (shared attributes) first. Shared Interleaved buffers would be based on it even if we would support shared interleaved buffers with different parameters. |
This turned out to be a very useful sample for optimizing different steps in gltf-transform, so thank you for sharing it! I was able to significantly reduce the time and memory cost of optimizing similar files in gltf-transform ... > gltf-transform optimize ~/Downloads/oval.gltf ~/Desktop/oval+opt.glb
✔ dedup 731ms
✔ instance 1ms
✔ palette 27ms
✔ flatten 1ms
✔ join 1,115ms
✔ weld 23ms
✔ simplify 26ms
✔ resample 0ms
✔ prune 0ms
✔ sparse 9ms
✔ textureCompress 0ms
✔ meshopt 40ms
info: oval.gltf (4.78 MB) → oval+opt.glb (197.54 KB) With that optimization it's reduced to ~30,000 vertices uploaded to the GPU, and 1 draw call. But the optimization time is still, perhaps, slower than you'd want to do at runtime in the browser without a Web Worker. This requires gltf-transform v4, currently in alpha release, which can be installed with I'm definitely 100% supportive of @takahirox's suggestion for supporting better sharing of vertex buffers across geometries. However, I think there's one more parallel thing we could consider adding here — if ( geometry.index.count < geometry.attributes.position.count / 2 ) {
warnOnce( 'THREE.GLTFLoader: One or more meshes draw <50% of their vertices. Compacting.' );
geometry = BufferGeometryUtils.toCompact( geometry );
} The (proposed) implementation of Any preferences on adding that? |
Description
For some GLTF files like the one in this discussion, there are a large number of primitives which share vertex data.
The GLTF loader uses
toTrianglesDrawMode()
to convert triangle fans and triangle strips into regular triangles. The issue is that it clones the geometry, including the entire array buffer:three.js/examples/jsm/utils/BufferGeometryUtils.js
Lines 846 to 852 in e65206d
This means that for inefficient files like
oval.gltf
(see link above), it creates nearly 10K copies of the vertex array buffer. The GLTF file has just 1.3 MB of vertex data, but the browser uses about 9 GB just to load the GLTF.Solution
I noticed that for primitives that use regular triangles, the array buffers are not duplicated, but are shared among all the geometries that use the same buffer in the GLTF file. As a proof of concept, I modified
toTriangleDrawMode
in this way:After cloning the geometry, it resets the new array back to the original, and allows GC to clean up the cloned arrays. Obviously, a better solution would be to avoid cloning the array buffer in the first place.
This does not stop Three.js from uploading the array buffers to the GPU separately, so it's still going to waste the same of memory on the GPU, but at least the RAM usage will be much lower. I believe this issue discusses this problem in more detail: #17089
Alternatives
Naturally, I've tried optimizing the GLTF file using both
gltfpack
andgltf-transform
. And although both of those do work, they end up taking just as much memory to do the optimization. Reducing the number of primitives (and thus, draw calls) would definitely be the ideal solution, but I cannot control the generation of the GLTF files, since they are uploaded by users on a website.Additional context
No response
The text was updated successfully, but these errors were encountered: