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: Fail gracefully if an object could not be loaded, improve file compatibility #22957

Merged
merged 1 commit into from
Dec 5, 2021

Conversation

gkjohnson
Copy link
Collaborator

@gkjohnson gkjohnson commented Dec 4, 2021

cc @yomboprime

Description

  • Changes "parse scope" defaults to use values that accommodate real work demo files
    • Change parse scope to default empty arrays for "faces", "lines" and "conditionalLines" since not all internal file definitions include an "!LDRAW_ORG" command where they were intialized.
    • Change default parse scope type of "type" to "Model" since not all internal file definitions include an "!LDRAW_ORG" command.
  • Fail gracefully if a file could not be loaded instead of trying to finalize an "undefined" object.

Tested with the "Scorpion Temple" file here.

image

Some other notes for a future PR

  • There's some weird smoothing on the scorpion claw that I'll try to track down at some point. Maybe smoothing is happening before all part edges are included?

  • For some reason these pieces that are supposed to be a yellow color are rendering as black. Perhaps some metallic material settings need tweaking?

… part defaults for faster loading & more robust loading
@gkjohnson
Copy link
Collaborator Author

gkjohnson commented Dec 4, 2021

For some reason these pieces that are supposed to be a yellow color are rendering as black. Perhaps some metallic material settings need tweaking?

It seems the material currently being set is a Phong Material because they're marked as "pearlescent" but for some reason the environment map is not being applied properly -- is there a bug with phong at the moment?

Here's what it looks like when using MeshStandard instead of MeshPhong material:

@yomboprime
Copy link
Collaborator

is there a bug with phong at the moment?

I remember there was some change about Phong's envmap that I thought it would affect this, but don't remember the change or where was it. :-/

@mrdoob
Copy link
Owner

mrdoob commented Dec 5, 2021

It seems the material currently being set is a Phong Material because they're marked as "pearlescent" but for some reason the environment map is not being applied properly -- is there a bug with phong at the moment?

Phong material doesn't support IBL. Standard can't be used for these pieces too?

@yomboprime
Copy link
Collaborator

Phong material doesn't support IBL. Standard can't be used for these pieces too?

Yes, it can be used.

@gkjohnson
Copy link
Collaborator Author

My impression from the code was that phong was being used so the specular could be set to a complementary color in order to model something like SSS or iridescence for "pearlescent" materials. I think a glossy standard material would look just fine too, though. Happy to handle that in another PR when I get a chance.

@mrdoob
Copy link
Owner

mrdoob commented Dec 5, 2021

Happy to handle that in another PR when I get a chance.

Sounds good!

@mrdoob mrdoob added this to the r136 milestone Dec 5, 2021
@mrdoob mrdoob merged commit 2a3e413 into mrdoob:dev Dec 5, 2021
@mrdoob
Copy link
Owner

mrdoob commented Dec 5, 2021

Thanks!

@gkjohnson gkjohnson deleted the ldraw-graceful-failure branch December 6, 2021 16:29
@gkjohnson
Copy link
Collaborator Author

Regarding the smoothing issue I don't think it can be fixed (easily, at least). The smoothing algorithm works by looking at the vertices of the edges to see if they line up with the polygon vertices and if they do then that edge is determined to be "sharp" or unmerged. However in the scorpion claw piece the surface coloring is created from multiple meshes which breaks up the polygon edge -- and that isn't mimicked in the edge line geometry so there is no edge with matching vertices found in those cases 🤷 :

Addressing this would require more complex and more intensive searching to see if the polygon edge is a subset of one of these edge lines. Probably not worth it 😁

@gkjohnson
Copy link
Collaborator Author

Oo actually a relatively simple and fast(er) way to check this rather than brute force would to be to index all the edge lines by normalized direction, store the line vertices as projected distances, and then for each polygon edge iterate over all the parallel edges and see if the face edge falls entirely within any of the lines.

It would be a bit slower but I'd have to check by how much and it would look better in these cases 🤔. It would depend on how commonly there are a lot of parallel lines in a part. And worse case we could just run this approach when a piece is seen to have multiple materials on it. Or a face is sharing an edge with another face that has a different material.

Anyway -- notes for future self.

@yomboprime
Copy link
Collaborator

yomboprime commented Dec 18, 2021

A thought: There will be a lot of lines parallel to the main axes (but i guess probably most of them will be sharp)

@gkjohnson
Copy link
Collaborator Author

At the level of the whole model there will be a lot of parallel lines on the main axes but normals are smoothed per piece now to improve performance so at that level I don't think there should be too many parallel edges (I'm hoping at least 😁). And it occurs to me that it would actually be best to index the lines by ray (a projected origin point and direction) meaning redundant edges would now lie on the same line rather than just parallel. Still a few things to think through.

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