BSDF Weighting Issue with MultiGGX
Closed, ResolvedPublic

Description

While looking into D2677, I just noticed an obvious issue in the current master (b94a433ca34, to be precise):

Under certain settings, the results of Branched PT and regular PT will be completely different.

For example, take a default scene and set the cube to use a Principled BSDF with MultiGGX and Specular=Roughness=1, then render with BPT and PT. The BPT version will be more than twice as bright.

I'll look into this, I just thought I'd formally report it as a bug to make sure that this is known considering how close 2.79 is.

Okay, so a bit of testing showed that this is actually not caused by the Principled BSDF at all, it's already a problem in 2.78:

To reproduce the issue, take the default scene, remove the lamp and set the cube to diffuse (1,1,1). It will be exactly the same color as the world since its albedo is 1.
Then, set it to MultiGGX glossy with color (1,1,1) and roughness 1. Again, albedo is 1 so it's invisible.
But, when you mix the two closures with factor 0.5, it suddenly gets darker in PT, but not in BPT.

I think this is caused by the approximate PDF - while the paper points out that using an approximate PDF for MIS is fine, I'm now fairly sure that that refers to the two-sample MIS applied to light sampling vs. bsdf sampling, NOT applying one-sample MIS to closure picking. I'll look into getting this fixed.

Lukas Stockner (lukasstockner97) changed the title from "Principled BSDF Weighting Issue with MultiGGX" to "BSDF Weighting Issue with MultiGGX".Jun 18 2017, 8:04 AM

Okay, some more testing clearly shows that the approximate PDF causes the issue.

I've also found a missing factor in the current approximation that improves the situation somewhat, see P500.
But, while it fixes what definitely is a bug, it may also change render results compared to 2.78, which is why I didn't commit it yet. Any thoughts on how to handle this?

Also, of course it doesn't fix the underlying issue. In theory it would help to not include MultiGGX closures in the MIS weighting, but in practice I'd expect the noise impact to outweigh the correctness improvement.
Another improvement would be possible for the PDF approximation itself, which currently is fairly simple (singlescattering GGX + diffuse term to account for the GGX darkening). However, since the general PDF depends on four dimensions (phi of incoming and outgoing direction as well as theta difference and roughness), finding a perfect fit is fairly unlikely...

Considering this problem, for 2.79 I'd recommend making singlescattering GGX the standard for the Principled BSDF. That way, it should also be possible to use more accurate sampling weights without changing the result.

Changing the default to single scatter GGX is fine with me.

Bastien Montagne (mont29) triaged this task as "Normal" priority.Jun 21 2017, 12:36 PM