Tom94/practical-path-guiding

About the division of pdf in the record code

deadmarston opened this issue · 4 comments

Dear Tom,

Hello, Thank you for sharing this great work.
I have one confusion about your code:

About the "record" function in the "DTreeWrapper", I wonder why you decided to divide the woPdf here instead of using the radiance?

Float irradiance = rec.radiance / rec.woPdf;

building.recordIrradiance(dirToCanonical(rec.d), irradiance, rec.statisticalWeight, directionalFilter);
Tom94 commented

Hi there,

glad the code is useful to you! :)

Each quad of the DTree is meant to contain the total radiance that flows through it. "Total radiance" here means radiance integrated over the quad's solid-angle footprint. This integral of radiance (which turns out to have the same units as irradiance---hence the name) is estimated via Monte Carlo by taking random samples and dividing by their PDF. This PDF is woPdf.

Hi,

Thank you for the reply. :)

In your code of committing the radiance from NEE,

if (dTree) {
        Vertex v = Vertex{
            dTree,
            dTreeVoxelSize,
            Ray(its.p, dRec.d, 0),
            throughput * bsdfVal * dRec.pdf,
            bsdfVal,
            L,
            dRec.pdf,
            bsdfPdf,
            dTreePdf,
            false,
        };

Here radiance is throughput * bsdfVal * value * weight, throughput is throughput * bsdfVal * dRec.pdf and woPdf is dRec.pdf. According to your code, the radiance will divide the throughput and woPdf respectively, which means the final result irradiance to commit is value * weight / dRec.pdf / dRec.pdf. And the value has already divided the dRec.pdf during the sampling and let's interpret it as lightValue / dRec.pdf, as a result, the commited irradiance is 'lightValue * weight / dRec.pdf / dRec.pdf / dRec.pdf', which is somehow weird because I think it should be lightValue * weight / dRec.pdf. Could you explain it for me? Thanks. :)

Tom94 commented

You're completely right! Instead of
throughput * bsdfVal * dRec.pdf,
the code should say
throughput * bsdfVal / dRec.pdf,
to result in lightValue * weight / dRec.pdf, as you say.

I think the reason this was never discovered is that the code path is only hit when m_nee == EKickstart... which is a weird mode to begin with. It's also never used in the paper.

Do you want to create a pull request for fixing this bug (so you have credit in the commit history) or are you fine with me fixing it myself?

It's fine that you fix it yourself. :)