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 slanted normals #22181

Merged
merged 2 commits into from
Jul 26, 2021
Merged

LDrawLoader: Fix slanted normals #22181

merged 2 commits into from
Jul 26, 2021

Conversation

gkjohnson
Copy link
Collaborator

Related issue: --

Description

Fixes skewed normals from redundant quad vertices when generating smooth normals that were particularly noticeable on the studs. Here's a gif of the before and after fix:

ldraw-normal-diff

And screenshots:

BEFORE AFTER
image image

Also semi-related I notice that after 2f09982 the LDraw example page looks very washed out and the lego models don't look bright and vibrant the way I expect them to. Was this intentional? I'm not sure I feel an environment map is the best way to demonstrate the models.

cc @yomboprime

@yomboprime
Copy link
Collaborator

Also semi-related I notice that after 2f09982 the LDraw example page looks very washed out and the lego models don't look bright and vibrant the way I expect them to. Was this intentional? I'm not sure I feel an environment map is the best way to demonstrate the models.

Yes, I also see the colors are like pastel tones. Also it seems that all the edge (contour lines) materials, except the transparent ones, are all gray. I don't know if they are being affected by illumination or the environment map, which should not be the case.

@WestLangley
Copy link
Collaborator

You need tone mapping when using an HDR environment map, which RoomEnvironment aims to replicate.

@gkjohnson
Copy link
Collaborator Author

gkjohnson commented Jul 25, 2021

You need tone mapping when using an HDR environment map, which RoomEnvironment aims to replicate.

I don't think it's just tone mapping. The changes in 2f09982 set the output encoding to sRGBEncoding which means the loaded LDraw material colors are assumed to be Linear which isn't the case. And even so my impression is the models as loaded are designed to look like they're from a lego instruction booklet which are rendered fairly simply. Here's a quick survey of the look of the model with different settings. "1" is what's on dev right now:

lighting tone mapping material color space output encoding screenshot
1 env map none srgb srgb image
2 env map ACES srgb srgb image
3 env map ACES linear srgb image
4 directional + ambient light none linear srgb image
5 directional + ambient light none srgb linear image

I think 3 (environment map, tone mapping, and linear color space shading) is the most "realistic" looking model but 4 (simple directional light lighting, linear color space shading) best approximates the look of a lego model instructions. Unfortunately 4 requires manually converting colors from srgb to linear after loading them so 5 is the simplest approach. I'm happy to help adjust the example to whichever version above is the most desirable in another PR.

@WestLangley
Copy link
Collaborator

I don't think it's just tone mapping.

I did not mean to imply it was. I was just point out a fact. :-)

@WestLangley
Copy link
Collaborator

WestLangley commented Jul 26, 2021

Approach 5 looks good, but it is just not correct to implement lighting in sRGB colorspace. I assume we want to demonstrate best-practices in the examples. Approach 4 does that.

MeshStandardMaterial should be be used with an environment map -- even for plastics. The envMap does not have to be HDR.

A studio environment map may provide more uniform lighting. White light will avoid hue-shifts.

If I remember correctly, these models utilize many materials -- all with different values for environmentIntensity. That seems odd to me.

@mrdoob mrdoob added this to the r131 milestone Jul 26, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jul 26, 2021

I don't think it's just tone mapping. The changes in 2f09982 set the output encoding to sRGBEncoding which means the loaded LDraw material colors are assumed to be Linear which isn't the case.

Shouldn't the loader produce linear colors? What if someone wanted to mix Lego models with GLTF models? 🤔

I think 3 (environment map, tone mapping, and linear color space shading) is the most "realistic" looking model but 4 (simple directional light lighting, linear color space shading) best approximates the look of a lego model instructions.

I think I'm going to try doing a brighter RoomEnvironment and call it LegoEnvironment 🤞

@mrdoob mrdoob merged commit 29ed280 into mrdoob:dev Jul 26, 2021
@mrdoob
Copy link
Owner

mrdoob commented Jul 26, 2021

Thanks!

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.

4 participants