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

Fixes sheen intensity #4697

Merged
merged 7 commits into from Oct 6, 2022
Merged

Fixes sheen intensity #4697

merged 7 commits into from Oct 6, 2022

Conversation

GSterbrant
Copy link
Contributor

@GSterbrant GSterbrant commented Oct 5, 2022

Description

Our analytical approach inspired by Three.js inconsistently divides by PI to balance the energy over the hemisphere. This caused a strong reduction in sheen light intensities.

Furthermore, we also used the roughness of the sheen to sample different environment mips, but sampling the lowest mip is more correct as reflections will always be extremely rough, seeing as sheen is simulating quite big microfibers.

Before:
model-viewer (8)

After:
model-viewer (6)

This PR also fixes an issue where clear coat was using the specular color to tint reflections. According to the glTF spec, the f0 for clear coat is always white.

@GSterbrant GSterbrant added bug area: graphics Graphics related issue labels Oct 5, 2022
@GSterbrant GSterbrant self-assigned this Oct 5, 2022
Copy link
Contributor

@willeastcott willeastcott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. Do you advocate including this fix in a 1.57.1? Or wait until the next minor bump?

@GSterbrant
Copy link
Contributor Author

I hope this can go in 1.57.1 as it's a nasty bug affecting all cloth materials.

@slimbuck
Copy link
Member

slimbuck commented Oct 5, 2022

This PR is also updating clearcoat. Is that intended? You don't mention this in the description.

@GSterbrant
Copy link
Contributor Author

@slimbuck Thanks! Had forgotten about that. Added it to the comment.

@GSterbrant GSterbrant merged commit 3fe1073 into main Oct 6, 2022
@GSterbrant GSterbrant deleted the gsterbrant_sheen_spec_fixes branch October 6, 2022 08:48
mvaligursky pushed a commit that referenced this pull request Oct 6, 2022
* Fix double clearcoat spec multiplication.

* Use CC fresnel for lights

* Use CC fresnel for lights

* Add reflection sample with base mip offset for materials

* Defer multiplication of sheen specular color to combine step.

* We don't use reciprocal PI for energy preservation, so doing it for sheen was a mistake.

Also revert the mip offset reflection sampling.
Also, sheen should not be a factor of the materials reflectivity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants