Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement GI Reflections (SDFGI, VoxelGI) for LightmapGI #86135

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

guerro323
Copy link
Contributor

This complete a part of godotengine/godot-proposals#3012 (it doesn't address disabling VoxelGI reflections for SDFGI).
Depends on #86102 for Specular Occlusion.

image
(greatly exaggerated for showing the reflections, look in screenshots for the normal ones)
(scene from https://sketchfab.com/3d-models/japanese-street-at-night-fb1bdcd71a5544d699379d2d13dd1171)

This add support for reflections from GI such as SDFGI or VoxelGI onto lightmapped objects.

Benefits and constraints

  • It help level designers dealing with less reflection probes, especially in complex scenes (such as rounded walls, complex corridors that aren't straight, etc...)
  • Reflection probes can still be used and blends well with the GI Reflections and should be used for cube shaped interiors or when you need a mirror-like reflection which can be ruined by SDFGI/VoxelGI voxels look.
  • There is a cost of using LightmapGI + SDFGI, but it's less than SDFGI only as we only apply the reflection buffer (further optimizations could be possible).
  • VoxelGI + LightmapGI has a smaller cost than VoxelGI Only as we only trace for reflection.
  • Without specular occlusion it doesn't work well with simple shapes and no textures if you crank the roughness to 0 and metallic to 100.
  • Specular occlusion will help making the GI reflections look more right and occluded when it should be.

Screenshots and Comparisons

'Reflection' scene

Source: https://github.com/Calinou/godot-reflection

Lightmap 4.2 SDFGI Lightmap + Specular Occlusion Lightmap + SO + SDFGI Lightmap + SO + VoxelGI
image image image image image

Cube scene

Lightmap 4.2 SDFGI Lightmap + Specular Occlusion Lightmap + SO + SDFGI Lightmap + SO + VoxelGI
image image image image image

Japanese Street

Source: https://sketchfab.com/3d-models/japanese-street-at-night-fb1bdcd71a5544d699379d2d13dd1171

Lightmap 4.2 Lightmap + Specular Occlusion Lightmap + SO + SDFGI Lightmap + SO + VoxelGI
image image image image

Simple scene:

  • The left part of the floor is a standard default material, the other part is a metallic with roughness set to 0.
  • The black cubes are material with roughness set to 0 and black color.
  • The grey cube is a 100% metallic white material.
  • The white cube is a 100% specular white material.
Lightmap Only Lightmap + SDFGI
image image
image image

Caveats:

  • You may need to modify the values of SDFGI or VoxelGI as they may be darker when lightmaps are on. A good way to check if everything is right is to have a fully reflective surface and check if the GI reflections match the same brightness/color as your scene.

Open Questions:

  • Optimizations? It is possible to optimize SDFGI to only care about reflection data?
  • An option to disable this effect? Would there be an use case to only have GI reflections only on non-lightmapped objects?

¹ I posted some more screenshots (without specular occlusion) and benchmarks here godotengine/godot-proposals#3012 (comment).

@clayjohn
Copy link
Member

clayjohn commented Jan 6, 2024

Great work so far!

I've added this to the agenda for the next rendering meeting. This idea has popped up a few times, but I think it will require some discussion and a lot more work. Particularly we need to discuss the workflow issues here and how to optimize out the calculation of diffuse lighting from VoxelGI/SDFGI.

@atirut-w
Copy link
Contributor

atirut-w commented Jan 25, 2024

Testing this out on my project, SDFGI reflections are abnormally dark on lightmapped meshes.

Details

No lightmap on sphere:

image

Lightmap on sphere:

image

This issue is also apparent in a newly created project:

image

@guerro323 guerro323 force-pushed the lightmap-gi-gireflection branch from 8cae994 to 07f7523 Compare January 25, 2024 15:39
@guerro323
Copy link
Contributor Author

guerro323 commented Jan 25, 2024

@atirut-w This seems to come from the mix of a lightmap with a dynamic directional light and the default specular occlusion mode.
It seems that the lightmap is a little bit darkened when computed with a dynamic light which incorrect result to the specular occlusion.
You can remove this effect by switching to Project Settings → Lightmapping → Specular Occlusion Mode → Conservative (Fast) (I'll make this option the default as Reduce is too much problematic with dynamic lights; you can also disable specular occlusion)
image

@guerro323 guerro323 force-pushed the lightmap-gi-gireflection branch from 07f7523 to 7f78c6d Compare January 25, 2024 15:46
@atirut-w
Copy link
Contributor

you can also disable specular occlusion

Apparently, turning it off also darken reflections?

@atirut-w
Copy link
Contributor

atirut-w commented Jan 27, 2024

(Moved to #86102)

@guerro323 guerro323 force-pushed the lightmap-gi-gireflection branch from 6e447c1 to c1dee86 Compare January 27, 2024 09:26
@guerro323
Copy link
Contributor Author

guerro323 commented Jan 27, 2024

Apparently, turning it off also darken reflections?

I accidentally included an old commit of the specular occlusion in it which made disabling it buggy.
It should be fine now.

@guerro323 guerro323 force-pushed the lightmap-gi-gireflection branch from c1dee86 to 1bc7491 Compare January 27, 2024 10:39
@atirut-w
Copy link
Contributor

It should be fine now.

Well, about that... disabling specular occlusion now turn reflections black...
image

@guerro323
Copy link
Contributor Author

Would it be possible to have a simple scene that reproduce this? I'm not able to get black reflections :/

@atirut-w
Copy link
Contributor

I managed to reproduce a slightly different behavior where everything except the sky is black: Lightmap.zip

@guerro323
Copy link
Contributor Author

guerro323 commented Jan 27, 2024

It is fine on my side.
I'm not sure why it doesn't work for you, maybe it is related to the shader cache?
image

@atirut-w
Copy link
Contributor

Weird. Clearing the shader cache doesn't solve it either.

Here's the system information anyway, so that it may help: Godot v4.3.dev (1bc749129) - Windows 10.0.22621 - Vulkan (Forward+) - dedicated AMD Radeon RX 6700 XT (Advanced Micro Devices, Inc.; 31.0.23013.1023) - AMD Ryzen 7 5700X 8-Core Processor (16 Threads)

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased against master 17e7f85), it works as expected.

LightmapGI only

Screenshot_20240127_204434 webp

LightmapGI + SDFGI

Non-reflective materials appear darker because the material isn't fully rough (it's the fallback 3D material).

Screenshot_20240127_204457 webp

SDFGI only

Screenshot_20240127_204516 webp

No GI

Screenshot_20240127_204531 webp

@guerro323 guerro323 force-pushed the lightmap-gi-gireflection branch from 1bc7491 to 5e29632 Compare February 7, 2024 20:36
@guerro323 guerro323 force-pushed the lightmap-gi-gireflection branch from 5e29632 to d9ca537 Compare February 7, 2024 20:45
@akien-mga akien-mga requested a review from clayjohn February 8, 2024 09:15
@atirut-w
Copy link
Contributor

atirut-w commented Feb 8, 2024

It is fine on my side. I'm not sure why it doesn't work for you, maybe it is related to the shader cache?

I narrowed it down to something about AMD GPUs. I tested the MRP on my laptop which has Intel Xe graphics and the issure was not occuring there. I think this needs more testing on AMD GPUs like mine (RX 6700 XT) because it's a pretty breaking bug.

One silver lining is that it only appears when specular occlusion is disabled, I guess?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants