[Example] Fix rounding error for cornell_box.py #1908
Conversation
[fix] fix rounding error artifact for cornell_box
examples/cornell_box.py
Outdated
| @@ -354,6 +358,7 @@ def sample_direct_light(hit_pos, hit_normal, hit_color): | |||
| w = mis_power_heuristic(brdf_pdf, light_pdf) | |||
| nl = dot_or_zero(brdf_dir, hit_normal) | |||
| direct_li += fl * w * nl / brdf_pdf | |||
| ''' | |||
There was a problem hiding this comment.
Why is this part commented?
There was a problem hiding this comment.
for my opinion:
[1] this part sample a reflect direction, so that is indirect lighting, not direct lighting
[2] next iter will count direct lighting as previous indirect lighting, so the radiance is over counted
There was a problem hiding this comment.
Now thinking it back, this is actually doing multiple importance sampling (MIS). See http://www.pbr-book.org/3ed-2018/Light_Transport_I_Surface_Reflection/Direct_Lighting.html#fragment-SampleBSDFwithmultipleimportancesampling-0, so it's not really over counted.
I cannot say that the MIS impl was correct, but could you keep this part, and just tweak the epsilon to see if the dark ring problem goes away?
There was a problem hiding this comment.
There is no doubt that you are right!
This implementation is the same with pbrt-v3 ,totally right!
---------------------------advice -----------------------
I have advice for efficience:
change the calculation when hitting a light for depth > 0 and always record the brdfPdf
if hit_light:
acc_color += throughput * light_color * powerHeuristic(brdfPdf, lightPdf) / brdfPdf
break
Then comment the brdf mis part
--------------------compare------------------------------
[current method] there are two sample direction, three brdf color eval , one intersect detection for light in one iter
[anthoer method] one sample dir, two brdf color eval, none for light intersection
where I learn from:
https://github.com/knightcrawler25/Optix-PathTracer/blob/master/src/optixPathTracer/hit_program.cu
https://github.com/knightcrawler25/Optix-PathTracer/blob/master/src/optixPathTracer/light_hit_program.cu
There was a problem hiding this comment.
I am so sorry that I commit 5 times for such minor bug······
And seems the 5th commits still fails：
Error: Process completed with exit code 1.
There was a problem hiding this comment.
am so sorry that I commit 5 times for such minor bug
No worries && no need to apologize at all!
Just FYI, the check failed at Code Format https://github.com/taichi-dev/taichi/pull/1908/checks?check_run_id=1187328749. Usually you can add @taichi-gardener for review, which is a bot that does the code format automatically :)
I have advice for efficience:
Ah I see. Do I understand correctly that, if the direct light sampling actual hits the light source, then we no longer need to do the BRDF sampling (Sorry I'm no expert on this, and it's been a while since i wrote this example... )?
Codecov Report
@@ Coverage Diff @@
## master #1908 +/- ##
=======================================
Coverage 43.86% 43.86%
=======================================
Files 45 45
Lines 6190 6190
Branches 1099 1099
=======================================
Hits 2715 2715
Misses 3306 3306
Partials 169 169 Continue to review full report at Codecov.
|
lyd405121
changed the title
add space for code format
lyd405121
changed the title
This reverts commit b79ce63.
[1] Add space for code format [2] Delete over count dor indirect lighting
[1] previous comment is based on direct sampling light,not a good strategy [2] more info can get from mit course:https://graphics.stanford.edu/courses/cs348b-03/papers/veach-chapter9.pdf
[fix]
fix rounding error artifact for cornell_box
Related issue = #
[Click here for the format server]