[HDRP] Lit coat ssr-rtr: Fix various issues with using SSR lighting with IBL fallback (case 1380351) #6106
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lback when a coat is present: 1) SSR lighting was applied using the coatMask in such a way that the bottom layer incorrectly gained coat-traced light as the coatMask went to 0, even prior #5565. #5565 added the usage of the coat lighting (irrespective of coatMask considerations, ie even when coatMask = 1) when the bottom layer has a roughness close enough to 0, ie to that of the coat lobe for which we traced the indirect light. However, even when the bottom layer roughess was very different (blendFactor = 1), coatMask could escape that logic as it went to 0. (In that case, the comment in the code "// On top of this above we also add an additional hack to limit the smoothness used on based layer depending on coat layer for SSR application (See below)" was wrong) 2) The reflectionHierarchyWeight (again even before #5565) was incorrectly including an F term, while the consumed / used-up light hierarchy weight is orthogonal to the specific material shading details (reflectance factors, ie pre-integrated FGD or coatF here). So regardless of if we used the ssr coat lighting for the bottom layer, as coatF grew, we would artificially remove total bottom layer light at grazing angles. Ergo, this was incorrectly darkening the grazing angle fresnel reflection because of darkening of the bottom layer. Moreover, with the separate coat hierarchy weight, the envlighting can no longer use the same probe volume intersection weight, and a separate update and capping must be done for the coat reflection hierarchy. 3) In #5565, the code states we lerp-in coat light contribution for the bottom layer when its smoothness is above 0.9 and lerp-out from 0.9 to 0.8, but the code was actually fading from 0.9 to 0.7, not 0.8. I assumed the code behavior was desired (since probably eyeballed) and used the same ranges but made the code cleaner. 4) More importantly (vs 3), the behavior since #5565 could actually erroneously eliminate the coat lobe and use only the bottom lobe with the coat-traced light: this happened when the bottom roughness was close to the coat roughness, as in that case we had a "blendfactor" that would lerp from coat F to bottom FGD, instead of actually combining both lobes with their respective wanted contributions.
|
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. HDRP Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
This was referenced Oct 21, 2021
slunity
changed the title
|
This PR fixes this issue https://fogbugz.unity3d.com/f/cases/1380351/ |
anisunity
changed the title
sebastienlagarde
approved these changes
Nov 22, 2021
sebastienlagarde
pushed a commit
that referenced
this pull request
Dec 8, 2021
…ith IBL fallback (case 1380351) #6106
sebastienlagarde
added a commit
that referenced
this pull request
Dec 8, 2021
* Fixed references to probes not cleared when unloading a scene #5945 * [Fogbugz # 1365368] Fixing debug views for drs #5948 * [HDRP] Stacklit RTR: Fix issues with RTR reflections from stacklit materials #6103 * [HDRP] Lit coat ssr-rtr: Fix various issues with using SSR lighting with IBL fallback (case 1380351) #6106 * [HDRP] StackLit coat ssr-rtr light hierarchy and IBL fallback fixes. #6107 * AxF ssr-rtr: Implement refined light reflection hierarchy with separate weight for coat and base lobe to correctly fallback from SSR light to light reflection probes and also implement the same coat-traced light "reuse" as Lit for bottom lobe when roughnesses are similar. (#6108) This also cleans up the overlapping contributing reflection probes / dual normal setup problem in EvaluateBSDF_Env(). Based on previously discussed #4968. * Fix issue with reflection probe normalization via APV (#6140) * fix issue * - * [not ready] HDRP Scene Template: Bumped up IET framework version to 2.1 and better default layout for 1080p monitors (#6153) * bumped up iet framework to 2.1, created new 1080p and 4k friendly layout * Merge branch 'master' into hdrp-template-bumpup-iet-framework-version Co-authored-by: sebastienlagarde <sebastien@unity3d.com> * [HDRP] Fix custom pass utils in XR #6271 * Update reference screenshots Co-authored-by: Adrien de Tocqueville <adrien.tocqueville@unity3d.com> Co-authored-by: Kleber Garcia <kleber.garcia@unity3d.com> Co-authored-by: slunity <37302815+slunity@users.noreply.github.com> Co-authored-by: FrancescoC-unity <43168857+FrancescoC-unity@users.noreply.github.com> Co-authored-by: pierre-unity <39901544+pierre-unity@users.noreply.github.com> Co-authored-by: Antoine Lelievre <antoinel@unity3d.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose of this PR
See #5565
and #4968
SSR lighting was applied using the coatMask in such a way that the bottom layer incorrectly gained coat-traced light as the coatMask went to 0, even prior #5565.
#5565 added the usage of the coat lighting (irrespective of coatMask considerations, ie even when coatMask = 1) when the bottom layer has a roughness close enough to 0, ie to that of the coat lobe for which we traced the indirect light.
However, even when the bottom layer roughess was very different (blendFactor = 1), coatMask could escape that logic as it went to 0.
(In that case, the comment in the code "// On top of this above we also add an additional hack to limit the smoothness used on based layer depending on coat layer for SSR application (See below)" was wrong)
The reflectionHierarchyWeight (again even before #5565) was incorrectly including an F term, while the consumed / used-up light hierarchy weight is orthogonal
to the specific material shading details (reflectance factors, ie pre-integrated FGD or coatF here).
So regardless of if we used the ssr coat lighting for the bottom layer, as coatF grew, we would artificially remove total bottom layer light at grazing angles.
Ergo, this was incorrectly darkening the grazing angle fresnel reflection because of darkening of the bottom layer.
Moreover, with the separate coat hierarchy weight, the envlighting can no longer use the same probe volume intersection weight, and a separate update and capping must be done for the coat reflection hierarchy.
https://fogbugz.unity3d.com/f/cases/1380351/
(Notice the grazing angle darkening in the A/B compare)
In #5565, the code states we lerp-in coat light contribution for the bottom layer when its smoothness is above 0.9 and lerp-out from 0.9 to 0.8, but the code was actually fading from 0.9 to 0.7, not 0.8. I assumed the code behavior was desired (since eyeballed) and used the same ranges but made the code cleaner.
More importantly (vs 3), the behavior since #5565 could actually erroneously eliminate the coat lobe and use only the bottom lobe with the coat-traced light: this happened when the bottom roughness was close
to the coat roughness, as in that case we had a "blendfactor" that would lerp from coat F to bottom FGD, instead of actually combining both lobes with their respective wanted contributions.
Additional video showing coat mask behavior (eg for point 1 above):
Before:
https://drive.google.com/file/d/1z0i81gTs9PqZnZ2VnRV6XGQsF26GvhwJ/view?usp=sharing
https://drive.google.com/file/d/1f5e_Kus8WaXdE2UwQ-rb9DnBeQTO4YyI/view?usp=sharing
After:
https://drive.google.com/file/d/1ifHDZUlfdByzSL4ocN0jMYjiB8b_EuD4/view?usp=sharing
Testing status
Manual test scene, see note below for graphics tests.
Comments to reviewers
Note that other incoming PRs might also change graphic test results, might be worth it to bundle them together instead of changing screenshots for each.