bevy
bevy copied to clipboard
Fix specular envmap in deferred
Objective
- Fixes #11414
Solution
- Add specular occlusion to g-buffer so PbrInput can be properly reconstructed for shading with a non-zero value allowing the spec envmap to be seen
Oh btw, did we document anywhere that specular occlusion doesn't work on webgl2, and it'll use diffuse instead? If not, please add that to the StandardMaterial docs.
I dont see why it should be in standard material docs, but I could add it there. There's no specular_occlusion input, the occlusion_texture field only affects diffuse. This is an SSAO/PBR effect, maybe it can be mentioned on the SSAO docs.
Ah right. Docs on either the PbrInput in the shader or in SSAO, or in deferred would be fine, up to you where you think it's best. I just think we should note this down somewhere.
@JMS55 There shouldn't be any limitation keeping specular occlusion from working in webgl2 vs diffuse with the current impl. The current XeGTAO impl in bevy doesn't work in webgl2 (though it could), but that's a separate issue.
Yeah it would just mean adding another g-buffer texture on webGL, which could hurt performance. I think that can be done in a follow up with its own perf testing to assess whether its worthwhile. The visual difference will not be that big and we're already monochromizing the diffuse occlusion for gbuffer's sake.
I just realized this fix is wrong and the correct fix is much simpler, here's why:
- the deferred lighting example doesn't have SSAO on
- PbrInput::specular_occlusion is only written by SSAO
- reconstruction assumes there's data there even though SSAO wrote nothing and reads a zero
- spec envmap gets multiplied by zero in deferred without SSAO
this means that a correct fix would just be to initialize specular_occlusion to 1.0 when there's no SSAO. If SSAO is enabled, then we get proper specular occlusion. If its not, then we get spec envmap unoccluded (we have no data with which to occlude it with).
I definitely don't think we should add another texture for baked occlusion. At some point though we may add a color attachment for depth to enable the features the read depth directly in webgl. But it's really important to keep the attachments of the deferred prepass as small as possible. And we really should be removing the normal attachment and copying to the normal prepass texture in a subsequent pass, and then consider if there's a way to further minimize the size of the motion vectors.
Occlusion would usually be either real time, or part of baked light maps, so I don't see much value in it being part of the gbuffer at all and only included it because there was a bit of extra space and it meant I wasn't changing as much about the capabilities for the initial deferred PR. I think the moment there is something more worthwhile to put in the gbuffer we should probably ditch it altogether. Also our current form of specular occlusion is pretty poor imo and leads to a lot of both over and under occlusion in a lot of common situations, and a more appropriate solution would not involve having a specular occlusion channel in the gbuffer anyway.
For forward materials, there shouldn't be anything keeping baked occlusion from working in webgl2.
@rodolphito Oh, good catch regarding the default value! I hadn't taken a look at the code. The default for occlusion should definitely be 1.0 or vec3(1.0) depending on the context for the reasons you stated.
I just tested the new fix, seems to be working.