Solari brdf fixes#23818
Closed
dylansechet wants to merge 4 commits into
Closed
Conversation
0a0f302 to
31356c0
Compare
Contributor
Author
|
Splitting into smaller PRs. |
yonas
pushed a commit
to yonasBSD/bevy
that referenced
this pull request
Apr 24, 2026
# Objective BRDF fixes for solari. Split off from bevyengine#23818. ## Solution Use a local TBN instead of mikktspace, which was producing seams on the spheres used for the white furnace test. ## Testing On main: <img width="1920" height="1013" alt="1_main_F_AB" src="https://github.com/user-attachments/assets/c05acf43-faa1-4128-9cdc-807ed2364518" /> This PR: <img width="1920" height="1013" alt="2_proper_tbn" src="https://github.com/user-attachments/assets/7230e9d9-02ec-4435-8d74-1b7b7a314e27" />
tychedelia
pushed a commit
to processing/bevy
that referenced
this pull request
Apr 30, 2026
# Objective BRDF fixes for solari. Split off from bevyengine#23818. ## Solution Use a local TBN instead of mikktspace, which was producing seams on the spheres used for the white furnace test. ## Testing On main: <img width="1920" height="1013" alt="1_main_F_AB" src="https://github.com/user-attachments/assets/c05acf43-faa1-4128-9cdc-807ed2364518" /> This PR: <img width="1920" height="1013" alt="2_proper_tbn" src="https://github.com/user-attachments/assets/7230e9d9-02ec-4435-8d74-1b7b7a314e27" />
This was referenced May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
BRDF fixes for solari!
Solution
A downside to this approach is that it breaks reciprocity because of the simplifications that Filament's multi-scattering formulation makes. As far as I can tell the path tracer is unidirectional, so this shouldn't cause issues. The fix to restore reciprocity would be to add the full Kulla-Conty compensation lobe, which requires an extra LUT.
Note: I've mostly focused on getting this correct, performance can probably be improved. For example, this does entirely too many
F_ABlookups.Testing and showcase
White furnace
We're still not passing, but the changes improve it a lot.
On main:




With the LUT-based F_AB:
Adding the local TBN:
Adding the BRDF fixes:
Bunch of spheres
Notice the darkening on main for the middle row of spheres, with metallic=0.5.
Main:


This PR:
Dragons
The shading differences are particularly visible on the dragons in the front row.
There is some kind of exposure or tonemapping difference between bevy and cycles that's been driving me crazy, but I haven't been able to track it down.
Individual images
Main:

This PR:

Cycles:

More Dragons
Individual images
Main:This PR:

Cycles:
