Skip to content

Solari: Fix MIS not using consistent PDF measures#24671

Merged
alice-i-cecile merged 2 commits into
bevyengine:mainfrom
JMS55:solari7-pdf-fix
Jun 22, 2026
Merged

Solari: Fix MIS not using consistent PDF measures#24671
alice-i-cecile merged 2 commits into
bevyengine:mainfrom
JMS55:solari7-pdf-fix

Conversation

@JMS55

@JMS55 JMS55 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Objective

  • MIS between emissive and NEE light samples use area measure for the emissive sample PDF, but solid angle measure for the NEE sample PDF.

Solution

  • Consistently use solid angle PDFs for MIS
  • Also move isinf to sampling.wgsl for consistency

@JMS55 JMS55 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Jun 19, 2026
@JMS55 JMS55 added this to the 0.20 milestone Jun 19, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering Jun 19, 2026

@stuartparmenter stuartparmenter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The math here seems right, but a few extra comments would probably be helpful in the future -- namely the difference between the two PDFs and the NdotV/ray_distance params.

@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Code-Quality A section of code that is hard to understand or change D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Jun 21, 2026

@dylansechet dylansechet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! This unfortunately didn't seem to be the issue with the Cornell box demo scene, and doesn't bring us closer to cycles there
.

@JMS55

JMS55 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@dylansechet I think after some research this only affects variance, so yeah I don't expect it to change the result.

@JMS55 JMS55 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 22, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 22, 2026
Merged via the queue into bevyengine:main with commit 56b08cd Jun 22, 2026
46 checks passed
@github-project-automation github-project-automation Bot moved this from Needs SME Triage to Done in Rendering Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Code-Quality A section of code that is hard to understand or change D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants