SSS diffusion bug with multiple intersections in probe disk #84536

New Issue

Maximilian Tarpini · 2021-01-08T19:07:34+01:00

2021-01-08 19:07:34 +01:00

System Information
Ubuntu 18.04.3 LTS (Bionic Beaver), Linux kernel 4.15.0-128-generic

Blender Version
blender-2.91.0

Short description of error
While using SSS shader node with diffusion profiles there's a nasty bug that occurs when multiple instersections are found from within the disk radius. Practically accumulating multiple samples results in a kind of 'glowing irradiance' probably due to wrong PDFs being used for disk sampling. It renders correctly when just one intersection is found.

For example this line that's weighting the radial sampling with MIS.. shoudn't be:

float w = sqr(pdf_N) / sqr(pdf_N) + sqr(pdf_T) + sqr(pdf_B);

instead of

float w = pdf_N / sqr(pdf_N) + sqr(pdf_T) + sqr(pdf_B);

ie. added sqr() for pdf_N as in the formal power heuristic ?
That would be in line with the balance heuristic, darkening the whole SSS but no more 'glowing'.

Exact steps for others to reproduce the error
Render any model that have close geometric features first with diffusion and then with randomwalk will put in evidence the problem.

I attach here the relative squared error between randomwalk and diffusion renders.

**System Information** Ubuntu 18.04.3 LTS (Bionic Beaver), Linux kernel 4.15.0-128-generic **Blender Version** blender-2.91.0 **Short description of error** While using SSS shader node with diffusion profiles there's a nasty bug that occurs when multiple instersections are found from within the disk radius. Practically accumulating multiple samples results in a kind of 'glowing irradiance' probably due to wrong PDFs being used for disk sampling. It renders correctly when just one intersection is found. For example this line that's weighting the radial sampling with MIS.. shoudn't be: ``` float w = sqr(pdf_N) / sqr(pdf_N) + sqr(pdf_T) + sqr(pdf_B); ``` instead of ``` float w = pdf_N / sqr(pdf_N) + sqr(pdf_T) + sqr(pdf_B); ``` ie. added sqr() for pdf_N as in the formal power heuristic ? That would be in line with the balance heuristic, darkening the whole SSS but no more 'glowing'. **Exact steps for others to reproduce the error** Render any model that have close geometric features first with diffusion and then with randomwalk will put in evidence the problem. I attach here the relative squared error between randomwalk and diffusion renders. ![cycles_sssbug_RSE.png](https://archive.blender.org/developer/F9556031/cycles_sssbug_RSE.png) ![cycles_sssbug_randomwalk.png](https://archive.blender.org/developer/F9556040/cycles_sssbug_randomwalk.png) ![cycles_sssbug_diffusion.png](https://archive.blender.org/developer/F9556039/cycles_sssbug_diffusion.png)

Maximilian Tarpini commented

2021-01-08 19:07:34 +01:00

Added subscriber: @maxt

Maximilian Tarpini changed title from ~~Cycles: SSS diffusion bug with multiple intersections in probe disk~~ to SSS diffusion bug with multiple intersections in probe disk

2021-01-08 19:12:38 +01:00

Maximilian Tarpini commented

2021-01-12 00:38:02 +01:00

Jeez this thing gets worse everytime I test it.

Attached scene.
meatboy.zip

Jeez this thing gets worse everytime I test it. ![glowingteeths.png](https://archive.blender.org/developer/F9560399/glowingteeths.png) Attached scene. [meatboy.zip](https://archive.blender.org/developer/F9560408/meatboy.zip)

Brecht Van Lommel commented

2021-01-12 14:11:20 +01:00

Added subscriber: @brecht

Brecht Van Lommel commented

2021-01-12 14:11:20 +01:00

Changed status from 'Needs Triage' to: 'Needs User Info'

Brecht Van Lommel commented

2021-01-12 14:11:20 +01:00

This technique is not energy conserving and glowing is a known limitation. There's no practical way to normalize over the number of hits in a path tracer, as each sample is independent. Normalizing when multiple hits are found for a single sample is not correct either.

It's one of the main reasons random walk was added, and with D9932: Cycles: Implement Dwivedi guiding for path-traced subsurface scattering we'll likely make that the default.

Regarding squaring pdf_N, see the comment above that code:

    /* Multiple importance sample between 3 axes, power heuristic
     * found to be slightly better than balance heuristic. pdf_N
     * in the MIS weight and denominator cancelled out. */
    float w = pdf_N / (sqr(pdf_N) + sqr(pdf_T) + sqr(pdf_B));

There may be a bug in the implementation, but so far I do not see it demonstrated.

This technique is not energy conserving and glowing is a known limitation. There's no practical way to normalize over the number of hits in a path tracer, as each sample is independent. Normalizing when multiple hits are found for a single sample is not correct either. It's one of the main reasons random walk was added, and with [D9932: Cycles: Implement Dwivedi guiding for path-traced subsurface scattering](https://archive.blender.org/developer/D9932) we'll likely make that the default. Regarding squaring `pdf_N`, see the comment above that code: ``` /* Multiple importance sample between 3 axes, power heuristic * found to be slightly better than balance heuristic. pdf_N * in the MIS weight and denominator cancelled out. */ float w = pdf_N / (sqr(pdf_N) + sqr(pdf_T) + sqr(pdf_B)); ``` There may be a bug in the implementation, but so far I do not see it demonstrated.

Maximilian Tarpini commented

2021-01-12 16:37:57 +01:00

Never heard that this is a known limitation.
In fact this is my still wip directional dipole implementation using disk sampling and I cannot see any glowing.
As soon as I finished that I'll try to see where yours fails, initially I just wanted to have it as a comparison because yours it's 30% faster than mine and that's why I found the misbehaviour.

About randomwalk as a default.. be aware that it needs really good geometry to work cool.. for example the scene attached above not only fails with cycles diffusion but it double fails with random walk (maybe with dwivedi will be even worse coz it has to target formally an exit interface).. because the teeths and eyes are fully modelled.. so the internal parts are screwing up the walk that at some point will anyway eval light.. hence the black silhouettes. Yeah one can put teeths and eyes on some separated geometry but that's not always possible. That's why imo diffusion SSS should be completely working because for many geometry cases there's no way randomwalk can work out of the box.

Never heard that this is a known limitation. In fact this is my still wip directional dipole implementation using disk sampling and I cannot see any glowing. As soon as I finished that I'll try to see where yours fails, initially I just wanted to have it as a comparison because yours it's 30% faster than mine and that's why I found the misbehaviour. ![diffusion_notglowing.png](https://archive.blender.org/developer/F9561158/diffusion_notglowing.png) About randomwalk as a default.. be aware that it needs really good geometry to work cool.. for example the scene attached above not only fails with cycles diffusion but it double fails with random walk (maybe with dwivedi will be even worse coz it has to target formally an exit interface).. because the teeths and eyes are fully modelled.. so the internal parts are screwing up the walk that at some point will anyway eval light.. hence the black silhouettes. Yeah one can put teeths and eyes on some separated geometry but that's not always possible. That's why imo diffusion SSS should be completely working because for many geometry cases there's no way randomwalk can work out of the box. ![rw_balckartefacts.png](https://archive.blender.org/developer/F9561163/rw_balckartefacts.png)

Jorge García-Brazales commented

2021-01-15 16:44:33 +01:00

Added subscriber: @strangerman

Maximilian Tarpini commented

2021-01-28 16:23:50 +01:00

You were right saying it's not energy conserving ;) In my first attemp I was only partly integrating indirect rays (ie. diffuse lookup instead of full SSS) so it was less evident.
The only remedy is to filter out diffusion when wi wo are facing, this is highlighted by Burley at some point here: https://tinyurl.com/yykunttx
He did not mention how to do it, so for myself I am simply fading out diffusion with something like this :

const float cos = clamp(dot(surface.N, hitData.N), -1.0f, 1.0f);
const float fadeout = sqrt((1.0f + cos) * 0.5f);
eval *= fadeout;

Works cool :

One last thing I do is to assign some more importance to last intersections (closer and more likely to be reachable from the point being shaded) when many and picking randomly a subset of those.

You were right saying it's not energy conserving ;) In my first attemp I was only partly integrating indirect rays (ie. diffuse lookup instead of full SSS) so it was less evident. The only remedy is to filter out diffusion when wi wo are facing, this is highlighted by Burley at some point here: https://tinyurl.com/yykunttx He did not mention how to do it, so for myself I am simply fading out diffusion with something like this : ``` const float cos = clamp(dot(surface.N, hitData.N), -1.0f, 1.0f); const float fadeout = sqrt((1.0f + cos) * 0.5f); eval *= fadeout; ``` Works cool : ![meatboy_fade.png](https://archive.blender.org/developer/F9602647/meatboy_fade.png) ![meatboy_nofade.png](https://archive.blender.org/developer/F9602650/meatboy_nofade.png) ![meatboy_RSE.png](https://archive.blender.org/developer/F9602652/meatboy_RSE.png) One last thing I do is to assign some more importance to last intersections (closer and more likely to be reachable from the point being shaded) when many and picking randomly a subset of those.

Brecht Van Lommel commented

2021-02-01 20:09:03 +01:00

That render looks good, feel free to contribute a patch to improve the current Burley SSS.

I'm not sure what you mean by "last intersection", but note that in general intersections can be found in arbitrary order, depending on the BVH implementation. To converge to the same result, the result should be independent of this order.

That render looks good, feel free to contribute a patch to improve the current Burley SSS. I'm not sure what you mean by "last intersection", but note that in general intersections can be found in arbitrary order, depending on the BVH implementation. To converge to the same result, the result should be independent of this order.

Ankit Meel commented

2021-04-07 11:25:17 +02:00

Changed status from 'Needs User Info' to: 'Confirmed'

Maximilian Tarpini was assigned by Ankit Meel

2021-04-07 11:25:17 +02:00

Ankit Meel commented

2021-04-07 11:25:17 +02:00

Added subscriber: @ankitm