Bevy crashes when simultaneously setting `camera.is_active = false` and inserting `Disabled` to it

[M4tsuri]

Bevy version

0.16.1

What you did

Here is a mini repro:

use bevy::{ecs::entity_disabling::Disabled, prelude::*};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(Update, input)
        .run();
}

#[derive(Component)]
struct SingleCamera(Entity);

fn setup(mut commands: Commands) {
    let camera = commands.spawn((
        Camera3d::default(),
        Camera {
            is_active: true,
            ..Default::default()
        },
        // Disabled
    )).id();

    commands.spawn(SingleCamera(camera));
}


fn input(
    mut commands: Commands,
    input: Res<ButtonInput<KeyCode>>,
    camera: Query<&SingleCamera>,
    mut cameras: Query<(&mut Camera, Has<Disabled>)>
) {
    let entity = camera.single().unwrap().0;
    let mut camera = cameras.single_mut().unwrap();
    if input.just_released(KeyCode::Space) {
        // now set is_active to true and insert the "Disabled" component
        camera.0.is_active = false;
        commands.entity(entity).insert(Disabled);
    }
}

Run this App and press "Space", there will be a crash with message

thread 'Compute Task Pool (4)' panicked at /bevy/crates/bevy_core_pipeline/src/core_3d/mod.rs:833:22:
The depth texture usage should already exist for this target

Additional information

I tried to dig into the source code to figure out what exactly causes this. Here are my findings.

It seems that the components like ExtractedView are not automatically cleaned up from render world after each run of render pipeline. This the reason why we need to manually remove them once the corresponding camera becomes inactive.

bevy/crates/bevy_render/src/camera/camera.rs

Lines 1133 to 1144 in 383b351

	if !camera.is_active {
	commands.entity(render_entity).remove::<(
	ExtractedCamera,
	ExtractedView,
	RenderVisibleEntities,
	TemporalJitter,
	RenderLayers,
	Projection,
	NoIndirectDrawing,
	ViewUniformOffset,
	)>();
	continue;

However, when the camera is disabled by a Disabled component, it is filtered out from the query, thus makes the extracted components remain in the render world, even we set camera.is_active to false. However, there are serval tasks like prepare_core_3d_depth_textures highly relies on the existence of these extracted components to determine currently active cameras.

bevy/crates/bevy_core_pipeline/src/core_3d/mod.rs

Line 766 in 383b351

pub fn prepare_core_3d_depth_textures(

So this function will panic as the camera simply does not exist.

I added a Has<Disabled> filter to query of extract_cameras and changed the condition here

bevy/crates/bevy_render/src/camera/camera.rs

Line 1133 in 383b351

if !camera.is_active {

to

if !camera.is_active || disabled {

Then the mini repro works.

P.S. I'm not familiar when bevy's codebase, so my analysis could be wrong. I would appreciate it if someone can correct me.

[alice-i-cecile]

Good reproduction, and nice attempt for the fix :) It could be more robust though, including to other forms of disabling components. The correct fix is to skip iterations of loops where camera entities aren't found.

The next-level fix would be to add observer events for when entities are disabled and then use that to update things like camera caches. That's more controversial and involved though, and I still think we should be more gracefully handling unexpectedly missing cameras if that was included.

[M4tsuri]

Thank you for your correction. I'm pretty interested in submitting a fix for this. Do you mean that we need to add Camera itself to query filters of prepare_core_3d_depth_textures?

[alice-i-cecile]

Let me ask on Discord for you quick; I don't immediately have the time to dig into this myself :)

[IceSentry]

Do you have a use case for using Disabled on a camera? The is_active field is the current intended mechanism to disable a camera. Pretty much everything in the render world was designed before entity disabling was a thing and I'm pretty sure a lot of assumptions were made that would completely break in the context of entity disabling. I don't think there's a simple fix here. The first thing that would need to happen is to be able to extract the Disabled marker.

[M4tsuri]

My usecase is rather niche. I was trying to build a recorder over a camera with the Readback component. But it seems that there is no straightforward way to turn it off. So this is where the Disabled component comes in. My plan was to put Readback and Camera in the same bundle as well as adding a Disabled component when needed to disable the recorder. Then I got this panic. However I can easily avoid this by separating Readback and Camera into two different bundles.

I looked into the codebase and found that there are indeed multiple places that rely on similar pattern to do proper clean-ups. This would definitely need some work to be fully addressed. Before that, it would be great if we mention this situation in docs.