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

Prevent very far away point lights from being culled #98641

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

Conversation

Flarkk
Copy link
Contributor

@Flarkk Flarkk commented Oct 29, 2024

Very far away Omni and Spot lights get culled and can't contribute to the scene's lighting even though their radius is large enough.

This is due to several floating point precision issues this PR resolves :

  1. The light clustering logic computes the squared length of the light position in view space. This involves squaring its components which generates INFINITY if one of them is greater than sqrt(MAX_REAL_T_VALUE) = 1.8446744x10^19 with 32 bit floats). Consequently the light's sphere of influence is always considered behind the near plane or beyond the far plane and it gets culled.
  2. In the compatibility renderer, a similar issue arises when the length of the light position in view space is computed and compared to the fadeoff distances. The length obtained is then INFINITY and the light gets culled against the fade far distance.
  3. In scene and fog shaders, the distance from the light to the vertex is computed. Again, it involves squaring the vector's components, which generate INFINITY when they're greater than SQRT(MAX_FLOAT_VALUE). Consequently the light gets a zero attenuation factor and a zero light vector, therefore doesn't affect the fragment.

This PR solves all 3 above issues issues 1. and 3. (the compatibility renderer is left unchanged) by scaling down the light position in view space by its L1-norm + 1 before getting the length, then scales it back.

The scene below is lit by two omni lights positioned behind the camera at approx x = y = z = 1E30.

With this PR Without
Capture d’écran du 2024-10-29 14-48-53 Capture d’écran du 2024-10-29 14-43-56

FAQ

What about Directional lights ?
There is no such issue for Directional lights, as no distance computation is involved (only the light direction plays a role, and it's a unit vector).

What's the cost of this change in the scene fragment shader ?

The length correction fix is computed once per point light 
--> +3 abs() +3 add() +2 mul() per point light

... and once per point light with shadows 
--> +3 abs() +3 add() +2 mul() per point light with shadows

1 to 2 `normalize()` per point light are replaced by a less costly division by the length :
+-> -1 to -2 `normalize()` per point light
--> +1 to +2 mul() per point light

Is it really worth it ?
This small tweak unlocks using omni and spot lights in the range [1E19; 1E38] in single precision, speaking of attenuation radius and distance to origin, in addition to the commonly used [0; 1E19] range. This extended range is especially useful in space scenes and other extremely large environments.

Why not just using the double precision build ?
The double precision build can't solve the problem in the scene shader. It would also be a much more resource intensive alternative to solve a simple problem : being able to use the lights across the whole value range of 32 bits floating point, not only the first half of it.

It's not a surprise that rendering very deep scenes raise many numerical precision issues
Absolutely, and this is what I'm into ! I started digging into this rabbit hole a while ago, and also stumbled upon #95944 and #98610 so far. There will be likely more to come

@Flarkk Flarkk requested a review from a team as a code owner October 29, 2024 15:05
@Flarkk Flarkk changed the title Prevent very far away lights from being clipped away Prevent very far away lights from being clipped out Oct 29, 2024
@Chaosus Chaosus added this to the 4.4 milestone Oct 29, 2024
@Flarkk Flarkk changed the title Prevent very far away lights from being clipped out Prevent very far away point lights from being clipped out Oct 29, 2024
@clayjohn
Copy link
Member

I'm not sure about forcing this change on everyone. It's great for space games that want a long view distance, but aren't willing to pay the cost of double precision. But for everyone else its a performance cost for no gain and for engine maintainers its additional complexity and maintenance burden.

Our fragment shaders are already instruction bottlenecked. So this would have a very real cost for many games who won't benefit from it.

The CPU code is less problematic as there likely won't be any performance regression.

@Flarkk
Copy link
Contributor Author

Flarkk commented Oct 30, 2024

Yes I'm very aware of the performance bottleneck of fragment shaders, and I get your point.

To clarify one thing : this issue prevents anyone from having long ranging lights working in space games and alike, including people using double precision (it doesn't help in shaders).
In other words Godot doesn't support long ranging point lights at all right now, which can be a hurdle for space games.

I have a couple of ideas to further reduce the performance cost and maintenance burden :

  • Leave the compatibility renderer unchanged. Apply this PR only to forward clustered and mobile
  • Pack the modified length operation into a macro or a function like LENGTH_SAFE() for readability

Will update the PR and see if it looks more acceptable. Feedback appreciated 🙏

@Flarkk Flarkk force-pushed the clipped_far_lights branch 2 times, most recently from c3ff473 to ed704ad Compare October 30, 2024 09:34
@fire
Copy link
Member

fire commented Oct 31, 2024

Can you clarify which max values in meters would start having problems?

@Flarkk
Copy link
Contributor Author

Flarkk commented Oct 31, 2024

Problems arise when :

  • the light itself is very far away from the camera : it has one or more coordinates greater than ~1.84e+19m in view space (culling issue)
  • and/or the lit vertex is very far from the camera : vector from light to vertex has one or more coordinates greater than ~1.84e+19m in view space (lighting issue in the fragment shader)

To give a comparison 1.84e+19m is approximately 1/1000th the size of the Milky Way. The issue would become a problem in the cases you want for example to render solar systems, nebulas or galaxies with real sized units.

@Flarkk Flarkk changed the title Prevent very far away point lights from being clipped out Prevent very far away point lights from being culled Nov 11, 2024
@Flarkk Flarkk force-pushed the clipped_far_lights branch from ed704ad to 59e0858 Compare November 27, 2024 14:07
@Flarkk
Copy link
Contributor Author

Flarkk commented Nov 27, 2024

Rebased on top of #94981

@Flarkk Flarkk force-pushed the clipped_far_lights branch from 59e0858 to 930d9f3 Compare December 15, 2024 17:43
@Flarkk
Copy link
Contributor Author

Flarkk commented Dec 15, 2024

Rebased

@Repiteo Repiteo modified the milestones: 4.4, 4.5 Feb 24, 2025
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.

5 participants