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

Code on the website that will lead to confusion #11

Open
Enigmatisms opened this issue Mar 5, 2023 · 2 comments
Open

Code on the website that will lead to confusion #11

Enigmatisms opened this issue Mar 5, 2023 · 2 comments

Comments

@Enigmatisms
Copy link

In chapter 15.2 - Sampling Volume Scattering, for H-G phase function sampling:

<<Compute  for Henyey–Greenstein sample>>= 
Float cosTheta;
if (std::abs(g) < 1e-3)
    cosTheta = 1 - 2 * u[0];
else {
    Float sqrTerm = (1 - g * g) /
                    (1 - g + 2 * g * u[0]);
    cosTheta = (1 + g * g - sqrTerm * sqrTerm) / (2 * g);                         // HERE
}

There should be a negative sign in front of cosTheta RHS.

The code block is not consistent with , and is not consistent with

  • PhaseHG function definition: Float denom = 1 + g * g + 2 * g * cosTheta;, if no minus sign given, denom should be 1 + g * g - 2 * g * cosTheta. According to the ray direction convention in PBRT-v3, all 'incident ray' points outwards, therefore cosTheta RHS should have a minus sign.
  • The formula in that chapter, listed right above the code block.
  • Actual experiments... Initially I implemented h-g sampling based on the code above, which costed me two days to debug my code. The following experiments are based on pbrt-v3, and the code doesn't have the problem mentioned above. You will notice that we can not really distinguish between these two the incorrect results, even though one exhibits forward scattering and the other is backward.
correct g = 0.5 correct g = -0.5 incorrect g = 0.5 incorrect g =-0.5
cornell-correct-pos05 cornell-correct-neg05 cornell-pos05 cornell-neg05
@mmp
Copy link
Owner

mmp commented Mar 5, 2023

Interesting; that code is correct in the pbrt-v3 source code but seems to be incorrect in the online text. I will look into what happened there. https://github.com/mmp/pbrt-v3/blob/aaa552a4b9cbf9dccb71450f47b268e0ed6370e2/src/core/medium.cpp#L194.

Sorry for any inconvenience.

@Enigmatisms
Copy link
Author

Thanks. Looking forward to reading more new advanced topics in PBR-book.

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