Fixed cluster sampling issues for volumetric point and spot lights

[JeroenHoogers]

Objective

• Fixes SpotLight / PointLight artifacts in FogVolume #18371
• Fixes sampling volumetric sampling issues when camera is outside of Fog Volume

Solution

• The problem was that only the point / spot lights that overlap with the first z-slice of the cluster grid were considered in the ray marching algo. In the updated code, I moved the ray marching to the outer loop. Then for each sample, I get the cluster index from the sample's view position, with the index I find all lights contributing to this cluster and add up their light contributions.
• Another issue I found while looking into this issue is that the ray origin for the volumetric tracing was the camera origin, even when the camera was outside the FogVolume. This is now fixed in both directional light and the point / spotlight implementations.

Testing

I tested using the code given in the bug report and adapted it to the latest version of bevy:

use bevy::{
    color::palettes::css::RED,
    core_pipeline::{tonemapping::Tonemapping},
    light::{
        FogVolume, VolumetricFog, VolumetricLight,
    },
    post_process::bloom::Bloom,
    prelude::*,
};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .insert_resource(GlobalAmbientLight::NONE)
        .add_systems(Startup, setup)
        .add_systems(Update, move_camera)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn((
        Transform::from_xyz(-2., 0., 0.),
        SpotLight {
            intensity: 500_000.0, // lumens
            color: RED.into(),
            shadow_maps_enabled: true,
            ..default()
        },
        VolumetricLight,
    ));

    commands.spawn((
        Transform::from_xyz(2., 0., 0.),
        SpotLight {
            intensity: 500_000.0, // lumens
            color: RED.into(),
            shadow_maps_enabled: true,
            ..default()
        },
        VolumetricLight,
    ));

    commands.spawn((
        FogVolume::default(),
        Transform::from_scale(Vec3::splat(35.0)),
    ));

    commands.spawn((
        Camera3d::default(),
        Camera { ..default() },
        Transform::from_xyz(0., 0., 5.),
        Tonemapping::TonyMcMapface,
        Bloom::default(),
        VolumetricFog {
            ..default()
        },
    ));
}

fn move_camera(
    mut query: Query<&mut Transform, With<Camera3d>>,
    input: Res<ButtonInput<KeyCode>>,
    time: Res<Time>,
) {
    for mut transform in query.iter_mut() {
        if input.pressed(KeyCode::ArrowUp) {
            transform.translation.z -= 10. * time.delta_secs();
        }

        if input.pressed(KeyCode::ArrowDown) {
            transform.translation.z += 10. * time.delta_secs();
        }

        if input.pressed(KeyCode::KeyZ) {
            transform.translation.z = 5.;
        }

        if input.pressed(KeyCode::KeyX) {
            transform.translation.z = 130.;
        }
    }
}

---

Showcase

After changes (using code from: #18371):

volumetric_spotlights.mp4

Note: The flickering of the light is due to the regular sampling interval, adding more samples or jittering the ray origin minimizes this problem.

Ray marching when camera is outside of FogVolume (Before):

After:

[mate-h]

This fix has a similar theme to this PR #23406. Upon cross checking they are separate fixes, both needed. the other one is for fog AABB traversal / far-plane intersection.
this one fixes per-sample view position which affects both clustering and the shadow cascades sampled. so worth testing the shadows as well to see.

[mate-h]

I wonder if flipping these loops around between for each cluster and for each step introduces any performance regression. while it's definitely the right approach, this requires recomputing the cluster each step meaning more work per fragment. I wonder how/if we can save this. Optimization could be left for later anyway in a later PR.

[JeroenHoogers]

Thank you for looking at this! You are right, there is likely some overhead related to finding the right cluster slice for every sample. I think a relatively easy optimization would be to cache the cluster-able object indices, computing them only once per cluster instead of for every sample. Then I re-compute the cluster index + object indices as soon as the ray exits the current cluster's z-boundary. I can look into it and try to add it to this PR.

[JeroenHoogers]

I looked into this and came up with a solution that avoids the (expensive) view -> cluster lookups altogether, but it requires some pre-computation:

• One time: pre-compute a view_z_planes[] array storing the min view_z (entry plane) for every cluster z_slice. This buffer only needs to be updated if the projection matrix changes.
• Bind the new view_z_planes[] array as a uniform / storage buffer.

Inside the volumetric_fog fragment shader:

• Compute x and y cluster indices once per fragment (trivial).
• Compute initial z_slice (usually 0, except when the camera is outside of the volume).
• For every sample, check if we intersected one or more z_plane(s) if (P_view.z > view_z_planes[z_slice + 1]) (exit plane)? If yes:
  • Increase z_slice
  • Re-compute cluster index from x,y and z indices using existing function: fragment_cluster_index(p: vec3<u32>, cluster_dimensions: vec4<u32>) -> u32
  • Get the new light indices using updated cluster_index.

This way we only increase the z_slice index as we march, avoiding the expensive logarithmic view_z -> z_slice calculations completely.

Let me know what you think. Also if we want to go ahead with it, should it be part of this PR or a new one?

[mate-h]

Thanks for looking into this - I think it's best if we keep this change in a separate PR and keep the scope of this one small. Since it's a visual bugfix first I'm going to approve this one knowing theres a way forward for performance.

[mate-h]

on the volumetric fog example on mac I see these numbers (doesn't have many lights)
this branch: 7.21ms
main: 7.37ms

for a proper performance comparison I suggest a stress test volumetric fog with many lights in this case 32:

I see a clear performance regression, took 2 measurements per branch

this branch: 192.77ms 186.10ms
main: 160ms 161.77ms

[mate-h]

Posted a branch might help with your testing https://github.com/mate-h/bevy/tree/m/volume-many-lights

[mate-h]

This looks like the correct fix compared to the directional path above. Same discrete Beer–Lambert extinction per step as before, the atmosphere-side is more analytic along the actual path, which could be a separate improvement.

[mate-h]

looks unused, can remove this

[mate-h]

Oh and also thanks for posting this PR, must have been a tough one to track down!