Skip to content

Add spherical harmonics sky #108127

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

darksylinc
Copy link
Contributor

@darksylinc darksylinc commented Jun 30, 2025

Note

We were pressed for time so testing is appreciated (both performance, quality, and stability).

For ambient lighting Godot supports using cubemap arrays vs a single cubemap (controlled by rendering/reflections/sky_reflections/texture_array_reflections). The former gives a lot more quality*, but is expensive. The latter is simply faster and more compatible.

However the "single cubemap" version has two problems:

  1. It's too low quality (it's essentially 2x2 cubemap; that is a total of 64 samples).
  2. It appeared as a performance issue when benchmarking on the Quest 3.

This PR replaces the use of the "single cubemap" version for L1 Spherical Harmonics in hopes of achieving:

  1. Faster performance (claim not challenged / profiled).
  2. Considerably higher quality (the SH collects 1024 samples and has no aliasing / filtering issues due to its mathematical nature) than the single cubemap version.

Evaluating performance in all devices is complex. Bandwidth-limited GPUs are going to prefer the SH version, while ALU-limited GPUs will prefer the cubemap version. Furthermore it is to be expected that the cubemap version could win in very high resolutions (e.g. 4k rendering) due to texture cache effects; specially on Desktop GPUs that are able to hide the texture fetch latency.

With @clayjohn we are considering making the SH version the only version available for everything (i.e. get rid of the cubemap array too!) to:

  1. Increase look consistency between pipelines (PC vs Mobile settings, Clustered vs Mobile).
  2. Decrease maintenance (and shader compilation) complexity.
  3. The user should not have to worry about this detail as there are 3 competing solutions (cubemap arrays, single cubemap, SH) to do the same thing.

Cubemaps arrays are not entirely going away though, because they are still used for specular reflections. This only affects "sky" ambient lighting.

Note

To test this feature you must set rendering/reflections/sky_reflections/texture_array_reflections to false

(*) I'm not exactly sure about quality, because comparing the cubemap array, it's very apparent it's significantly brighter than it should be.

@dsnopek
Copy link
Contributor

dsnopek commented Jun 30, 2025

Thanks!

However, when trying this on Meta Quest 3, the colors come out much darker with this PR:

master (e1b4101) This PR
com oculus vrshell-20250630-092002 com oculus vrshell-20250630-091517

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, ambient lighting appears to be broken on my PC in Forward+ and Mobile when texture array reflections are disabled. It looks correct in master, or when texture array reflections are enabled.

Testing project: test_pr_108127.zip

image

This seems to reflect @dsnopek's above testing where ambient lighting also appears broken (it's effectively pure black).

PC specifications
  • CPU: AMD Ryzen 9 9950X3D
  • GPU: NVIDIA GeForce RTX 5090
  • RAM: 64 GB (2×32 GB DDR5-6000 CL30)
  • SSD: Solidigm P44 Pro 2 TB
  • OS: Linux (Fedora 42)

@darksylinc
Copy link
Contributor Author

darksylinc commented Jun 30, 2025

Testing project: test_pr_108127.zip

The link is broken.

Also does the problem get solved if you go to sh_from_cubemap.glsl and uncomment SH_DEBUG_MODE (and rebuild Godot) ?

// Uncomment this to use the slow version, which will compute everything in one thread.
// This is useful to debug problems when you suspect the issue is in the parallel reduction algorithm.
// #define SH_DEBUG_MODE
// Change it into this:
#define SH_DEBUG_MODE

@dsnopek
Copy link
Contributor

dsnopek commented Jun 30, 2025

Also does the problem get solved if you go to sh_from_cubemap.glsl and uncomment SH_DEBUG_MODE (and rebuild Godot) ?

It makes no difference in my testing on the Meta Quest - everything is still overly dark.

@Calinou
Copy link
Member

Calinou commented Jun 30, 2025

The link is broken.

It works on my end, can you try this instead? https://0x0.st/8UUP.zip
It's the same file reuploaded to another location.

@darksylinc
Copy link
Contributor Author

It works on my end, can you try this instead? https://0x0.st/8UUP.zip
It's the same file reuploaded to another location.

Thanks! The link now works. Maybe it was a temporary error.

I can repro the problem now. The SH is not building if set the Ambient to "Background".

@darksylinc darksylinc force-pushed the matias-spherical-harmonics-sky branch from 44e90e4 to c89f5cd Compare June 30, 2025 17:46
@darksylinc
Copy link
Contributor Author

Pushed a fix! Thanks!

This:

if (ambient_source == RS::ENV_AMBIENT_SOURCE_SKY) {
	sky.copy_spherical_harmonics_to_scene_data(p_render_data->environment, scene_state.uniform_buffers[p_index]);
}

Should've been:

if (ambient_source == RS::ENV_AMBIENT_SOURCE_SKY || ambient_source == RS::ENV_AMBIENT_SOURCE_BG) {
	sky.copy_spherical_harmonics_to_scene_data(p_render_data->environment, scene_state.uniform_buffers[p_index]);
}

@dsnopek could you try again? Sorry & Thanks!

@dsnopek
Copy link
Contributor

dsnopek commented Jun 30, 2025

@dsnopek could you try again? Sorry & Thanks!

Sure!

It's no longer overly dark, but it's a bit too green:

master This PR
com oculus vrshell-20250630-092002 com oculus vrshell-20250630-132450

UPDATE: This is really weird! I've run my test project about 4 times with this PR, and running it for about ~1min on the 4th time, suddenly the greenness went away and everything looked normal. It didn't happen any other time, despite definitely spending at least 1 minute testing it on other occasions. I just restarted the app again, and it went back to being green

@darksylinc
Copy link
Contributor Author

darksylinc commented Jun 30, 2025

I assume the green-ness happens only on device and not in your computer?

If so, I very strongly suspect the green-ness will go away if you define SH_DEBUG_MODE in sh_from_cubemap.glsl and rebuild Godot:

// Uncomment this to use the slow version, which will compute everything in one thread.
// This is useful to debug problems when you suspect the issue is in the parallel reduction algorithm.
// #define SH_DEBUG_MODE
// Change it into this:
#define SH_DEBUG_MODE

Edit: Adding some context. The SH are generated via a Compute Shader and a parallel sum reduction via Shared Memory. Since Compute Shaders, compute inline barriers and the use of Shared Memory have a history of uncovering driver bugs; I wouldn't be surprised the problem is a driver bug.

SH_DEBUG_MODE forces SH generation to happen in a single thread. No inline barriers or shared memory involved. This is much slower, but more likely to succeed.

@dsnopek
Copy link
Contributor

dsnopek commented Jun 30, 2025

I assume the green-ness happens only on device and not in your computer?

I've only been testing on the Meta Quest, and not on my computer.

I just tried on my computer for the first time (unchecking rendering/reflections/sky_reflections/texture_array_reflections), and I'm not getting the greenness - in the editor, at least. Unfortunately, running my project on desktop seems to be completely broken with master at the moment, but I guess that's some other issue for me to debug :-)

UPDATE: Actually, I did end up getting the greenness for a couple seconds in the editor on desktop! Then it went back to normal. I'm not sure how to trigger it. (This was compiled with SH_DEBUG_MODE enabled - I hadn't commented it out again when I built the editor.)

If so, I very strongly suspect the green-ness will go away if you define SH_DEBUG_MODE in sh_from_cubemap.glsl and rebuild Godot

I tried this and, unfortunately, it didn't help on the Meta Quest - it's still very green :-/

@darksylinc
Copy link
Contributor Author

darksylinc commented Jun 30, 2025

Things that come to mind:

  1. Missing barrier (shouldn't happen? that's what the render graph is for).
  2. The env map texture wasn't loaded yet; and the SH never got the chance to get updated again
  3. The scene's UBO is updated slightly different than everything else, because we perform a second copy to overwrite SH values sent from CPU (because SH is calculated on GPU and always kept there). Perhaps some min alignment issue is causing the copy to fail?

Do you have the project so I can test it? I see that you're using godot-4-3d-third-person-controller but it appears to be modified (since the original demo uses Sky instead of Background for ambient setting).

Are there any validation errors when run with --gpu-validation (on the failing device)?

@dsnopek
Copy link
Contributor

dsnopek commented Jun 30, 2025

Do you have the project so I can test it?

I'm using this branch on my fork:

https://github.com/dsnopek/gdquest-tps-demo/tree/standalone-vr-slush

Are there any validation errors when run with --gpu-validation (on the failing device)?

There are some validation errors that I don't usually see:

Validation messages
W 0:00:30:511   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x232b000000232b, Set 1, Binding 0, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22cf00000022cf[Downsampled Radiance Octmap Mip 0  View] format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1261139837060219
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9898903184876331
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9797748115120847, Name "Downsampled Radiance Octmap Mip 0  View"
	Command Buffer Labels - 1
		Label[0] - Downsample radiance map (L81) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:511   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x232d000000232d, Set 1, Binding 0, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22d000000022d0[Downsampled Radiance Octmap Mip 1  View] format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1261139837060219
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9901102208131885
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9798847626748624, Name "Downsampled Radiance Octmap Mip 1  View"
	Command Buffer Labels - 1
		Label[0] - Downsample radiance map (L82) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:516   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x232f000000232f, Set 1, Binding 0, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22d100000022d1[Downsampled Radiance Octmap Mip 2  View] format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1261139837060219
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9903301231387439
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9799947138376401, Name "Downsampled Radiance Octmap Mip 2  View"
	Command Buffer Labels - 1
		Label[0] - Downsample radiance map (L83) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:517   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x23310000002331, Set 1, Binding 0, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22d200000022d2[Downsampled Radiance Octmap Mip 3  View] format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1261139837060219
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9905500254642993
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9801046650004178, Name "Downsampled Radiance Octmap Mip 3  View"
	Command Buffer Labels - 1
		Label[0] - Downsample radiance map (L84) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:517   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x23330000002333, Set 1, Binding 0, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22d300000022d3[Downsampled Radiance Octmap Mip 4  View] format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1261139837060219
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9907699277898547
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9802146161631955, Name "Downsampled Radiance Octmap Mip 4  View"
	Command Buffer Labels - 1
		Label[0] - Downsample radiance map (L85) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:517   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x23350000002335, Set 1, Binding 0, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22d400000022d4[Downsampled Radiance Octmap Mip 5  View] format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1261139837060219
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9909898301154101
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9803245673259732, Name "Downsampled Radiance Octmap Mip 5  View"
	Command Buffer Labels - 1
		Label[0] - Downsample radiance map (L86) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:517   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x23370000002337, Set 1, Binding 0, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22d500000022d5[Downsampled Radiance Octmap Mip 6  View] format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1261139837060219
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9912097324409655
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9804345184887509, Name "Downsampled Radiance Octmap Mip 6  View"
	Command Buffer Labels - 1
		Label[0] - Downsample radiance map (L87) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:517   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x23390000002339, Set 2, Binding 5, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22ca00000022ca format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1288627627754644
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9914296347665209
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9792250556981962
	Command Buffer Labels - 1
		Label[0] - Fast filter radiance (L88) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:517   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x23390000002339, Set 2, Binding 3, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22c800000022c8 format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1288627627754644
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9914296347665209
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9790051533726408
	Command Buffer Labels - 1
		Label[0] - Fast filter radiance (L88) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

W 0:00:30:517   _debug_messenger_callback: VALIDATION - Message Id Number: 20145586 | Message Id Name: Undefined-Value-StorageImage-FormatMismatch-ImageView
	(Warning - This VUID has now been reported 10 times, which is the duplicated_message_limit value, this will be the last time reporting it).
vkCmdDispatch(): the storage image descriptor [VkDescriptorSet 0x23390000002339, Set 2, Binding 2, Index 0] is accessed by a OpTypeImage that has a Format operand Rgba16f (equivalent to VK_FORMAT_R16G16B16A16_SFLOAT) which doesn't match the VkImageView 0x22c700000022c7 format (VK_FORMAT_A2B10G10R10_UNORM_PACK32). Any loads or stores with the variable will produce undefined values to the whole image (not just the texel being accessed).
Spec information at https://docs.vulkan.org/spec/latest/chapters/textures.html#textures-format-validation
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 1288627627754644
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9914296347665209
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 9788952022098631
	Command Buffer Labels - 1
		Label[0] - Fast filter radiance (L88) (Compute){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:639 @ _debug_messenger_callback()

E 0:00:30:519   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10026446533698463
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L89) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:586   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187392
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10066028952298435
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:725   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10066028952298435
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:752   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187392
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10091317719737306
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:785   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10091317719737306
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:814   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187392
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10091317719737306
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:838   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10091317719737306
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:875   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187392
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10091317719737306
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:906   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187408
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10091317719737306
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

E 0:00:30:934   _debug_messenger_callback: VALIDATION - Message Id Number: -836363996 | Message Id Name: VUID-vkCmdDrawIndexed-viewType-07752
	(Warning - This VUID has now been reported 10 times, which is the duplicated_message_limit value, this will be the last time reporting it).
vkCmdDrawIndexed(): the sampled image descriptor [VkDescriptorSet 0x23530000002353, Set 1, Binding 3, Index 0] ImageView type is VK_IMAGE_VIEW_TYPE_CUBE_ARRAY but the OpTypeImage has (Dim = 2D) and (Arrayed = 1).
The Vulkan spec states: If a VkImageView is accessed as a result of this command, then the image view's viewType must match the Dim operand of the OpTypeImage as described in Compatibility Between SPIR-V Image Dimensions and Vulkan ImageView Types (https://docs.vulkan.org/spec/latest/chapters/drawing.html#VUID-vkCmdDrawIndexed-viewType-07752)
	Objects - 4
		Object[0] - VK_OBJECT_TYPE_COMMAND_BUFFER, Handle -5476376611660187392
		Object[1] - VK_OBJECT_TYPE_PIPELINE, Handle 10091317719737306
		Object[2] - VK_OBJECT_TYPE_DESCRIPTOR_SET, Handle 9942883649987411
		Object[3] - VK_OBJECT_TYPE_IMAGE_VIEW, Handle 67070209294397
	Command Buffer Labels - 1
		Label[0] - Command graph (L10) (Draw){ 1.0, 1.0, 1.0, 1.0 }
  <C++ Source>  drivers/vulkan/rendering_context_driver_vulkan.cpp:642 @ _debug_messenger_callback()

There's some I'm used to seeing, and I think I've filtered those out correctly

@darksylinc
Copy link
Contributor Author

Ok that would definitely explain the problem. I don't know why I'm not seeing them, but I do remember seeing that Godot had different RGB10A2 vs RGBA16F paths for PC and Mobile.

@Calinou
Copy link
Member

Calinou commented Jun 30, 2025

but I do remember seeing that Godot had different RGB10A2 vs RGBA16F paths for PC and Mobile.

To clarify, it's not platform-specific but renderer-specific. Forward+ always uses RGBA16F, while the Mobile renderer always uses RGB10A2 (even on desktop platforms).

@darksylinc
Copy link
Contributor Author

Thanks! That saves me time!

@darksylinc
Copy link
Contributor Author

darksylinc commented Jul 5, 2025

@dsnopek OK fixed!

I was able to repro the bug on an Adreno device. The problem was simply mediump. Removing it fixed the problem. I can't tell if it's a driver bug or a precision issue because the calculations are flying too close to the sun when it comes to 16-bit float so I just upgraded the floats to 32-bit because it's the safe option.

I also tweaked the math to prevent divisions by 0 from happening.

Regarding your validation errors: I didn't look into it; but it became apparent after reading them that the problem was caused by already existing code; so the problem could be on master as well. It's also confusing because Only Mobile uses RGB10A2, while Clustered uses R16F. But only Clustered uses Compute. You didn't say which rendering method you were using.

While looking into this bug, I stumbled into #108317 which can cause problems when generating the cubemap and the SH.

@lander-vr
Copy link
Contributor

Some tests with various HDRIs.

4.5Beta2 PR Eevee Ground truth (Cycles)
image image image image
image image image image
image image image image
image image image image
image image image image

I also noticed this strange dot appearing, regardless of the HDRI:
image
25_07_06_14_05__godot windows editor x86_64_xLpl8lDPsa

There's a significant lack of directionality and contrast in the lighting. Even using high contrast HDRIs it almost looks like just a flat ambient color. Even when the HDRI contains a very strong light source, it doesn't seem to be taken into account.

I'm not entirely sure, but I believe Eevee also uses L1 harmonics (I can spot 3 "bands" of ambient lighting, but my understanding of spherical harmonics is, let's say, nonexistent), maybe this could be a point of reference?

@darksylinc
Copy link
Contributor Author

Do you have a link for the 3rd (starting from the top) HDRI? I want to take a look at that one since it looks the flattest and dull when it appears it shouldn't.

The 2nd HDRI looks like a greyish environment with a strong "diagonal" light on the bottom lower corner. If that is the case, that is the worst case for SH, since L1 order is bad at high frequency data (i.e. small & sudden changes in brightness or colour).

Regarding the dot: I can see it too and I am now looking into it.

User Geomerics' SH version
Optimize evaluate_sh_l1_geomerics() function
SH required denormalization
User Fibonacci Sphere sampling for more uniform sampling
@darksylinc darksylinc force-pushed the matias-spherical-harmonics-sky branch from 25ceaf2 to 5709832 Compare July 6, 2025 15:25
@darksylinc
Copy link
Contributor Author

darksylinc commented Jul 6, 2025

OK pushed a fix for the yellow (sometimes purple) dot. Sometimes the length of "dir" was above 1
(e.g. 1.0005739926) causing negative values, which turned into NaNs.

I'll await for that HDRI so I can take a deeper look (I mean this one in case there's confussion):
image

Edit: The reason I ask is because I cannot reproduce that "dull" look you mention of. I am only comparing Array vs SH version (i.e. not against ground truth) since SH is using the same input as Array. It looks slightly different, yes; but not as dull and with lack of contrast like your results. I want to use the same HDRIs to understand what's going on and whether it's a math problem, a limitation of the technique, or a software bug (i.e. your machine for some reason is incorrectly calculating the SH).

@lander-vr
Copy link
Contributor

lander-vr commented Jul 6, 2025

Here are all the HDRIs I used.
https://polyhaven.com/a/empty_play_room
https://polyhaven.com/a/moon_lab
https://polyhaven.com/a/fireplace
https://polyhaven.com/a/tv_studio

And in case it's useful to you in any way, here's my specs:
Godot v4.5.beta2 - Windows 10 (build 19045) - Multi-window, 2 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 (NVIDIA; 32.0.15.6094) - AMD Ryzen 5 5600X 6-Core Processor (12 threads) - 31.92 GiB memory

@@ -418,6 +425,8 @@ void SkyRD::ReflectionData::create_reflection_fast_filter(bool p_use_arrays) {
}
}
RD::get_singleton()->draw_command_end_label(); // Filter radiance

copy_effects->calculate_sh_from_cubemap(downsampled_radiance_cubemap, sh_coeff_buffer);
Copy link
Member

Choose a reason for hiding this comment

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

Using the downsampled_radiance_cubemap might partly explain why the SH version is missing highlights. It uses either a half res cubemap (when using the "high quality" path) or a 64x64 cubemap (when using the "fast filter" path). In either case its a lower res cubemap that is already blurred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was a bit confused on where I should grab the texture from, I thought I got it from the right place (resolution is 128x128 IIRC) but it's definitely possible that it's wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch that perhaps the problem is simply the wrong input.

@clayjohn
Copy link
Member

clayjohn commented Jul 6, 2025

I'm not entirely sure, but I believe Eevee also uses L1 harmonics (I can spot 3 "bands" of ambient lighting, but my understanding of spherical harmonics is, let's say, nonexistent), maybe this could be a point of reference?

Eevee looks like it is using L2 spherical harmonics. Here is a reference that compares L1 to L2.

https://therealmjp.github.io/images/sss/sh_dirlight_comparison.png

Notice how the best you get with L1 is basically a smooth gradient, while with L2 you can get a clear band. L2 is much better at approximating directional lighting as a result.

However, the question we have to ask is whether we should be using environment maps to encode directional lighting. Typically in games you remove the directional lighting from an HDRI before using it since it is better approximated with a DIrectionalLight rather than encoding into the IBL (https://blog.selfshadow.com/publications/s2016-shading-course/unity/s2016_pbs_unity_hdri_notes.pdf pg 71 and https://seblagarde.wordpress.com/wp-content/uploads/2015/07/course_notes_moving_frostbite_to_pbr_v32.pdf pg 59). In Godot, this has been a long standing issue because the HDRIs you download online from places like Polyhaven do not remove the directional lighting (the bright highlight from the sun), and users expect to be able to drop in the HDRI directly and have it look as good as in Blender. Which, isn't really feasible to do in real time.

Also note, Filament appears to use L2 when encoding to SH (https://google.github.io/filament/Filament.html#lighting/imagebasedlights/distantlightprobes),

@darksylinc
Copy link
Contributor Author

Notice how the best you get with L1 is basically a smooth gradient, while with L2 you can get a clear band. L2 is much better at approximating directional lighting as a result.

I haven't looked at the provided HDRIs yet, but one of the reasons I want to take a look is that I can hack the 1st order multiplier and you get much higher contrast and directionality. While it's not PBR-correct, artistically it looks very pleasant and satisfying to the eye. Thus I want to explore that a bit to see if it's worth it.

@michaelharmonart
Copy link

the 9 coefficients for L2 SH is still far less than even the minimum 24 pixels of the 2x2 per face cubemaps that could be used currently.

happy to test performance with both L1 and L2 for my VR app, if performance is the concern

@clayjohn
Copy link
Member

clayjohn commented Jul 6, 2025

the 9 coefficients for L2 SH is still far less than even the minimum 24 pixels of the 2x2 per face cubemaps that could be used currently.

happy to test performance with both L1 and L2 for my VR app, if performance is the concern

Its not the storage size that we are worried about for L2 (since we are storing the cubemap anyway) Its the cost of reconstruction. L2 requires more than 2x the number of instructions to reconstruct compared to L1. Therefore it is significantly slower than L1.

@darksylinc
Copy link
Contributor Author

darksylinc commented Jul 6, 2025

the 9 coefficients for L2 SH is still far less than even the minimum 24 pixels of the 2x2 per face cubemaps that could be used currently.

To explain it in layman terms, Spherical Harmonics is much like lossy-compressing a picture in JPG. It uses math to encode color (in fact JPG is very similar because both SH and JPG work & compress in the frequency domain).

However that means reconstructing with higher fidelity means evaluating more math operations, thus at L2 we need a ton of muls and additions, despite having "just" 9 coefficients per component.

A cubemap may only store 24 pixels. (6x2x2), but it is very fast because at any time we only need to lookup 1 pixel and 3 more of its neighbours (to perform bilinear interpolation). This is cheap because the cubemap stays decompressed at all times.

But just like 12kb of JPG can easily store 200kb of BMP data or more, "Just" 4 coefficients of SH L1 compress a lot more than 24 pixels.

@michaelharmonart
Copy link

i forgot that the ALU costs for higher order SH are what they are. I'm almost always drawcall or texture sample bottlenecked for webXR so it's not something I think of as much.

Maybe if SH are planned to entirely replace cubemaps for ambient lighting, there could be an option to pick between L1 and L2, since there may be cases where the sky lighting has enough directional information to make L2 worth it

@clayjohn
Copy link
Member

clayjohn commented Jul 6, 2025

I was experimenting with the moon lab HDRI from @lander-vr and noticed that there may be additional issues with the HDRIs that we just aren't handling right now.

I tried enabling process/hdr_clamp_exposure in the import settings for the HDRI to see what the impact would be. And contrary to my expectations clamping the range of the HDRI made it brighter

process/hdr_clamp_exposure = false
Screenshot 2025-07-06 at 2 57 21 PM

process/hdr_clamp_exposure = true
Screenshot 2025-07-06 at 2 57 13 PM

I suspect this is due to an overflow at some point in the import / rendering process

process/hdr_clamp_exposure = false
Screenshot 2025-07-06 at 3 01 06 PM

process/hdr_clamp_exposure = true
Screenshot 2025-07-06 at 3 01 15 PM

Note: All these screenshots are taken with Beta 1

edit: Okay I confirmed, the darkening is from pixel overflow. There is a single pixel that maps to INF when the sky texture is reduced to RGBA16

@darksylinc
Copy link
Contributor Author

OK I took a look at the HDRi provided.

At a glance it looks like simply a limitation of SH L1.

However directionality can be boosted (it's no longer PBR-correct though). Boosting directionality can brighten or darken the lighting.

Normal SH

SH
SH02

2x Boosted SH

SH_directionality_boosted
SH_directionality_boosted02

Adding a slider to boost directionality can give artists more control.

@lander-vr
Copy link
Contributor

lander-vr commented Jul 6, 2025

However, the question we have to ask is whether we should be using environment maps to encode directional lighting. Typically in games you remove the directional lighting from an HDRI before using it since it is better approximated with a DIrectionalLight rather than encoding into the IBL

An important thing to consider is that ambient light sources themselves often do have lots of directionality, as they usually don't cover a hemisphere (let alone cover a hemisphere uniformly), and I'm afraid L1 SH simply wont be able to deal with the slightest contrast, even row 2 and 5 in my comparison table seem like reasonable amounts of directionality that I feel like should be able to be conveyed. That isn't really something you can tackle using directional lights.

I imagine L1 wouldn't be able to really deal with even a sunset sky, where you'd want to differentiate between the warmer ambience from the sunset side, reflected light from the ground, and the dark side on the "night" side of the sky.

Example from a sunset HDRI:

EEVEE PR
image image

I'd argue exploring L2 SH could still be worth it, and keeping L1 SH as an option as @michaelharmonart suggested, similar to the current cubemap arrays vs a single cubemap option, because while we are getting an extremely subtle gradient with L1 SH, practically speaking it's pretty close to just a solid color ambience.

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.

7 participants