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: Fix scenario where geometry was not getting smoothed #22261

Merged
merged 4 commits into from
Aug 3, 2021

Conversation

gkjohnson
Copy link
Collaborator

Related issue: --

Description

  • Fixes a scenario where subparts / primitives were direct children of a Model type file meaning they would never get smoothed.
  • Ensure the part name is made available to the parseScope for subobjects so parts are named correctly.
  • Remove some use of class functions in favor of arrow functions.

This will probably be my last LDrawLoader PR for a bit but I think there are still a few things that could be done to improve load times. Specifically I think we could cache the parsed versions of the parts files and clone it when needed rather than the original text to avoid parsing all the same content over and over again. And once that's done you could cache the smoothed normals per parsed file to avoid smoothing some edges over and over which I think would give a nice boost. But that's for another time!

BEFORE AFTER
AT-ST Tubes image image
AT-ST "Neck" image image
AT-ST Rubberband image image

cc @yomboprime

@yomboprime
Copy link
Collaborator

Looks awesome!.

I could dip into the geometry caches but I'm currently in another projects (perhaps later this year)

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

mrdoob commented Aug 3, 2021

Thanks!

@gkjohnson gkjohnson deleted the ldraw-fix-tubes branch August 3, 2021 21:18
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