[Merged by Bors] - Rebase of existing PBR work #1554
Conversation
|
Ping @StarArawn @IngmarBitter: are you okay if we close out the old PRs and work off of this? |
|
@mtsr @alice-i-cecile If this is the preferred PR I'm fine with closing mine out. I don't really see a commit history with my commits, but I'm cool with that I suppose for the sake of getting the ball rolling on this. |
|
@StarArawn I'm sorry about that. I did a rebase, but somewhere the author got mixed up. I can see if I can fix it. If you check, you'll see your third commit it worked the way it should. It's the first two that got messed up. |
|
I fixed the author on your commits, @StarArawn . |
This comment has been minimized.
This comment has been minimized.
| vec3 diffuse = diffuseColor * Fd_Burley(roughness, NdotV, NoL, LoH); | ||
|
|
||
| light_accum += | ||
| ((diffuse + specular) * light.color.xyz) * (light.color.w * NdotL); |
There was a problem hiding this comment.
Unless you're going to multiply the incoming light intensity by PI, you'd need to do that here so that the light intensities behave the way artists expect. Also, you could just remove the 1 / PI parts from the equations above and just leave a comment in them saying they'd be cancelled out anyway.
https://seblagarde.wordpress.com/2012/01/08/pi-or-not-to-pi-in-game-lighting-equation/
There was a problem hiding this comment.
Are you on the bevy discord, by any chance? Would make it easier to ask some questions that don't really need to be archived here. My nick is the same there.
There was a problem hiding this comment.
Do you mean 1.0 / 4*PI here? See https://google.github.io/filament/Filament.html#mjx-eqn-pointLightLuminanceEquation
There was a problem hiding this comment.
I've added what I think is the physically correct term in ac1e23c
I'll read the linked article and see if I can figure it out.
There was a problem hiding this comment.
I can't find anything like this term PI in either the filament document or the code and my math is not good enough to reproduce the derivations.
There was a problem hiding this comment.
I've got a better grasp on this now, but I'd love for somebody else to chime in on how we want the light to behave. I agree that if we end up multiplying by PI we might as well remove the 1.0 / PI instead.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
running through the 3d examples:
|
|
example |
|
bors r+ |
This is a rebase of StarArawns PBR work from #261 with IngmarBitters work from #1160 cherry-picked on top. I had to make a few minor changes to make some intermediate commits compile and the end result is not yet 100% what I expected, so there's a bit more work to do. Co-authored-by: John Mitchell <toasterthegamer@gmail.com> Co-authored-by: Ingmar Bitter <ingmar.bitter@gmail.com>
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
This PR adds normal maps on top of PBR #1554. Once that PR lands, the changes should look simpler. Edit: Turned out to be so little extra work, I added metallic/roughness texture too. And occlusion and emissive. Co-authored-by: Carter Anderson <mcanders1@gmail.com>
|
you guys did great work on this! Thanks! |
This is a rebase of StarArawns PBR work from #261 with IngmarBitters work from #1160 cherry-picked on top.
I had to make a few minor changes to make some intermediate commits compile and the end result is not yet 100% what I expected, so there's a bit more work to do.