Added support for visibility setting on Directional Lights

[lsemp3d]

Note: This is a Draft PR, feedback on the approach is welcome, I will have to apply the same paradigm to the other light types, so I want to make sure this is a good direction. Ideally, lights should've derived from the same base light data, but unfortunately it appears each light type was implemented independently, this means I will have to replicate this code accordingly.

What does this PR do?

Adds support to set Directional Lights visible through an editor setting and through script.

2024-04-20.17-46-41.mp4

Tested on Windows and Linux

Closes #17738

[moudgils]

In the video why does it look like disabling GI also disables the light as well?

[moudgils]

I think you may have missed uploading the shader file related to this.

[lsemp3d]

In the video why does it look like disabling GI also disables the light as well?

It does, that's the behavior I noticed before these changes. Do you happen to know how to visualize the light when GI is off?

[lsemp3d]

If this approach is acceptable I will begin adding support for the other light types

[moudgils]

If this approach is acceptable I will begin adding support for the other light types

I dont see the shader changes related to this change. Where is m_isVisible added to the shader for directionalLight and how is it used?

[lsemp3d]

If this approach is acceptable I will begin adding support for the other light types

I dont see the shader changes related to this change. Where is m_isVisible added to the shader for directionalLight and how is it used?

the visibility determination is done on the CPU and not in the shader, in order to propagate the component property m_isVisible it needed to be added to the LightData structure. m_isVisible will be available in the shader but it is unused. I would've preferred a more straightforward approach, however, given that lights are managed in the feature processor and accessed through handles, I needed to ensure the configuration got set on a per-light basis which meant I needed to follow the same paradigm used by the other properties like m_affectsGI

[akioCL]

If this approach is acceptable I will begin adding support for the other light types

I dont see the shader changes related to this change. Where is m_isVisible added to the shader for directionalLight and how is it used?

the visibility determination is done on the CPU and not in the shader, in order to propagate the component property m_isVisible it needed to be added to the LightData structure. m_isVisible will be available in the shader but it is unused. I would've preferred a more straightforward approach, however, given that lights are managed in the feature processor and accessed through handles, I needed to ensure the configuration got set on a per-light basis which meant I needed to follow the same paradigm used by the other properties like m_affectsGI

There's no need to add the m_isVisible to the LightData structure. You can change the IndexedDataVector for a MultiIndexedDataVector, you can just add a new attribute. This attribute would be per light, and you can just access it with m_lightData.GetData<1>, where 1 would be the index of the new attribute. PointLights, DiskLights and SimpleSpotlights already use a MultiIndexedDataVector. DirectionalLights and SimplePointlights use IndexedDataVector that would need to be replaced.

[akioCL]

This code is not really disabling any lights. What you are doing is just disabling the effect on GI, but the light is still being used. For disabling you have 2 options: either you remove the lightdata from the m_lightData (like calling ReleaseLight) so it's not in the lightdata vector or you add the m_isVisible attribute to the lightdata, and then modify the shaders to ignore the light if it has the m_isVisible = false. You have to keep in mind that we would need to modify the ray tracing shaders too, and I think those are saved as precompiled shaders (due to confidentiality issues).

[moudgils]

This code is not really disabling any lights. What you are doing is just disabling the effect on GI, but the light is still being used. For disabling you have 2 options: either you remove the lightdata from the m_lightData (like calling ReleaseLight) so it's not in the lightdata vector or you add the m_isVisible attribute to the lightdata, and then modify the shaders to ignore the light if it has the m_isVisible = false. You have to keep in mind that we would need to modify the ray tracing shaders too, and I think those are saved as precompiled shaders (due to confidentiality issues).

Agreed. Btw modifying m_lightData via AcquireLight/ReleaseLight on the cpu is expensive and probably not recommended. I would go with option 2 and can help with RT shaders.

[moudgils]

m_affectsGI

FYI - The reason why m_affectsGI is not searchable in the existing shaders as it is used within precompiled RT shaders. Basically in those shaders we use m_affectsGI as a boolean to skip gpu workload related to GI calculations.

[lsemp3d]

Thanks for the feedback! It's exactly the guidance I needed! I'll update, and I agree, I'll avoid acquiring/releasing lights, will look at MultiIndexedDataVector first

[hosea1008]

There is already a way to disable the lighting for a light, if the lightingChannels are all 0, in the shader the apply lighting codes would be skipped.

So maybe you can try turning off all the lighting channels to achieve the same result? But it is not a good design to coupling lighting channel with lighting visibility, but you can refer to this PR for applying the shader changes? #17232