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

Vulkan: LightmapGI does not take custom sky rotation into account #56135

Closed
Tracked by #56033
Calinou opened this issue Dec 21, 2021 · 1 comment · Fixed by #95000
Closed
Tracked by #56033

Vulkan: LightmapGI does not take custom sky rotation into account #56135

Calinou opened this issue Dec 21, 2021 · 1 comment · Fixed by #95000

Comments

@Calinou
Copy link
Member

Calinou commented Dec 21, 2021

Related to #56079.

Godot version

4.0.dev (489f11e)

System information

Fedora 34, GeForce GTX 1080 (NVIDIA 470.74)

Issue description

LightmapGI does not take custom sky rotation into account.

This works in 3.4.1.stable and 3.5.beta (f2ddafd), so this is a regression in master.

Default sky rotation

Note that lighting comes from the bottom due to #56079. Regardless, changing the sky orientation doesn't affect the rotation of environment lighting in the slightest.

2021-12-21_18 27 32

Custom sky rotation

Sky rotation in the background does not reflect the sky rotation taken into account by the generated (ir)radiance map due to #56136. Nonetheless, the sky is still rotated in the generated (ir)radiance map.

2021-12-21_17 39 47

Steps to reproduce

  • Add a WorldEnvironment node with a PanoramaSkyMaterial. Adjust the sky's rotation in the resource options (try (0, 0, 1.57)).
  • Add a DirectionalLight and set its Energy to 0, so that the preview light is disabled and it does not intefere with baking.
  • Add a LightmapGI node. Set the environment mode to Scene.
  • Instance two identical 3D scenes imported with the Static Lightmaps option. Right-click one of them in the Scene tree dock then enable Editable Children. Select its child mesh and set Global Illumination > Mode to Disabled, so that it is not included in baking. This step is not required, but it's useful to compare real-time ambient lighting with baked ambient lighting.
  • Select the LightmapGI node and click Bake at the top of the 3D editor viewport.

Minimal reproduction project

test_lightmap_environment_custom_orientation_master.zip

@permelin
Copy link
Contributor

permelin commented Mar 1, 2024

Admittedly, I'm in over my head here, but this seems to do the trick. I'm absolutely not 100% sure about the second change, but it feels correct if we're to bring the ray into the local space of the sky. And without it the light is always off by 180° around the y axis.

diff --git a/scene/3d/lightmap_gi.cpp b/scene/3d/lightmap_gi.cpp
index 86ff6d15dd..182c2124fa 100644
--- a/scene/3d/lightmap_gi.cpp
+++ b/scene/3d/lightmap_gi.cpp
@@ -1031,6 +1031,7 @@ LightmapGI::BakeError LightmapGI::bake(Node *p_from_node, String p_image_data_pa
 
 					if (env.is_valid()) {
 						environment_image = RS::get_singleton()->environment_bake_panorama(env->get_rid(), true, Size2i(128, 64));
+						environment_transform = Basis::from_euler(env->get_sky_rotation());
 					}
 				}
 			} break;
diff --git a/modules/lightmapper_rd/lm_compute.glsl b/modules/lightmapper_rd/lm_compute.glsl
index 1d088450e9..f75d8a9b91 100644
--- a/modules/lightmapper_rd/lm_compute.glsl
+++ b/modules/lightmapper_rd/lm_compute.glsl
@@ -460,7 +460,7 @@ void trace_direct_light(vec3 p_position, vec3 p_normal, uint p_light_index, bool
 #if defined(MODE_BOUNCE_LIGHT) || defined(MODE_LIGHT_PROBES)
 
 vec3 trace_environment_color(vec3 ray_dir) {
-	vec3 sky_dir = normalize(mat3(bake_params.env_transform) * ray_dir);
+	vec3 sky_dir = normalize(ray_dir * mat3(bake_params.env_transform));
 	vec2 st = vec2(atan(sky_dir.x, sky_dir.z), acos(sky_dir.y));
 	if (st.x < 0.0) {
 		st.x += PI * 2.0;

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

Successfully merging a pull request may close this issue.

4 participants