-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Upgrade SDFGI to HDDAGI #86267
base: master
Are you sure you want to change the base?
Upgrade SDFGI to HDDAGI #86267
Conversation
Pretty good job, performance increased when moving , and this time decided to use the default cell size , because as you said i exaggerated with that 1cm. It seem´s pretty good though the problem is still gi colliding with the geometry for me, in comparison this is with an empty scene. Vídeo sin título - Screen Recording - 17_12_2023, 22_17_50.webm Vídeo sin título - Screen Recording - 17_12_2023, 22_24_17.webm |
scene/resources/environment.h
Outdated
enum DynamicGICascadeFormat { | ||
DYNAMIC_GI_CASCADE_FORMAT_16x16x16, | ||
DYNAMIC_GI_CASCADE_FORMAT_16x8x16, | ||
DYNAMIC_GI_CASCADE_FORMAT_MAX, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
16×8×16 should be before 16×16×16 as it's faster (and lower quality), to match other Godot enums.
While this means the default current value of SDFGI_Y_SCALE_75_PERCENT
(1
) will map to DYNAMIC_CASCADE_GI_FORMAT_16x16x16
, this isn't really an issue in practice because Godot doesn't save default values to scene/resource files. This means that if you were previously using the default value, you can be using the new default value even if it has to be 0
instead of 1
. This is the same approach followed in #75468.
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/sdfgi/frames_to_converge", PROPERTY_HINT_ENUM, "5 (Less Latency but Lower Quality),10,15,20,25,30 (More Latency but Higher Quality)"), 5); | ||
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/sdfgi/frames_to_update_lights", PROPERTY_HINT_ENUM, "1 (Slower),2,4,8,16 (Faster)"), 2); | ||
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_converge", PROPERTY_HINT_ENUM, "6 (Less Latency/Mem usage & Low Quality),12,18,24,32 (More Latency / Mem Usage & High Quality)"), 1); | ||
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_update_lights", PROPERTY_HINT_ENUM, "1 (Faster),2,4,8,16 (Slower)"), 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on my testing, lower values run slower but provide lower latency, so the hint should be clarified:
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_update_lights", PROPERTY_HINT_ENUM, "1 (Faster),2,4,8,16 (Slower)"), 2); | |
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_update_lights", PROPERTY_HINT_ENUM, "1 (Less Latency but Slower),2,4,8,16 (More Latency but Faster)"), 2); |
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/sdfgi/frames_to_update_lights", PROPERTY_HINT_ENUM, "1 (Slower),2,4,8,16 (Faster)"), 2); | ||
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_converge", PROPERTY_HINT_ENUM, "6 (Less Latency/Mem usage & Low Quality),12,18,24,32 (More Latency / Mem Usage & High Quality)"), 1); | ||
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_update_lights", PROPERTY_HINT_ENUM, "1 (Faster),2,4,8,16 (Slower)"), 2); | ||
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_update_inactive_probes", PROPERTY_HINT_ENUM, "1 (Faster),2,4,8,16 (Slower)"), 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on my testing, lower values run slower but provide lower latency, so the hint should be clarified:
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_update_inactive_probes", PROPERTY_HINT_ENUM, "1 (Faster),2,4,8,16 (Slower)"), 3); | |
GLOBAL_DEF(PropertyInfo(Variant::INT, "rendering/global_illumination/hddagi/frames_to_update_inactive_probes", PROPERTY_HINT_ENUM, "1 (Less Latency but Slower),2,4,8,16 (More Latency but Faster)"), 3); |
Streaks aligned with the X axis are visible when volumetric fog is enabled and GI Inject is greater than simplescreenrecorder-2023-12-18_00.04.08.mp4Minimal reproduction project: test_hddagi_volumetric_fog.zip |
The new Occlusion Bias setting helps solve some of the dark artifacting mentioned in the previous thread. I feel that the splotchiness of SDFGI is almost completely reduced and not an issue anymore as well.
As mentioned in the previous thread, the ability for users to author probe positions manually in a domain (similar to Unity's Adaptive Probe Volumes or its aging Light Probes system) would allow artists to resolve most of these issues manually, and most users probably aren't making environments large enough or procedural enough to warrant a cascading solution (nor can Godot handle large streaming worlds). The option for both would be welcome. I'm aware there are solutions in the works for these issues, just putting them here to keep record. |
An even better solution is allowing users to set Cell Size and Cascade Distance independently, for each cascade - performance be damned. Hide it behind an advanced toggle or something if you must, but it always irked me that we never had full control over SDFGI cascades. The default, unchangeable ratios are horrendously crawly, no stability whatsoever, and make cascade 0 so close on smaller cell sizes that it becomes irrelevant for the majority of the scene. But I suppose this merits a separate proposal. |
This is mostly why I want to implement the high density probes option. The main problem right now is that, on small indoors, the GI runs out of probes at a certain distance and hence it stops receiving lighting. Or, sometimes, all 4 probes are occluded at that position. With high density that problem should mostly resolve, but it is independent to this PR, which aims to replace SDFGI first.
This is dependent on work Clay is doing to fix the normal buffer resolution. I can't merge this PR until that one is done.
Thats kind of the same and this is the main problem with DDGI and these types of probe based GI. I am hoping the situation improves with the high density probes option.
My plan is to have both high density probes and also a screen space part . SSIL is not designed for this kind of GI, so I need to write a proper screen space tracing that traces the distances smaller than a single probe. With that all pixels should get proper lighting. But then again, the plan with this PR is to supersede SDFGI, since It's pretty large and difficult to keep up to date as-is. Take it as foundation work, then I will work on the other stuff I mentioned. |
The problem is the density, not the distance. Cascades are always 16x16 regions, so customizing different cell size or cascade distance will always bite you one way or the other. This is why my plan to work on the high density probes. |
Yes, and my idea was being able to set arbitrary probe densities by changing Cell Size and Cascade Distance independently - i.e. if you want more probe density in cascade 0, you keep Cascade Distance at the default 12.8, but set Cell Size smaller. Is the 16x16 region limitation something that can't be changed/solved? |
@Jamsers that is unfortunately not possible because cascades use a ton of memory and more density scales them exponentially and make them unusable pretty quick. This is why I need a special, separate, technique to increase the density without affecting memory so much. |
After trying I would say it in a pretty good state( literally it has more or less fps than volumetric fog in a empty scene. Also,if i decrease cascades to 1 and put the light update to 16 it might be at the same vfog level of fps in a small scene with cubes). All pretty good, now the issue is with scenes like sponza where even if geometry is simple i still find that huge fps drops( motion is not the one having the fault here as it happens statically too) when using the meshes, like the bigger, the more hddagi consumes from it. look the difference with my above videos and the images below. Can this be sorted out ,or the only way would be with the cascade sething, because if simple scenes is like this, not sure of big ones. although maybe the issue is just my igpu(radeon vega 3), as it not as good. |
@Saul2022 just so you understand better the dependency on geometry. HDDAGI performance does of course depend on the amount of geometry in the level, what it does not depend is on the geometry complexity. This means if a scene with 1 million polygons occupies the same physical space as a scene with 1500 polygons, it is pretty much the same for HDDAGI. |
Alright, thank you for clearing it out. |
@@ -82,6 +82,54 @@ | |||
<member name="background_mode" type="int" setter="set_background" getter="get_background" enum="Environment.BGMode" default="0"> | |||
The background mode. See [enum BGMode] for possible values. | |||
</member> | |||
<member name="dynamic_gi_bounce_feedback" type="float" setter="set_dynamic_gi_bounce_feedback" getter="get_dynamic_gi_bounce_feedback" default="1.0"> | |||
How much light bounces back to the probes. This increases the amount of indirect light received on surfaces. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still seems to be the case as of the latest revision of this PR, so I suggest documenting it:
How much light bounces back to the probes. This increases the amount of indirect light received on surfaces. | |
How much light bounces back to the probes. This increases the amount of indirect light received on surfaces. | |
[b]Note:[/b] Values higher than [code]1.0[/code] may result in infinite feedback loops with bright surfaces. This can cause GI to appear extremely bright over time. |
The amount of cascades used for global illumination. More cascades allows the global illumination to reach further away, but at the same time it costs more memory and GPU performance. Adjust this value to what you find necessary in your game. | ||
</member> | ||
<member name="dynamic_gi_enabled" type="bool" setter="set_dynamic_gi_enabled" getter="is_dynamic_gi_enabled" default="false"> | ||
Turns on Dynamic GI. This provides global illumination (indirect light and reflections) for the whole scene. Only static objects contribute to GI while dynamic objects can also recieve it (check whether your object is static, dynamic or disabled with [member GeometryInstance3D.gi_mode]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns on Dynamic GI. This provides global illumination (indirect light and reflections) for the whole scene. Only static objects contribute to GI while dynamic objects can also recieve it (check whether your object is static, dynamic or disabled with [member GeometryInstance3D.gi_mode]. | |
If [code]true[/code], dynamic global illumination is enabled for the whole scene (indirect light and reflections). Only static objects contribute to GI while dynamic objects can also receive it (check whether your object is static, dynamic or disabled with [member GeometryInstance3D.gi_mode]). |
Adjust the amount of energy that geometry recieved from GI. Use this only as a last resort because it affects everything uniformly and decreases the quality. If needed, consider using [member Environment.dynamic_gi_bounce_feedback] or [member Light3D.light_indirect_energy] to inject more energy into the system. | ||
</member> | ||
<member name="dynamic_gi_filter_ambient" type="bool" setter="set_dynamic_gi_filter_ambient" getter="is_dynamic_gi_filtering_ambient" default="true"> | ||
Filter the ambient light, this results in higher quality transitions between the cascades. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filter the ambient light, this results in higher quality transitions between the cascades. | |
If [code]true[/code], filters the ambient light added by the dynamic GI system. This results in higher quality transitions between the cascades. |
Filter the ambient light, this results in higher quality transitions between the cascades. | ||
</member> | ||
<member name="dynamic_gi_filter_probes" type="bool" setter="set_dynamic_gi_filter_probes" getter="is_dynamic_gi_filtering_probes" default="true"> | ||
Filter the probes (averaging probes with neighbouring probes) to smooth out the light transitions. This option can be used safely, as occlusion between probes is considered when filtering, but it may also result on lower light frequency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filter the probes (averaging probes with neighbouring probes) to smooth out the light transitions. This option can be used safely, as occlusion between probes is considered when filtering, but it may also result on lower light frequency. | |
If [code]true[/code], filters the probes (averaging probes with neighbouring probes) to smooth out indirect light transitions that may appear on surfaces that are mostly indirectly lit. This option can be used without the risk of introducing light leaking, as occlusion between probes is considered when filtering. Enabling probe filtering may result in reduced high-frequency detail in indirect lighting. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Alright folks, I know everyone is excited for the improvements that HDDAGI brings, but please keep the conversation on topic. This is a place to discuss technical aspects of the code, not to endlessly discuss how soon this should be merged. If the off topic conversation continues, we will lock this thread so that only maintainers can comment. We want to merge HDDAGI as soon as it is finished. It will be finished when Juan finds the time to finish it or when someone else decides the take over the work. Nobody can wave a magic wand to get this merged sooner, even though we are all excited by it. |
#ifdef MOLTENVK_USED | ||
imageStore(geom_facing_grid, grid_pos, uvec4(imageLoad(geom_facing_grid, grid_pos).r | facing_bits)); //store facing bits | ||
imageStore(geom_normal_bits, igrid_pos, uvec4(imageLoad(geom_normal_bits, igrid_pos).r | (1 << bit_ofs))); //store solid bits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be
imageStore(geom_normal_bits, igrid_pos, uvec4(imageLoad(geom_normal_bits, igrid_pos).r | (1 << bit_ofs))); //store solid bits | |
imageStore(geom_normal_bits, igrid_pos, uvec4(imageLoad(geom_normal_bits, igrid_pos).r | bit_normal)); //store solid bits |
It seemed to be the one issue with rebasing to master and getting this running on macos
RadiantUwU@af0374a EDIT: a new issue arisen: VoxelGI reflections dont work. |
What a coincidence I was just doing this exactly. Left a few comments on that commit
|
@@ -1573,47 +1516,71 @@ void fragment_shader(in SceneData scene_data) { | |||
|
|||
if (!sc_use_forward_gi && bool(instances.data[instance_index].flags & INSTANCE_FLAGS_USE_GI_BUFFERS)) { //use GI buffers | |||
|
|||
vec2 coord; | |||
ivec2 coord = ivec2(gl_FragCoord.xy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume somewhere around here is what makes the GI look choppy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the comments, there's also one Texture RID getting leaked which is potentially related to the texture views but I'm unsure.
Unrelated to the leaked handles I took some reflection focused screenshots to show the stock behavior with various gi/reflection things enabled: https://imgur.com/a/5NqunUw
I think that while the hddagi reflections capture much more detail than sdfgi, it does end up looking extremely blocky and sometimes in weird ways, like with the lamppost which shows up as a jagged tower of dark cubes. I don't think reflection quality should be a blocker, and it's already an improvement, but it does make me curious if some of the other gi approaches could be used to capture anisotropic qualities of solid geometry better than the current voxels.
cascade.light_process_dispatch_buffer = RD::get_singleton()->storage_buffer_create(sizeof(uint32_t) * 4, Vector<uint8_t>(), RD::STORAGE_BUFFER_USAGE_DISPATCH_INDIRECT); | ||
cascade.light_process_dispatch_buffer_copy = RD::get_singleton()->storage_buffer_create(sizeof(uint32_t) * 4, Vector<uint8_t>(), RD::STORAGE_BUFFER_USAGE_DISPATCH_INDIRECT); | ||
|
||
cascade.light_position_bufer = RD::get_singleton()->storage_buffer_create(sizeof(HDDAGIShader::Light) * MAX(HDDAGI::MAX_STATIC_LIGHTS, HDDAGI::MAX_DYNAMIC_LIGHTS)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, and light_position_bufer
isn't getting freed
filter_modes.push_back("\n#define MODE_BILATERAL_FILTER\n"); | ||
filter_modes.push_back("\n#define MODE_BILATERAL_FILTER\n#define HALF_SIZE\n"); | ||
filter_shader.initialize(filter_modes, defines); | ||
filter_shader_version = filter_shader.version_create(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filter_shader_version
isn't getting freed
p_render_buffers->create_texture_view(RB_SCOPE_GI, RB_TEX_AMBIENT_U32, RB_TEX_AMBIENT, tv); | ||
p_render_buffers->create_texture_view(RB_SCOPE_GI, RB_TEX_REFLECTION_U32, RB_TEX_REFLECTION, tv); | ||
p_render_buffers->create_texture_view(RB_SCOPE_GI, RB_TEX_REFLECTION_U32_FILTERED, RB_TEX_REFLECTION_FILTERED, tv); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ambient
, reflection
, and reflection_filtered
result in ERROR: Attempted to free invalid ID
spam when RenderSceneBuffersRD::cleanup
processes named_textures
. I'm still getting familiar with the code but I'm guessing it has to do with the view being valid but the underlying shared texture being freed already, even though it does check p_named_texture.texture.is_valid()
?
This occurs because GI is rendered at half resolution by default with this PR, which isn't the case in Try comparing |
I thought this couldnt happen, but i was wrong. |
I'm not sure if I understand the max distance correctly but wanted to check with you just to be sure, I tried different values to see what the result is and it seems that at certain larger values it renders less than lower values and higher values don't follow the same pattern? Apart from that if you don't mind me asking, the darkes part of the shadow is the 0 cascade and there's no way to influence how far that renders, it's a hardcoded limit, is that correct (no setting changed that on my side so I'm just making sure that's how it's supposed to work)? |
Cascades is basically LOD amounts, the lower the less LOD amounts, Cascade 0 has the min cell size, and with each cascade the number doubles, same with the distance, till it reaches the max amount |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
We don't know much, but TLDR of the entire situation is that Juan is basically the only one working on realtime GI in Godot because it's actually a very difficult field and much like physics, you don't actually get any drive by contributors for something so complex. So progress on realtime GI in Godot is dependent on whether Juan has the free time to work on it. (I'm not sure but I think currently he's busy on W4 stuff) He's apparently thought of an approach that's even better than HDDAGI, and will eventually work on that instead of continuing HDDAGI. This PR is likely slated to be marked draft and comments closed off, to prevent people asking for progress on something that's meant to be more "it'll be done when it's done". But I'm not a member of the Godot production team so don't quote me on that. 😅 |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
Alright folks, as promised, I am locking this thread because it seems to attract too much off-topic discussion. As a reminder Github is a place where contributors work and we want to keep this a productive space. Coming in here to pester contributors by asking for ETAs, or to discuss unrelated topics is harmful to our ability to get work done. Improving SDFGI is something that is important to all of us. But it is not a high enough priority that reduz will drop everything else he is working on to finish it. This means the timeline for completing this is uncertain at best and depends on when reduz has time and energy to put towards it. If someone else wants to take over the work to speed up the timeline, they are welcome to. But until someone volunteers to take over the work, or reduz finds the to properly finish this, this comment section will remain locked. Please direct your energy towards something more productive. |
Supersedes #86007
This is a new global illumination system that is a full upgrade to SDFGI Key advantages are:
It is meant as a drop-in replacement, so games using SDFGI should get HDDAGI transparently.
TODO:
TODO AFTER MERGING:
Improvement Screenshots
Motion performance:
By far the biggest improvement is the performance when moving the camera. This is what makes SDFGI unusable on lower end hardware and can even push down the FPS very strongly on higher end hardware. This is because SDFGI needs to regenerate the entire SDF cascade using a jump flood even if it moved just a bit. HDDAGI uses HDDA so it only does local updates to the cascade and no scrolling.
Here is what happens in SDFGI when the camera moves fast:
sdfgi_motion.mp4
As you can see FPS take a large dip. This really sucks for any kind of games where the camera moves fast, like a racing game, a fast shooter, etc.
In comparison, HDDAGI is unaffected by camera motion:
hddagi_motion.mp4
Static performance:
When rendering static (camera not moving) HDDAGI is also considerably faster than SDFGI:
This is SDFGI, the first set of dispatches is the light update, the second is the probe raytracing. On a Geforce 1650 it takes 2.43ms:
In HDDAGI, the same task takes 0.93ms. While not a great difference on a 1650, this makes a much larger difference on IGP:
Quality of Indirect Light
SDFGI does not properly do light conservation, while HDDAGI does. Here is comparison screens by @Jamsers:
SDFGI: Notice light is more uneven in general, and there are some color issues due to the use of spherical harmonics.
HDDAGI: Notice light is better, more evenly distributed:
Quality of reflections
SDFGI uses a very hacky way to obtain light from the SDF that result in very strange lighting from the reflections, plus the SDF gives it a very weird look (this is what you see in the reflections):
In contrast, HDDAGI filters voxels properly, so what is seen in reflections is more faithful to the actual geometry:
What is being reflected:
Here you can see in a more realistic scenario how close the reflections are to the actual geometry (note tonemapping is different in the reflection as this is a debug mode):
In motion
SDFGI suffers from jumping dark spots when lights or camera move. HDDAGI has a special probe filtering option (enabled by default) that gets rid of them.
SDFGI (notice the jumping dark spots):
filter_probes_light_off.mp4
HDDAGI (notice the general smoothness):
filter_probes_light_on.mp4
SDFGI (with a proper scene, note jumping dark spots on the tunnel
filter_probes_off.mp4
HDDAGI with filter probes:
filter_probes_on.mp4
Production edit: closes godotengine/godot-roadmap#32, closes #41154, closes godotengine/godot-proposals#3024