Skip to content

Solari: Improve specular GI#21907

Merged
james7132 merged 18 commits intobevyengine:mainfrom
JMS55:solari6-better-specular-gi
Dec 7, 2025
Merged

Solari: Improve specular GI#21907
james7132 merged 18 commits intobevyengine:mainfrom
JMS55:solari6-better-specular-gi

Conversation

@JMS55
Copy link
Contributor

@JMS55 JMS55 commented Nov 21, 2025

Go back to only sampling the world cache for rough and non-first-bounce surfaces, but sample direct lighting when that doesn't happen.

Mainline
image

This PR
image

Pathtraced reference
image

Fixes #21967.

@JMS55 JMS55 added this to the 0.18 milestone Nov 21, 2025
@JMS55 JMS55 added A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Refinement Improves output quality, without fixing a clear bug or adding new functionality. labels Nov 21, 2025
@JMS55
Copy link
Contributor Author

JMS55 commented Nov 21, 2025

There's still some weird glowing around the very edges of the monkey, but this is much closer.

@JMS55 JMS55 marked this pull request as draft November 23, 2025 16:47
@JMS55 JMS55 marked this pull request as ready for review November 27, 2025 18:25
@JMS55
Copy link
Contributor Author

JMS55 commented Nov 27, 2025

@SparkyPotato please review this carefully. I'm not super confident about my MIS code (but the good news is that implementing it myself, I have a bunch better understanding now).

@JMS55 JMS55 requested a review from IceSentry November 27, 2025 18:27
Comment on lines +18 to +19
const DIFFUSE_GI_REUSE_ROUGHNESS_THRESHOLD: f32 = 0.4;
const WORLD_CACHE_TERMINATION_ROUGHNESS_THRESHOLD: f32 = 0.4;
Copy link
Contributor

Choose a reason for hiding this comment

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

This threshold is below an input perceptual roughness of 0.63, which seems really high.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was based on a bunch of testing. 0.4 was the threshold where I didn't notice visual artifacts.

It'll eventually be configurable though.

let wi_tangent = vec3(dot(wi, T), dot(wi, B), dot(wi, N));

let p_light = 1.0 / inverse_p_light;
let p_bounce = max(0.001, ggx_vndf_pdf(wo_tangent, wi_tangent, ray_hit.material.roughness));
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't actually matter, because even if it were 0, the power heuristic would become p_light^2 / p_light^2 = 1. But this is only called when the surface isn't perfectly specular, so the pdf won't be 0 anyways. Maybe the NaNs you were seeing were related to p_light being 0, that this max also hides (but the output in this case is 0, not 1)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing shows that ggx_vndf_pdf is the one breaking things. p_light is fine.

@JMS55 JMS55 marked this pull request as draft November 28, 2025 15:46
@JMS55 JMS55 added the D-Complex Quite challenging from either a design or technical perspective. Ask for help! label Nov 30, 2025
@JMS55 JMS55 marked this pull request as ready for review November 30, 2025 17:53
@JMS55 JMS55 requested a review from SparkyPotato November 30, 2025 17:53
@IceSentry IceSentry 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 Dec 2, 2025
@james7132 james7132 added this pull request to the merge queue Dec 7, 2025
Merged via the queue into bevyengine:main with commit 6c22c9b Dec 7, 2025
36 checks passed
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-Refinement Improves output quality, without fixing a clear bug or adding new functionality. D-Complex Quite challenging from either a design or technical perspective. Ask for help! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

Solari: Incorrect MIS for directional lights

6 participants