[Merged by Bors] - Take DirectionalLight's GlobalTransform into account when calculating shadow map volume (not just direction)

[coreh]

Objective

This PR fixes #5789, by enabling movable (and scalable) directional light shadow volumes.

Solution

This PR changes ExtractedDirectionalLight to hold a copy of the DirectionalLight entity's GlobalTransform, instead of just a direction vector. This allows the shadow map volume (as defined by the light's shadow_projection field) to be transformed honoring translation and scale transforms, and not just rotation.

It also augments the texel size calculation (used to determine the shadow_normal_bias) so that it now takes into account the upper bound of the x/y/z scale of the GlobalTransform.

This change makes the directional light extraction code more consistent with point and spot lights (that already use transform), and allows easily moving and scaling the shadow volume along with a player entity based on camera distance/angle, immediately enabling more real world use cases until we have a more sophisticated adaptive implementation, such as the one described in #3629.

Note: While it was previously possible to update the projection achieving a similar effect, depending on the light direction and distance to the origin, the fact that the shadow map camera was always positioned at the origin with a hardcoded Vec3::Y up value meant you would get sub-optimal or inconsistent/incorrect results.

---

Changelog

Changed

• DirectionalLight shadow volumes now honor translation and scale transforms

Migration Guide

• If your directional lights were positioned at the origin and not scaled (the default, most common scenario) no changes are needed on your part; it just works as before;
• If you previously had a system for dynamically updating directional light shadow projections, you might now be able to simplify your code by updating the directional light entity's transform instead;
• In the unlikely scenario that a scene with directional lights that previously rendered shadows correctly has missing shadows, make sure your directional lights are positioned at (0, 0, 0) and are not scaled to a size that's too large or too small.

[HackerFoo]

Shouldn't the largest dimension be calculated after the transform?

let directional_light_texel_size = transform.radius_vec3a(Vec3A::new(
  directional_light.shadow_projection.right - directional_light.shadow_projection.left,
  directional_light.shadow_projection.top - directional_light.shadow_projection.bottom,
  0.
)).abs().max_element() / directional_light_shadow_map.size as f32;

[coreh]

Nice catch. Updated to use this approach, should give more accurate results.

[rparrett]

I can't review the math, but I've been testing with a modified lighting.rs that moves the whole scene around and this seems to work.

It would be great if these new behaviors were documented on DirectionalLight.

I do agree that even if this isn't the ultimate solution, it would be a huge win over the status quo.

[rparrett]

Pinging @ramirezmike

[coreh]

It would be great if these new behaviors were documented on DirectionalLight.

Added a section on directional light shadows, documenting how the volume (and shadow maps) work.

[rparrett]

I have been testing in this little "directional light sandbox" and this seems to be working well.

An interesting note is that prior to this PR, you can modify the translation of a DirectionalLight, and even the lighting example uses a directional light in a non-zero position. But it's not clear at all what the behavior is supposed to be. When you move the light, the shadow projection is affected, but not in any way that would seem to make sense. There may be a bug there. If so, this seems to fix it.

I'm really happy about the new docs as well.

[ramirezmike]

I checked out this PR locally and updated one of my games to use it and it works like a charm. My game has a system to update the camera's transform based on the player's transform and I just added DirectionalLight to that query filter and now the shadows work regardless of how far I'm from the origin.

I also can't comment too much on the math involved but the parts I grasp make sense and the docs are great 👍

I'll continue testing this out but pretty happy with it so far.

[rparrett]

I tested a bit more in the playground above with modifying the shadow projection directly and it seems like the behavior is the same before/after this PR. (when the scale is 1.)

[alice-i-cecile]

Very nice new docs, fixes an important limitation, reviewers seem to have thoroughly tested this. Added to the milestone.

[rparrett]

@superdump Do we need to adjust DEFAULT_SHADOW_NORMAL_BIAS now? Playing in the shadow_biases example, it seems like an equivalent value to the current default would now be ~0.9

[cart]

Code looks good, but I'm conflicted on this one. This is a useful intermediate step and solves a real problem, but it is also training people to think about DirectionalLights in a non-standard way (and write code that operates under that assumption). Everywhere else in the ecosystem (ex: Godot, Blender, etc), DirectionalLight position has no bearing on how light / shadows work. I would have preferred a solution that maintains that mental model, but I also understand that the approach in this PR is way more straightforward to implement.

That being said, theres enough positive sentiment here to get me on board. This just increases the pressure to get shadow cascades / view frustum fitting working, which are sorely needed already!

[superdump]

@cart - the approach that I implemented a while back was to create an oriented bounding box around the view frustum of the camera and adjust the light's shadow projection based on that. I abandoned it because the shadow map quality quickly degrades with the size of the frustum.

[coreh]

Merged and fixed the conflicts. The system now seems to split ShadowView and GpuDirectionalLight extraction in two separate loops, I believe from #6066. That actually paired nicely with this, the code is overall a bit cleaner.

I tested it and things seem to be working properly with multiple directional lights.

This is a useful intermediate step and solves a real problem, but it is also training people to think about DirectionalLights in a non-standard way (and write code that operates under that assumption).

😕 Yeah. Please let me know if you'd like me to add a note to the docs explicitly stating that things might change in the future.

Do we need to adjust DEFAULT_SHADOW_NORMAL_BIAS now? Playing in the shadow_biases example, it seems like an equivalent value to the current default would now be ~0.9

Just tried out the example, is it really supposed to look like this?

Tried it out in main and it looks the same, so it probably isn't a result of this PR:

Or is that what you mean? That we need to tweak the values somehow so that it doesn't look like this?

[rparrett]

As far as I know, that example was used to determine the current values for the default shadow bias constants.

I believe the idea is that you use the keys to adjust the biases individually until the shadows don't look bad and then use that value for the default.

But that's what I was seeking clarity from superdump about.

[superdump]

@superdump Do we need to adjust DEFAULT_SHADOW_NORMAL_BIAS now? Playing in the shadow_biases example, it seems like an equivalent value to the current default would now be ~0.9

Why does the normal bias need changing? What changed to need to change it? 🙂 I need to look more closely...

[rparrett]

Why does the normal bias need changing? What changed to need to change it? 🙂 I need to look more closely...

The sqrt business, I think.

[superdump]

@coreh yeah the shadow biases example is meant to start off looking bad with lots of shadow artifacts so you know what they look like and can learn how the biases impact them.

[superdump]

Why does the normal bias need changing? What changed to need to change it? 🙂 I need to look more closely...

The sqrt business, I think.

We changed from sqrt(2) * max(x, y) to sqrt(x^2 + y^2) practically I think. The sqrt(2) came from assuming square fragment aspect ratio so e.g. x == y and then sqrt(x^2 + x^2) == sqrt(2 * x^2) == sqrt(2) * x == sqrt(2) * max(x, x). I feel like that shouldn't have had a significant impact in default cases. Perhaps it or something else I missed does though.

[rparrett]

the shadow projection in shadow_biases is very not square.

[rparrett]

The current 0.6 default still seems like a good value in the lighting example. Is it expected for users to need to tune these biases when the projection shape changes?

I just noticed the different behavior in shadow_biases where it seemed likely the current defaults were determined (because pre-this-PR the current defaults perfectly eliminate artifacts there) and thought they might need to be updated.

Really don't want to derail this if it's very unlikely to affect normal users though.

[cart]

I'm down to merge this. Changing shadow bias behavior (for a given value) should be avoided unless absolutely necessary as artists will carefully tune the value to their specific scenes. It seems like doing it here was necessary though. And changing the default value is less destructive than changing the algorithm, so I'm happy to change that later if we later find a better value.

[cart]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• build-without-default-features (bevy)
• build-without-default-features (bevy_ecs)
• build-without-default-features (bevy_reflect)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12