Add reflection probe support to compatibility renderer

[BastiaanOlij]

VERY MUCH WIP

This PR adds reflection probe support to the compatibility renderer.

Status: [x] Initialise reflection probe atlas [x] Render 6 sides of a reflection probe [x] Populate MIP levels for roughness support [ ] Store reflection probe information from culling [ ] Render reflection probes on relevant geometry

Note: the current process for calculating MIP levels uses a simple linear downsample (through glBlitFramebuffer), we may do a follow up PR where we replace this with something more fancy for reflection probes that aren't set to real time update.

• Production edit: This closes https://github.com/godotengine/godot/issues/86318.

[BastiaanOlij]

Currently not sure yet how I'm going to approach actually applying the reflection probe data.

First attempt is going down the same path as the mobile renderer however I am worried about the performance impact of handling reflection probes this way. I also ran into a snag with using a samplerCubeArray:

ERROR: SceneShaderGLES3: Fragment shader compilation failed:
0(320) : error C7532: global type samplerCubeArray requires "#version 400" or later
0(320) : error C0000: ... or #extension GL_ARB_texture_cube_map_array : enable
0(320) : error C0000: ... or #extension GL_EXT_gpu_shader4_1 : enable

So if we continue down this road we'll be dependent on extensions that may, or may not, be supported.

edit the CI already is complaining about GL_TEXTURE_CUBE_MAP_ARRAY not being available in GLES platforms, only in OpenGL. So I think that's that.

I have two alternative options we could pursue, both will change the atlas logic so we have individual cubemaps instead of an array:

1. Only allow one reflection probe to effect geometry, this allows us to properly mix in the reflection color in the main scene shader and we can use a define to remove the code if no reflection probe applies.

2. Render reflection probes additive in a separate pass, similar to what we do lights that cast shadows. This will mean colors will look "off" but we can handle multiple probes.

[Lasuch69]

This is a lot more complicated than I thought.

Second option would be a lot better approach as you can just avoid probes overlapping when creating a scene. First approach feels terrible as it would make probes nightmare to work with in complex scenes (splitting big meshes would be necessary just like with the forward rendering light object limit).

I would vote for approach number two any day of the week.

[Calinou]

Second option would be a lot better approach as you can just avoid probes overlapping when creating a scene.

The issue is that any complex scene requires probe blending to look good. You really don't want probe thresholds to be apparent. If you recall Source 1 games, this is an issue that plagued most maps that had reflective floors in them (along with lacking seamless cubemaps).

You don't need to be able to blend a ton of probes together, but being able to blend 2 at any given pixel is absolutely essential in 2024, even for a low end-oriented renderer. The blending doesn't need to be super accurate (it's encouraged to do it over the shortest possible distance for performance reasons), but it needs to be there somehow.

I believe multipass rendering is also slow when you use greater amounts of reflection probes, so that's something to keep in mind as well.

[BastiaanOlij]

OK, figured out why the sky looks weird. It's rendered upside down when rendering the sides... oops

[BastiaanOlij]

Ok, the sky is now reflected properly, but in two sides the ground isn't being reflected... might be a culling issue...

[BastiaanOlij]

Ok, the problem turns out to be that when the maximum distance of the probe is set to 0, it's really limiting things too much and it looks weird.

Setting the distance to something sensible we get a good result:

[Calinou]

Ok, the problem turns out to be that when the maximum distance of the probe is set to 0, it's really limiting things too much and it looks weird.

Max Distance set to 0 is supposed to set the probe's far clip to the longest axis of the ReflectionProbe's size. This works correctly in Forward+ and Mobile from my testing.

This also occurs if you set Max Distance to a value lower than one of the aforementioned axes: it's always clamped so that everything in the probe's bounds is within the probe's far clip. The only case where Max Distance makes a visual difference is when it's greater than the length of the longest axis, so the probe can reflect something located outside its bounds.

[BastiaanOlij]

Max Distance set to 0 is supposed to set the probe's far clip to the longest axis of the ReflectionProbe's size. This works correctly in Forward+ and Mobile from my testing.

This also occurs if you set Max Distance to a value lower than one of the aforementioned axes: it's always clamped so that everything in the probe's bounds is within the probe's far clip. The only case where Max Distance makes a visual difference is when it's greater than the length of the longest axis, so the probe can reflect something located outside its bounds.

Indeed, and this logic is controlled by the core culling logic, so it is using the same code for all renderers. I have yet to figure out why this is going wrong, I just now know that the max distance is what triggers the problem :)

[BastiaanOlij]

Ok, so the issue with adding a 3rd reflection probe turns out to be a never ending loop in RendererSceneCull::render_probes that also happens with the Vulkan renderer.

[BastiaanOlij]

Ok, so the max distance issue is really simple and dumb :P Turns out that if max distance is 0, we use the size of the reflection probe. And as I have relatively small reflection probes with different size edges, it comes out looking weird. Not a bug.

[BastiaanOlij]

As far as I can tell cleaning up reflection probes now works, at least I can't reproduce the issue I had before.

[BastiaanOlij]

Radiance map is now calculated the same way for reflection probes, as for sky. I've moved all the logic for this into a new CubemapFilter class (@clayjohn would appreciate you taking a look at this and see if I've missed anything).

With Metallic = 1.0 and Roughness = 0.0 we can see that the existing sky reflection and new reflection probe reflection nice matches:

With Metallic = 1.0 and Roughness = 1.0 we can see that there is a little more reflection from the probe: I was unable to determine what causes this.

There may be a clue in that ambient mode doesn't seem to have an effect even though I did add the check in: which suggests there is an error somewhere. There are differences in how ambient light is calculated in the compatibility renderer compared to the two Vulkan renderers, this may hold a clue.

[BastiaanOlij]

After https://github.com/godotengine/godot/pull/89134 is merged this needs to be rebase and then reflection_probe_has_atlas_index implemented. That solves the crash issue with multiple probes.

[BastiaanOlij]

Rebased this on top of the reflection probe recurring error fix, that PR will need to be merged first.

This is pretty much all working now:

Only thing is that ambient isn't behaving as expected so here I need some feedback from people reviewing the code, hopefully spot what I'm doing wrong.

[BastiaanOlij]

@Calinou indeed, the two reflection probe limit is per draw call, that's the price we pay on low end hardware. This will require breaking up once meshes instead of having large scene filling meshes.

I'll have a look if I can figure out where we're leaking textures. Interesting it's not printing the texture description, I might improve that in utilities

[BastiaanOlij]

@Calinou note that if you run a dev build it will output which textures leaked by name.

[BastiaanOlij]

@clayjohn as discussed during the meeting, flipped the order around on the ambient color. The color setting does come through now. I'm not sure if it's what is expected, but as far as I can tell its working.

[timothyqiu]

Web exports look dark and no reflection is available. Browser's console outputs lots of invalid texture type errors for framebufferTextureLayer.

It turns out that glFramebufferTextureLayer doesn't like cubemap textures. Changing this line

https://github.com/godotengine/godot/blob/da7bb612b48dbc32105e38d3d0aa9a8b60482e2b/drivers/gles3/storage/light_storage.cpp#L895

to use glFramebufferTexture2D solves the issue:

glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_CUBE_MAP_POSITIVE_X + side, color, 0);

[BastiaanOlij]

Web exports look dark and no reflection is available. Browser's console outputs lots of invalid texture type errors for framebufferTextureLayer.

It turns out that glFramebufferTextureLayer doesn't like cubemap textures. Changing this line

@dsnopek @clayjohn what do you think of this? Should we add an extra #ifdef to use this notation for for web only, or would it be safe to just switch over for all platforms?

[dsnopek]

what do you think of this? Should we add an extra #ifdef to use this notation for for web only, or would it be safe to just switch over for all platforms?

I can confirm via independent Googling that glFramebufferTexture2D() needs to be used with WebGL. :-)

Assuming that will work fine on other platforms, my personal preference would be to avoid #ifdef's and just use that always (with a comment explaining why we're doing it this way). If there's some downside to using that on other platforms, then I guess an #ifdef would be better, but I personally wouldn't expect there to a downside (caveat being that I'm a novice with regard to rendering :-))

[BastiaanOlij]

@dsnopek @timothyqiu done, if you don't mind testing if it all works as advertised :)

[timothyqiu]

Tested on Linux, Windows (on Wine), and Web exports. Works as expected :tada:

[BastiaanOlij]

Thanks @clayjohn , should all be corrected now!

I think most of these snuck in when I changed from having everything in a single buffer, to having color and radiance separated to do the blur correctly.

Also missing the radiance mask was an oversight seeing that didn't exist before. Adding that functionality went in parallel to developing this :P Turned out to be an easy fix though

[akien-mga]

Thanks! Amazing work :tada: