Material, mesh, skin extraction optimization

[brianreavis]

Objective

The extraction systems for materials, meshes, and skins previously iterated over RemovedComponents<ViewVisibility> in addition to more specific variants like RemovedComponents<MeshMaterial3d<M>>. This caused each system to loop through and check many irrelevant despawned entities—sometimes multiple times. With many material types, this overhead added up and became noticeable in frames with many despawns.

Solution

This PR removes superfluous RemovedComponents iteration for ViewVisibility and GlobalTransform, ensuring that we only iterate over the most specific RemovedComponents relevant to the system (e.g., material components, mesh components). This is guaranteed to match what the system originally collected.

Before (red) / After (yellow):

Log plot to highlight the long tail that this PR is addressing.

[pcwalton]

The reason for iterating over ViewVisibility is that removing the ViewVisibility component should essentially disable an object. This is admittedly a rather surprising behavior, but I believe it matches what Bevy has traditionally done regarding "required components".

[brianreavis]

The reason for iterating over ViewVisibility is that removing the ViewVisibility component should essentially disable an object.

Isn't it impossible to remove ViewVisibility for a mesh given it's a required component for meshes? Though maybe I'm not totally following and you're saying it is technically possible (and that's a quirk of required components). Are there any cases where bevy does use that surprising behavior?

components.rs

#[derive(Component, ...)]
#[require(Transform, Visibility, VisibilityClass)]
pub struct Mesh3d(pub Handle<Mesh>);

visibility.rs

#[derive(Component, ...)]
#[require(InheritedVisibility, ViewVisibility)]
pub enum Visibility { ... }

[brianreavis]

I don't think this should be blocked by #18514 or other solutions for these reasons:

1. The worst-case scenario is that we retain the Entity → AssetId<M> mapping until the entity is despawned or made visible and then hidden. It's not going to cause unexpected rendering or extra work.
2. Removal of ViewVisibility / GlobalTransform components is a real edge case.
3. The optimization here can demonstrably shave nearly a millisecond off the sensitive ExtractSchedule in heavy-despawn frames.

[atlv24]

Unfortunately I do think this should be blocked on required component invariant enforcement. This is a win but at the price of a behavioral regression that will probably affect some users

[brianreavis]

This won't cause a behavioral change because the worst case scenario is that ViewVisibility is removed and the instance simply remains in the RenderMaterial2dInstances, RenderMaterialInstances, etc hashmaps until despawn.

[superdump]

Sounds reasonable to me. I am trusting that this has no negative side effects other than the potential memory leak if those entities are not despawned. But I kind of expect we have a number of cases of that in the render world, just that they haven’t been significant enough for someone to do something about them yet.