Validity-based warping of trilinear UVW for APV sampling to reduce leaking in certain situations #6458
Conversation
FrancescoC-unity
requested review from
sebastienlagarde,
JulienIgnace-Unity and
iM0ve
|
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 SRP Core 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. |
|
It appears that you made a non-draft PR! |
# Conflicts: # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ProbeReferenceVolume.cs # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ShaderVariablesProbeVolumes.cs # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ShaderVariablesProbeVolumes.cs.hlsl
There was a problem hiding this comment.
The PR adds 2 new parameters in the volume override, does not change already existing behavior. The two properties are: Leak Reduction Mode and Min Valid Dot Product Values. Leak reduction doesnt solve the leak issues completely, but at least in my projects the improvement was substantial. The second one has a very minor effect, more below.
What's tested:
- Quick check for regression (Baking, Clearing, Loading data, Dilation, Virtual offset)
- New properties on 2 projects
Showcase
Leak reduction off
Leak reduction on
Leak reduction off
Leak reduction on
Leak reduction off
Leak reduction on
Min Valid Dot Product Values, a slight change can be seen on top left of the screen.
https://user-images.githubusercontent.com/36502659/147084878-df5a2d04-6c1f-47a1-9ab8-5003df2fa314.mp4
And lastly, here I highlight some problematic case where the leak still cant be prevented. Its expected that this system will not prevent all leaks, but maybe the showcase can still be useful.
https://user-images.githubusercontent.com/36502659/147085111-fe2c1a3c-f7ad-45a5-a453-56f786584a56.mp4
# Conflicts: # com.unity.render-pipelines.core/Editor/Lighting/ProbeVolume/ProbeGIBaking.Dilate.cs # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ProbeBrickPool.cs # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ProbeReferenceVolume.cs # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ShaderVariablesProbeVolumes.cs # com.unity.render-pipelines.core/Runtime/Lighting/ProbeVolume/ShaderVariablesProbeVolumes.cs.hlsl # com.unity.render-pipelines.high-definition/Runtime/Lighting/ProbeVolume/ProbeVolumeLighting.cs
|
Remaining fails are known. Merging |
IMPORTANT This might even be removed, however given that it produces some results, to avoid carrying it over forever in all experimentaton I prefer to open a PR for it. We'll make a decision on what to do with it before going out of experimental
IT IS NOI a perfect or even great solution, but is cheap and provides something decent in certain cases, so it is worth letting it through. By default it is off for now
This PR introduces a simple and very crude method for reducing light leaking in some situations. To keep this cheap we will only warp the UVW used to trilinearly sample the SH data. This has some inherent limitations I will discuss in a bit.
The PR introduces two weighting:
I also added a factor to kill values that are small but not yet zero. This weighting alone doesn't seem to be enough, so we need the following:
This is a bit different. When a probe is invalid it is because it is inside the surface. Dilation will fill said probes with valid looking data, but this data will likely come from both sides of the surface, acting as a carrier of leaking.
Essentially however we can treat the validity as a way of detecting that a surface is on the way and part of the samples used in trilinear interpolation.
The idea is to skew the UVW in such a way that invalid probes (likely to contain leaky data) will have a lesser contribution to the final result.
This idea is in itself very crude, it will fail anytime a probe does not fall inside an object; however it can be surprisingly effective in certain cases and it is relatively cheap.
Unfortunately, if we limit ourselves to warping texture coordinates and therefore a single sample of the SH data, there is no way to completely kill all invalid probes from the equation, the best we can do is to move the sample away from them.
Consider for example
Green being valid probes and red invalid probes. The sample position in blue will be at most moved to the center of the lattice, which will reduce the contribution of the bad corners, but it won't by any mean kill them. To do that we'd need to manually filter (code is already there with the define MANUAL_FILTERING), but that will of course incur in much higher cost.
Even when killing contribution would be possible, if we stick with a single sample, then wed be reducing the dimensionality of the interpolation, potentially leading to either discontinuities or degenrate cases. More investigation will be needed, but I am not convinced we can't get anything that is not explicitly flawed if we stick to only a trilinear sample.
We might need to consider exposing that as an option, however being that the validity based approach even in that case is not perfect, I am not exposing that yet in hopes of finding better solutions if we relax a bit the cost constraint.
To make the sample of the validity cheap, because we are really just interested in a binary representation, for each probe I pack in a single channel 8bit texture the data for each neighbour probe that will be sampled for trilinear interpolation (1 bit per probe). This way, only one extra sample is needed and said sample is small, impacting performance very little.
That said, combined with use of normal bias and view bias (I am allowing bigger values of this latter one in this PR), some nice results can be obtained.
The fatal flaw however is when a surface is present, but no probe fall inside it. This will make the algorithm think that all probes from both sides of a surface are valid, leading to some leaking untouched (see the blue parts in the images below - the red leaking is improved, the blue one is left as-is)
Here the blue problem is due to the warping of uvw are not able to kill completely an invalid probe.
More investigation is needed for said cases
So : this is cheap and works in some cases, more investigation is needed for other options but we need tor relax a bit the performance constraints (making them an option probably)
I am also going to add a way to manually mark some probe as invalid next to help guide this algorithm or to have artist guided leak prevention in certain cases.