Move screen space effects into a separate class

[BastiaanOlij]

Moving screen space effects into their own effects class. Includes:

• Screen Space Indirect Lighting
• Screen Space Ambient Occlusion
• Screen Space Reflection

SSIL and SSAO do not have stereoscopic support, will deal with this in a separate PR.
SSR stereoscopic support is not working yet, still debugging the issue.
Everything else is working.

[BastiaanOlij]

@Calinou if you have some good test scenes to test these with, can you give this PR a twirl? I think I've got everything working correctly but I've only tested screen space reflection properly so far.

[Calinou]

Tested on https://github.com/godotengine/godot/files/7854794/test_ssil_3.zip, with SSAO, SSR and SSIL enabled at the same time.

I confirm this PR works as expected on both clustered and mobile¹ backends. It also fixes #62525 even when this PR is rebased against latest master locally.

However, this PR doesn't fix #56724 or #50151 – SSIL and SSR artifacts are still common when resizing the viewport.

Footnotes

1. Mobile doesn't support SSAO, SSIL or SSR, but it doesn't print errors when enabling those features. We should eventually hide those options in the Environment resource when using the mobile backend, still. ↩

[BastiaanOlij]

This PR is pretty much done except for:

• SSAO and SSIL do not have stereoscopic support, this is something we will do in a later PR.
• SSR does have stereoscopic support but currently this doesn't work. I believe the issue is in the code for unprojecting not working correctly.

Setting this to "ready for review" as all the other work can be checked and fixing SSR for stereoscopic can be moved to a separate PR.

[BastiaanOlij]

Found a little more I want to do around SSIL and SSAO

[BastiaanOlij]

Allocation of buffers for SSAO is now moved from RendererSceneRenderRD._process_ssao to SSEffects::ssao_allocate_buffers.

With all buffers in our SSAORenderBuffers struct and all settings in our SSAOSettings struct, also greatly cleaned up passing things around.

[Calinou]

I tested the latest revision of this PR on the above MRP and it still works as expected. 👍

[BastiaanOlij]

Made the same change to SSIL, pretty happy about the cleanup this gives, nearly all logic around SSIL, SSAO and SSR is now contained in this class. Really the only bit that we can't make modular is the shader code that is in the scene shader.
Other than that, these effects can be pretty much dropped into a renderer.

[reduz]

My general thinking is that I am still not sure the array approach makes more sense for compute multiview, it makes code harder to read, harder to test if you don't have multiview and for compute there is zero difference between doing 1 call or two, or three.

And there is zero effort in just creating the individual textures to pass to the effects if you want to run them more than once. I would be much happier if the compute shaders had no idea about what multiview is and this was handled entirely from C++.

[reduz]

I think for practical reasons, we really should do away with it everywhere it makes sense. Otherwise think that contributors who don't care about multiview (the vast majority) that want to tinker with shader will have a very hard time testing to see if these work (or just plain ignore that side of the shader) resulting in more breakage for the multiview users, because honestly we cant expect that every change to a shader needs to be tested in multiview.

In fact, if going forward we implement things like an easier to use post processing architecture using compute, we need to make sure that it can go transparently to multiview without the developer interaction, otherwise it will be very hard to ensure it works there and XR users will complain that this is not supported or not tested.

Unlike C++ these errors are not catched at compile time since we dont even plan on compiling those shaders when not in use.

In the end, you will get the opposite effect to what you want, multiview will be a burden to other developers and/or will break a lot more often.

So IMO, I would really make the rendering code the least multiview centric as possible. At much require the function to operate on array of buffers (separate elements) but not texture arrays.

The least invasive multiview changes are, the more chances it has that it will be well supported and not break because the reality is that most developers don't care about it.

Multiview code needs to be written with this mindset, making sure it works but not ever expecting to take center stage.

[BastiaanOlij]

@reduz indeed, my main concern was one of performance whether there is overhead in performing the same action twice. However with screen space effects where we're still performing both passes in the same command queue there doesn't seem to be a difference.

I think the only two things we can't get away from when rendering stereo:

1. when using framebuffers (so raster) we need to use multiview, especially when rendering geometry where multiview does take away a lot of overhead compared to running things twice
2. that we do need to do full unprojecting which means storing projection matrices in uniforms as we run out of space in push constants. Right now I'm introducing these where needed but I'm working on changing the way we're handling scene data so we have a uniform with all data available for any shader to use.

[reduz]

@BastiaanOlij You have to think of Compute as something that processes little blocks of data on demand. If you do two calls to it (and not barrier the calls) its exactly the same as doing one for both since you just keep the GPU busy doing what it has to do.

[reduz]

would still wait on @clayjohn approval since this touches a lot of his code.

[BastiaanOlij]

@reduz ok, I did add the bariers in this case because I'm processing one eye at a time so we don't have to double the sizes of all the buffers. In this case I wonder if it would make much difference seeing plenty is done in parallel already. But maybe reshuffling and making all intermediate buffers have 2 layers would allow us an improvement here?

[Calinou]

Performance comparison between master ee53a51 and this PR (rebased against ee53a51):

master ee53a51

1024×600

Project FPS: 1028 (0.97 mspf)
Project FPS: 1025 (0.97 mspf)
Project FPS: 1026 (0.97 mspf)
Project FPS: 1026 (0.97 mspf)
Project FPS: 1025 (0.97 mspf)

2560×1440

Project FPS: 212 (4.71 mspf)
Project FPS: 212 (4.71 mspf)
Project FPS: 211 (4.73 mspf)
Project FPS: 212 (4.71 mspf)
Project FPS: 212 (4.71 mspf)

This PR

1024×600

Project FPS: 1028 (0.97 mspf)
Project FPS: 1025 (0.97 mspf)
Project FPS: 1026 (0.97 mspf)
Project FPS: 1025 (0.97 mspf)
Project FPS: 1025 (0.97 mspf)

2560×1440

Project FPS: 209 (4.78 mspf)
Project FPS: 211 (4.73 mspf)
Project FPS: 211 (4.73 mspf)
Project FPS: 211 (4.73 mspf)
Project FPS: 212 (4.71 mspf)

Performance seems identical (within the margin of error).

[clayjohn]

Looks fine to me. I noticed that there are a lot of VRS changes that have been commented out, are those just placeholders for now? Or is it dead code that should be removed?

[BastiaanOlij]

Looks fine to me. I noticed that there are a lot of VRS changes that have been commented out, are those just placeholders for now? Or is it dead code that should be removed?

I've removed these. I think this is ready now

[akien-mga]

Thanks!