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

Added clear coat normal maps #17079

Merged
merged 18 commits into from
Aug 6, 2019
Merged

Added clear coat normal maps #17079

merged 18 commits into from
Aug 6, 2019

Conversation

arobertson0
Copy link
Contributor

In an effort to align more with the Enterprise PBR model (#16977) I've implemented clearcoat normals. This also addresses issue #12867

cc_normals_demo

When a physical material receives a clear coat normal map, it enables independent clear coat normals and calculates them using the same formulas as the normals, but with a different map.

I've had to change the signature of many GLSL functions that implicitly assumed the clear coat normal was identical to the geometry normal.

To get smooth normals, one can set the clearcoat normalmap to anything as then setting the scale to (0, 0).

I haven't yet added object space clearcoat normals because I wanted to reduce the PR size as much as possible. Same goes for node physical materials.

@WestLangley
Copy link
Collaborator

Thanks for this! It's great.

I haven't yet added object space clearcoat normals

I'm not sure we need to worry about adding support for that, but Ben will know.

Same goes for node physical materials

@sunag will need to be aware of this change, unless you wish to take care of it in a later PR.

If someone can contribute a clearcoat normal map that simulates scratches, that would be great. (So would a golf ball normal map for the sphere.)

@WestLangley
Copy link
Collaborator

It would be helpful if you could clean up your formatting. Try https://zz85.github.io/mrdoobapproves/. It may have a bit of trouble parsing your code, but you can work it out.

@sunag
Copy link
Collaborator

sunag commented Jul 24, 2019

Great job. I can add this feature in node material later once that this PR is merged.

@arobertson0
Copy link
Contributor Author

arobertson0 commented Jul 24, 2019

@sunag will need to be aware of this change, unless you wish to take care of it in a later PR.

Great job. I can add this feature in node material later once that this PR is merged.

I've already got work underway for node materials, I wanted to make the PRs as small as I could.

Try https://zz85.github.io/mrdoobapproves/

Can we add this to the contribution guidelines? That's an awesome tool!

@EliasHasle
Copy link
Contributor

EliasHasle commented Jul 26, 2019

(So would a golf ball normal map for the sphere.)

(@WestLangley I presume it would be better for that case to work with a detailed Icosahedron (Polyhedron), making a high-resolution map for a single face. :-) Update: Or something like this, avoiding a map altogether: https://eliashasle.github.io/golf_ball.html (phew!))

@arobertson0
Copy link
Contributor Author

@WestLangley, I'm pretty new at Github, so I was wondering what the next step was after linting.

@WestLangley
Copy link
Collaborator

I expect @mrdoob will want to wait until a few days after the next release -- assuming there will be a release this month.

Personally, I would prefer a better example, but that can be for a subsequent PR.

/ping @bhouston I think you were supportive of this, right?

@arobertson0
Copy link
Contributor Author

Personally, I would prefer a better example, but that can be for a subsequent PR.

I think the golf ball idea is great. It would definitely look better than the face/tentacle maps.
I'm not a 3D modelling guy, do you know where I could grab a model / map for this? I would love to add it to my upcoming Node Material PR.

@mrdoob
Copy link
Owner

mrdoob commented Jul 31, 2019

Unreal's clear coat example is pretty good:

Dual_CC_On

@mrdoob mrdoob added this to the r108 milestone Jul 31, 2019
@bhouston
Copy link
Contributor

/ping @bhouston I think you were supportive of this, right?

Yeah, this looks good. I am very interested in the node material implementation.

Unreal's clear coat example is pretty good:

I believe to achieve the UE4 effect they have anisotropy on the underlying threads + a normal map.

@WestLangley
Copy link
Collaborator

@mrdoob Let's merge this if you approve, as it impacts other concurrent issues.

@mrdoob mrdoob merged commit 1bb818e into mrdoob:dev Aug 6, 2019
@mrdoob
Copy link
Owner

mrdoob commented Aug 6, 2019

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Aug 7, 2019

@arobertson0 @bhouston I've refactored the clear coat code a bit. e3dc226 4df6d7c

Now, if material.clearCoatNormalMap is not defined, the clearcoat surface will be smooth.

Screen Shot 2019-08-07 at 2 26 54 PM

@mrdoob
Copy link
Owner

mrdoob commented Aug 7, 2019

@arobertson0 @bhouston I've also updated the example:
https://raw.githack.com/mrdoob/three.js/dev/examples/webgl_materials_clearcoat_normalmap.html

top left: -
top right: normalMap
bottom left: clearCoatNormalMap
bottom right: clearCoatNormalMap + normalMap

Screen Shot 2019-08-07 at 3 18 50 PM

The light code seems correct to me, but the reflection doesn't... Seems like the reflection is being "disturbed" by the normalmap in the clearcoat layer too?

(Trying to replicate the UE4 effect)

@mrdoob
Copy link
Owner

mrdoob commented Aug 8, 2019

Nevermind, I figured it out! 93668f7

https://raw.githack.com/mrdoob/three.js/dev/examples/webgl_materials_clearcoat_normalmap.html

Screen Shot 2019-08-07 at 6 18 42 PM

🙌

@bhouston
Copy link
Contributor

bhouston commented Aug 8, 2019

A few suggestions:

  • We should have some text that explains what you are looking at, describe each sphere what they are showing in as short as possible form.
  • Looking at the two bottom spheres, it seems buggy because of the sparkles. Is this a bug or the normal map used it just too extreme or the scale wrong?

image

/ping @arobertson0

@arobertson0
Copy link
Contributor Author

Looking at the two bottom spheres, it seems buggy because of the sparkles. Is this a bug or the normal map used it just too extreme or the scale wrong?

I quickly checked, this also happens without clearcoat, on a reflective sphere:

clearCoat: 0.0,
roughness:0.0,
normalMap: clearCoatNormaMap,

reflective

IMO, the effect is "correct". We're seeing specular reflections on microfacets that are aligned with the normal. It looks weird because, due to the high normal scale, the adjacent fragments have a wildly varying normals. So, the highlight doesn't ramp off smoothly.

@mrdoob
Copy link
Owner

mrdoob commented Aug 8, 2019

A few suggestions:

I'll work on improving the example and the bottom spheres later today. I was focusing on the top-right one.

@EliasHasle
Copy link
Contributor

EliasHasle commented Aug 8, 2019

IMO, the effect is "correct". We're seeing specular reflections on microfacets that are aligned with the normal. It looks weird because, due to the high normal scale, the adjacent fragments have a wildly varying normals. So, the highlight doesn't ramp off smoothly.

One will see this exact effect in cold snow with a certain crystal type/size at night, when dimly lit by street lights. The snow looks like a night sky of "stars". When you stand still, the "stars" are static. When you walk, the "stars" disappear and appear "randomly". It is beautiful! (I do not have anything to say on whether the effect is correct in this particular case, though.)

@mrdoob
Copy link
Owner

mrdoob commented Aug 8, 2019

Screen Shot 2019-08-08 at 9 48 12 AM

@arobertson0
Copy link
Contributor Author

arobertson0 commented Aug 8, 2019

Thanks for the great demo @mrdoob. Should I also add the node material demo to webgl_materials_clearcoat_normalmap, or do I put it in webgl_materials_nodes?

/ping @sunag

@mrdoob
Copy link
Owner

mrdoob commented Aug 8, 2019

Better add it to webgl_materials_nodes yes.

@WestLangley
Copy link
Collaborator

Temporary live link:

For easy comparison, my version with no scene lights, no rotating of spheres.

Textures may not be open-source.

WestLangley dev
https://raw.githack.com/WestLangley/three.js/dev/examples/webgl_materials_clearcoat_normalmap.html

three.js dev
https://raw.githack.com/mrdoob/three.js/dev/examples/webgl_materials_clearcoat_normalmap.html

@mrdoob
Copy link
Owner

mrdoob commented Aug 8, 2019

Bottom-right is nice!

@bhouston
Copy link
Contributor

bhouston commented Aug 8, 2019 via email

@mrdoob
Copy link
Owner

mrdoob commented Aug 8, 2019

Should we rename all the properties before it's too late?

clearCoat > clearcoat
clearCoatRoughness > clearcoatRoughness
clearCoatNormalScale > clearcoatNormalScale
clearCoatNormalMap > clearcoatNormalMap

@WestLangley
Copy link
Collaborator

Yes. I think so.

@mrdoob
Copy link
Owner

mrdoob commented Aug 21, 2019

@WestLangley Done! b75dd04

@arobertson0 @sunag @bhouston FYI

@WestLangley
Copy link
Collaborator

Don't forget the legacy getter/setters to prevent breakage.

@mrdoob
Copy link
Owner

mrdoob commented Aug 21, 2019

@WestLangley I was thinking that these properties were so new that getters/setter wouldn't be needed? Was clearcoat used/useful without clearcoatNormalMap?

@WestLangley
Copy link
Collaborator

Was clearcoat used without clearcoatNormalMap?

My personal examples are broken, so I used it...

I added a migration note, but we usually do more than that for user convenience.

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.

6 participants