Skip to content

Solari: Fix mirror artifacts #22468

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.

Sign up for GitHub

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

Jump to bottom
Merged
alice-i-cecile merged 31 commits into from
Jan 17, 2026
Merged

Solari: Fix mirror artifacts #22468

alice-i-cecile merged 31 commits into from
Jan 17, 2026

Conversation

@JMS55
Copy link
Contributor

@JMS55 JMS55 commented Jan 10, 2026 •
edited
Loading

Objective

  • Fix artifacts appearing in perfect mirrors

Solution

  • Rather than trying to hack the microfacet BRDF to work at low roughness, replace it with a separate mirror BRDF
  • The cos_theta part of the rendering equation has been moved into the BRDF, since the mirror BRDF does not use it (it cancels out)

Showcase

Before - artifacts (black lines)
image

After - no artifacts
image

JMS55 added 24 commits January 4, 2026 12:32
@JMS55
WIP
9448600
@JMS55
Scene work
03becb3
@JMS55
Fix
8200f9e
@JMS55
Fix
bebbe5f
@JMS55
Merge commit '59104abe9e4f94413c1a77b11d117d3a4fc3b9a7' into solari6-psr
c71d595
@JMS55
Cleanup
c9ec8d3
@JMS55
Misc refactor
395e055
@JMS55
Make gbuffer writable
48bf412
@JMS55
WIP
8d18855
@JMS55
Fixes
22eb306
@JMS55
Finish PSR
b5b770a
@JMS55
Misc
72bba7d
@JMS55
Merge branch 'main' into solari6-psr
1d2407b
@JMS55
Refactoring
c02721d
@JMS55
Merge branch 'solari6-psr' of github.com:JMS55/bevy into solari6-psr
4737c3f
@JMS55
Do not terminate in world cache on first bounce no matter what (preve…
605558e 
…nts WC from showing up in reflections when you're really close to something)
@JMS55
Solari: Proper mirror BRDF instead of abusing microfacet model
4367ac3
@JMS55
Save change for another PR
9e4badf
@JMS55
Save change for another PR
232a4bb
@JMS55
Fix link
3fbf46f
@JMS55
Merge branch 'main' into solari6-psr
9533746
@JMS55
Fix no-DLSS mode (stupid naga_oil behavior)
9a41ea3
@JMS55
Merge commit '9a41ea39e0d8828a0ccd1269179913f0aeeae913' into solari6-…
f4a3fe5 
…proper-mirror-brdf
@JMS55
Fixes
6c0ab64
@JMS55 JMS55 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Jan 10, 2026
@JMS55 JMS55 requested a review from SparkyPotato January 10, 2026 17:00
@JMS55 JMS55 added this to the 0.19 milestone Jan 10, 2026
@JMS55 JMS55 changed the title Solari: Fix mirror artifacts Solari: Fix mirror artifacts and world cache showing up in mirrors Jan 10, 2026
@alice-i-cecile alice-i-cecile added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Jan 14, 2026
@JMS55 JMS55 changed the title Solari: Fix mirror artifacts and world cache showing up in mirrors Solari: Fix mirror artifacts Jan 15, 2026
@JMS55 JMS55 added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jan 15, 2026
@JMS55 JMS55 marked this pull request as ready for review January 15, 2026 00:07
alice-i-cecile
alice-i-cecile reviewed Jan 15, 2026
View reviewed changes
alice-i-cecile
alice-i-cecile requested changes Jan 15, 2026
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

image

This is the offending artifact right? The visual difference looks good; just one small request for a maintainability comment.

@JMS55
Copy link
Contributor Author

JMS55 commented Jan 15, 2026

Yeah it fixes that artifact.

@JMS55 JMS55 requested a review from atlv24 January 15, 2026 15:26
atlv24
atlv24 reviewed Jan 15, 2026
View reviewed changes
@alice-i-cecile alice-i-cecile self-requested a review January 15, 2026 18:53
@JMS55 JMS55 requested a review from atlv24 January 16, 2026 15:07
@JMS55
Copy link
Contributor Author

JMS55 commented Jan 16, 2026

CI is lying about the typo 🙄

@alice-i-cecile alice-i-cecile 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 Jan 17, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 17, 2026
Merged via the queue into bevyengine:main with commit bf72875 Jan 17, 2026
40 checks passed
@JMS55 JMS55 mentioned this pull request Jan 18, 2026
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jan 22, 2026
@JMS55
Solari: Better path termination heuristic (#22570)
d458904 
I was using a path termination heuristic designed for a radiance cache,
but solari uses an irradiance cache. Whoops.

This new heuristic is inspired by RTXGI, and prevents the world cache
from showing up in reflections properly.

To test, move the camera close to a mirror surface.


---

Also contains some bugfixes for bugs introduced in #22468.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@atlv24 atlv24 Awaiting requested review from atlv24

+1 more reviewer

@SparkyPotato SparkyPotato SparkyPotato approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Rendering
Status: Done

Milestone

0.19

Development

Successfully merging this pull request may close these issues.

4 participants

@JMS55 @alice-i-cecile @atlv24 @SparkyPotato