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

Book 3.4: Update sphere importance sampling f(x) #1550

Merged
merged 1 commit into from
May 8, 2024

Conversation

rupsis
Copy link
Contributor

@rupsis rupsis commented Apr 26, 2024

Addresses #1500

@rupsis rupsis requested a review from a team April 26, 2024 15:23
@rupsis
Copy link
Contributor Author

rupsis commented Apr 26, 2024

@dimitry-ishenko Might want to take a peek as well.

@hollasch
Copy link
Collaborator

hollasch commented May 1, 2024

Sorry for the delay in giving feedback here. This is tightly coupled with Dimitry's issue #1536, which I believe should also be included in this change. I'm working on broadening this change to include #1536 (two paragraphs up), which covers the mapping of a point on the unit sphere to its two polar angles.

@rupsis
Copy link
Contributor Author

rupsis commented May 1, 2024

Sorry for the delay in giving feedback here. This is tightly coupled with Dimitry's issue #1536, which I believe should also be included in this change. I'm working on broadening this change to include #1536 (two paragraphs up), which covers the mapping of a point on the unit sphere to its two polar angles.

Sounds good. Want to take what I have here and roll them into your changes?

If so, feel free to close the PR out.

@hollasch
Copy link
Collaborator

hollasch commented May 2, 2024

I'm just working in this branch.

@hollasch
Copy link
Collaborator

hollasch commented May 8, 2024

I'm going to add my changes onto your own.

@hollasch hollasch merged commit d653f2a into dev May 8, 2024
@hollasch hollasch deleted the 1500-sphere-importance-sampling branch May 8, 2024 23:14
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

Successfully merging this pull request may close these issues.

2 participants