Skip to content
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

A potential performance improvement to rankEdgeCollapses #714

Closed
wants to merge 3 commits into from

Conversation

Sluggernot
Copy link

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)

I was working in Godots main branch, trying to improve load times and made a ton of changes to this file. I ultimately found that this one small change accounted for 90% or more of my total improvements.
In godot, loading a ~34 MB glb file went from 18 seconds to 4.9 seconds.
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
Copy link
Author

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

@zeux
Copy link
Owner

zeux commented Jun 28, 2024

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
Copy link

Cleroth commented Jun 28, 2024

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

@Sluggernot
Copy link
Author

WOAH! Thanks

@Sluggernot
Copy link
Author

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

@zeux
Copy link
Owner

zeux commented Jun 28, 2024

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 godotengine/godot#93587 (comment).

@zeux zeux closed this Jun 28, 2024
@Sluggernot
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants