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

LDrawLoader: Improve smooth normal generation performance a bit #22228

Merged
merged 6 commits into from
Aug 2, 2021

Conversation

gkjohnson
Copy link
Collaborator

@gkjohnson gkjohnson commented Jul 30, 2021

Related issue: --

Description

Improves the generation time of smooth normals a bit by doing the following:

  • Avoid using Object.keys to get a single element from an object and instead use an immediately terminating for loop.
  • Cache index for half edge so we don't have to iterate over the other triangle edges to find a match.
  • Convert element vertex / normal storage to use arrays rather than dictionary keys.

With these changes the time to generate smooth normals goes from ~18.25 seconds to ~16 seconds when loading the AT-ST model. Nothing earth shattering but it is an improvement. I also switched to using typed arrays when generating geometry which saves another ~0.5 seconds on the AT-ST total processing time.

Next up I'm planning to bookkeep quads as 4 vertices so we can cut down on trying the smooth the plane diagonal as well as some other performance improvements. I also want to improve the ergonomics of the loader specifically relating to loading models with the LDraw parts library.

cc @yomboprime

@mrdoob mrdoob merged commit efd6466 into mrdoob:dev Aug 2, 2021
@mrdoob mrdoob added this to the r132 milestone Aug 2, 2021
@mrdoob
Copy link
Owner

mrdoob commented Aug 2, 2021

Thanks!

@gkjohnson gkjohnson deleted the ldraw-normal-performance branch August 2, 2021 15:28
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.

2 participants