Skip to content

Tweak clearcoat wording & formatting a bit. #1776

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

Merged
merged 3 commits into from
Mar 11, 2020
Merged

Tweak clearcoat wording & formatting a bit. #1776

merged 3 commits into from
Mar 11, 2020

Conversation

emackey
Copy link
Member

@emackey emackey commented Mar 3, 2020

Fixed a word wrap issue, and tried to clarify the essence of what prompted the recent formula change.

@emackey emackey requested a review from UX3D-nopper March 3, 2020 20:16
@UX3D-nopper
Copy link
Contributor

The resulting f_clearcoat color is expected to have a Fresnel term blended into it as a result of the above. is for me just confusing/obsolete. We are mentioning F, but not G or D or the dot products either.

@emackey
Copy link
Member Author

emackey commented Mar 4, 2020

Is there some way we can reword this just to convey why we changed the formula?

This extension uses a different formula than Substance Painter, for example, and yet the results look very, very similar, nearly pixel-perfect similar in certain situations. In SP clearcoat calls their pbrComputeSpecular(...) with a specular color of vec3(1.0) instead of our vec3(0.04). Then they use the Fresnel 0.04 term in a mix statement the way we had originally specified here. It's just a question of where the Fresnel 0.04 comes into clearcoat coloring, either before mixing with the base layer, or during the mix.

During the email chain we had several people ask if f_clearcoat already included that term or not, and confusion over that led us to change the formula. I think we need to say something to clarify that (and it sounds like I didn't accomplish that here, can someone suggest better wording please?)

@UX3D-nopper
Copy link
Contributor

Let's asks the others later today.

@emackey
Copy link
Member Author

emackey commented Mar 11, 2020

The offending sentence was removed, so this PR is mostly error correction and formatting changes now. I'm merging it time for today's meeting.

As a reminder, new PRs for issues discussed here are still welcome after ratification so long as they don't change the IP.

@emackey emackey merged commit 81a16c4 into master Mar 11, 2020
@emackey emackey deleted the clearcoat-tweak branch March 11, 2020 13:26
@donmccurdy donmccurdy added this to the PBR Next milestone Apr 28, 2020
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.

5 participants