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

Docs: mistake in "Importance sampling for the IBL" section #6294

Closed
htlpt opened this issue Nov 12, 2022 · 2 comments · Fixed by #6700
Closed

Docs: mistake in "Importance sampling for the IBL" section #6294

htlpt opened this issue Nov 12, 2022 · 2 comments · Fixed by #6700
Assignees
Labels
documentation Documentation issues

Comments

@htlpt
Copy link

htlpt commented Nov 12, 2022

Describe the bug
Mistake in the Jacobian of transform between distributions (missing a reciprocal)

Maybe a terminology problem ?

To Reproduce
Steps to reproduce the behavior:

  1. Go to 9.2 Importance sampling for the IBL
  2. equation 126, 127

Screenshots
image

Expected behavior
Eq.126 :
image

Eq.127:
image

Additional context
some references:

[1] Physically Based Rendering, Third Edition 13.5.1
image

[2] Microfacet Models for Refraction through Rough Surfaces, EGSR 2007

Jacobian of transformation from l->h is shown, h->l should be the reciprocal

3}4EGR~Q7@@ 8%MGV54PY

@htlpt
Copy link
Author

htlpt commented Nov 12, 2022

also in 5.3.4.2.2 "Discrete Domain", the pdf is missing a "4"
image

image

@romainguy
Copy link
Collaborator

Thanks, those look like omissions/typos indeed.

@pixelflinger pixelflinger added the documentation Documentation issues label Nov 16, 2022
pixelflinger added a commit that referenced this issue Mar 31, 2023
pixelflinger added a commit that referenced this issue Mar 31, 2023
plepers pushed a commit to plepers/filament that referenced this issue Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants