Attaching a PointLight2d component to an entity with a SpriteBundle causes the light not to render when the sprite is off-screen

[isidornygren]

Quite a wordy title, but should be reproducible by:

1. Creating an entity with a SpriteBundle component, rendering a sprite
2. Attaching a PointLight2d to it (with a radius larger than the sprite)
3. Either moving the sprite off-camera, or the camera away from the sprite until the sprite is just outside the screen, which should cause the light not to be rendered.

As an example, I've moved the PointLight2d component in the dungeon example into the sprite entity:

fn spawn_candles(mut commands: Commands, spritesheet: Res<CandleSpritesheet>) {
    // let light = commands
    //     .spawn((PointLight2dBundle {
    //         transform: Transform::from_xyz(0.0, 4.0, ENTITY_INDEX),
    //         point_light: PointLight2d {
    //             radius: 48.0,
    //             color: Color::Srgba(YELLOW),
    //             intensity: 2.0,
    //             falloff: 4.0,
    //             ..default()
    //         },
    //         ..default()
    //     },))
    //     .id();

    commands.spawn((
        Candle,
        AnimationTimer(Timer::from_seconds(0.2, TimerMode::Repeating)),
        SpriteBundle {
            transform: Transform::from_xyz(0., 2., ENTITY_INDEX),
            texture: spritesheet.texture.clone(),
            ..default()
        },
        PointLight2d {
            radius: 48.0,
            color: Color::Srgba(YELLOW),
            intensity: 2.0,
            falloff: 4.0,
            ..default()
        },
        TextureAtlas {
            layout: spritesheet.layout.clone(),
            ..default()
        },
    ));
    // .add_child(light);
}

And then add the following as a separate system in order to slowly move the camera away from the light

fn move_camera(
    time: Res<Time>,
    mut camera: Query<&mut Transform, With<GameCamera>>,
) {
    let mut camera_transform = camera.single_mut();
    camera_transform.translation.x += 16.0 * time.delta_seconds();
}

This might be a non-issue as lights shouldn't be added to a Sprite'd component which I think is fine if that's the case. It did however make me delve a bit into why this happened and I'm pretty sure it's due to the ViewVisibility of the entity being set to false when the sprite is off-screen, which is then filtered out in the extraction step:

# src/render/extract.rs

pub fn extract_point_lights(
    mut commands: Commands,
    point_light_query: Extract<Query<(Entity, &PointLight2d, &GlobalTransform, &ViewVisibility)>>,
) {
    for (entity, point_light, global_transform, view_visibility) in &point_light_query {
        if !view_visibility.get() {
            continue;
        }

So I think that if a light doesn't have a sprite component, it also doesn't get its aabb computed in bevy's internal visibility computation step, which should mean that the light will always be rendered, even if it's far off screen. I might be wrong here though as I'm not certain if this library (or bevy) handles visibility in some other smart way.

I think that if my assumptions are correct, it would probably be good to figure out a way to filter out non-visible lights in the extraction step based on their radius and transform. If that is by creating a new Visibility component and then copying what bevy does (e.g set a ComputedLightVisibility to false initially, and then going through each light, checking its aabb against visible cameras and setting any visible lights to true), or by smartly hooking into bevy's existing visibility calculation system somehow. Maybe there is a possibility to add a aabb struct which bevy can use?

[jgayfer]

Thanks for the write up! I haven't delved into this myself, but I'm fairly confident your analysis is correct.

I consider this something we should address. There are workarounds, as you've mentioned, but from a pure usability standpoint, this behaviour will likely catch many off guard.