Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove lighting importance sampling #6

Open
KovenYu opened this issue Oct 14, 2022 · 1 comment
Open

Remove lighting importance sampling #6

KovenYu opened this issue Oct 14, 2022 · 1 comment

Comments

@KovenYu
Copy link

KovenYu commented Oct 14, 2022

Hi Team,

Thanks for sharing the wonderful work and amazing codebase! I wonder if it is possible to remove lighting importance sampling in the cuda kernel code, so that we only use BRDF importance sampling. My assumption is to remove:

// Light importance sampling sx = ((float)(params.perms[lightIdx][i] % params.n_samples_x) + uniform_pcg(rng_state)) * strata_frac; sy = ((float)(params.perms[lightIdx][i] / params.n_samples_x) + uniform_pcg(rng_state)) * strata_frac; ray_dir = lightSample(sx, sy, pdf_light); pdf_bsdf = bsdf_pdf(pDiffuse, pSpecular, gb_normal, wo, ray_dir, alpha); process_sample(idx, ray_origin, ray_dir, gb_pos, gb_normal, gb_view_pos, gb_kd, gb_ks, pdf_light + pdf_bsdf, sample_frac, diff, spec, diff_grad, spec_grad); diffAccum = diffAccum + diff; specAccum = specAccum + spec;
From L512, and then remove pdf_light in L527.

Am I doing correctly to achieve my goal? I have no prior CUDA programming experiences. Appreciate very much for any comment!

Thanks,
Koven

@JHnvidia
Copy link
Collaborator

Hi,

Spontaneously this seems correct. I think that the balance heuristic should simplify to normal sampling if you set pdf_light=0 (or remove it) in L527. It's notoriously easy to introduce errors in MC-rendering through, so it would be a good idea to forward render some reference mesh (e.g. using https://github.com/NVlabs/nvdiffrecmc/blob/main/dataset/dataset_mesh.py) at high sample counts with our original code and your modified version to verify that there are no intensity shifts. You can expect the noise to increase, but given high enough sample count the two versions should converge to the same image.

Light importance sampling is quite central to some of our results. Without it I would not expect scenes with strong directional lights or shadows to work as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants