zeux / meshoptimizer

Mesh optimization library that makes meshes smaller and faster to render
MIT License
5.49k stars 473 forks source link

A potential performance improvement to rankEdgeCollapses #714

Closed Sluggernot closed 2 months ago

Sluggernot commented 2 months ago

This change passes the tests, locally. I have observed that making these changes reduces the load time of meshes in Godot by ~60 to 75% Tests in Godot were done with several .glb files from official Godot test projects as well as randomly assorted files I had available. I freely admit I don't understand why this would make such a large difference. I originally had a huge set of changes put together in my copy of the Godot source, in this file, attempting to inline large amounts of calculations in the quadricError functions. I found this change entirely by accident. (And originally had it completely wrong, rewriting over the same reference for each iteration of this loop)

Sluggernot commented 2 months ago

Let me know what sort of proof you would like for the performance changes.

zeux commented 2 months ago

I have observed that making these changes reduces the load time of meshes in Godot by ~60 to 75%

Please include detailed profiling methodology - what platform have you built Godot on, what compiler are you using, what compilation options are you using, and how exactly you are measuring the time. I ran this change on gcc/linux using meshoptimizer's demo program (edit and, forgot to mention, I see no difference in performance on the demo program, as should be expected); you can do the same by building it via make or CMake and running demo/main with flags -d path-to-a-large-file.obj. For example:

~/meshoptimizer (perf) $ make config=release dev files=data/buddha.obj
build/release/meshoptimizer -d data/buddha.obj
# data/buddha.obj: 549409 vertices, 1087474 triangles; read in 89.62 msec; indexed in 104.78 msec
Simplify : 1087474 triangles => 217494 triangles (0.01% deviation) in 463.95 msec
SimplifyN: 1087474 triangles => 224268 triangles (0.30% deviation) in 216.43 msec, clusterized in 654.77 msec
Cleroth commented 2 months ago

Op removed a line in his testing. https://i.imgur.com/Rgi475N.png

Sluggernot commented 2 months ago

WOAH! Thanks

Sluggernot commented 2 months ago

I must need some sleep. Collapse doesnt have a distance_error data member...

zeux commented 2 months ago

Ah I see. Godot adds distance_error in a patch (it's important due to an abnormally high normal weighting it is using atm); it makes sense that if that line is removed, the overall performance of the process is different - for example I think that is used to determine ray cast thresholds, and setting this to zero may result in null rays which probably makes ray casting way faster. There's some other impact on the LOD generation flow from this in Godot.

I would not recommend trying to optimize meshopt_simplify without a lot of care. I spent some time analyzing this and wrote a larger comment here https://github.com/godotengine/godot/issues/93587#issuecomment-2196030252.

Sluggernot commented 2 months ago

Once again, I really appreciate your time on this. Thank you very much.