Fix the ordering of the systems introduced in #18734. (#18825)

There's still a race resulting in blank materials whenever a material of
type A is added on the same frame that a material of type B is removed.
PR #18734 improved the situation, but ultimately didn't fix the race
because of two issues:

1. The `late_sweep_material_instances` system was never scheduled. This
PR fixes the problem by scheduling that system.

2. `early_sweep_material_instances` needs to be called after *every*
material type has been extracted, not just when the material of *that*
type has been extracted. The `chain()` added during the review process
in PR #18734 broke this logic. This PR reverts that and fixes the
ordering by introducing a new `SystemSet` that contains all material
extraction systems.

I also took the opportunity to switch a manual reference to
`AssetId::<StandardMaterial>::invalid()` to the new
`DUMMY_MESH_MATERIAL` constant for clarity.

Because this is a bug that can affect any application that switches
material types in a single frame, I think this should be uplifted to
Bevy 0.16.

Author: pcwalton

crates/bevy_pbr/src/lib.rs

@@ -466,7 +466,14 @@ impl Plugin for PbrPlugin {
 
         // Extract the required data from the main world
         render_app
-            .add_systems(ExtractSchedule, (extract_clusters, extract_lights))
+            .add_systems(
+                ExtractSchedule,
+                (
+                    extract_clusters,
+                    extract_lights,
+                    late_sweep_material_instances,
+                ),
+            )
             .add_systems(
                 Render,
                 (

crates/bevy_pbr/src/material.rs

@@ -316,13 +316,10 @@ where
                 .add_systems(
                     ExtractSchedule,
                     (
-                        (
-                            extract_mesh_materials::<M>,
-                            early_sweep_material_instances::<M>,
-                        )
-                            .chain()
-                            .before(late_sweep_material_instances)
-                            .before(ExtractMeshesSet),
+                        extract_mesh_materials::<M>.in_set(ExtractMaterialsSet),
+                        early_sweep_material_instances::<M>
+                            .after(ExtractMaterialsSet)
+                            .before(late_sweep_material_instances),
                         extract_entities_needs_specialization::<M>.after(extract_cameras),
                     ),
                 )
@@ -415,7 +412,8 @@ where
 ///
 /// See the comments in [`RenderMaterialInstances::mesh_material`] for more
 /// information.
-static DUMMY_MESH_MATERIAL: AssetId<StandardMaterial> = AssetId::<StandardMaterial>::invalid();
+pub(crate) static DUMMY_MESH_MATERIAL: AssetId<StandardMaterial> =
+    AssetId::<StandardMaterial>::invalid();
 
 /// A key uniquely identifying a specialized [`MaterialPipeline`].
 pub struct MaterialPipelineKey<M: Material> {
@@ -637,6 +635,10 @@ pub struct RenderMaterialInstance {
     last_change_tick: Tick,
 }
 
+/// A [`SystemSet`] that contains all `extract_mesh_materials` systems.
+#[derive(SystemSet, Clone, PartialEq, Eq, Debug, Hash)]
+pub struct ExtractMaterialsSet;
+
 pub const fn alpha_mode_pipeline_key(alpha_mode: AlphaMode, msaa: &Msaa) -> MeshPipelineKey {
     match alpha_mode {
         // Premultiplied and Add share the same pipeline key
@@ -782,7 +784,7 @@ fn early_sweep_material_instances<M>(
 /// This runs after all invocations of [`early_sweep_material_instances`] and is
 /// responsible for bumping [`RenderMaterialInstances::current_change_tick`] in
 /// preparation for a new frame.
-fn late_sweep_material_instances(
+pub(crate) fn late_sweep_material_instances(
     mut material_instances: ResMut<RenderMaterialInstances>,
     mut removed_visibilities_query: Extract<RemovedComponents<ViewVisibility>>,
 ) {

crates/bevy_pbr/src/meshlet/instance_manager.rs

@@ -1,10 +1,10 @@
 use super::{meshlet_mesh_manager::MeshletMeshManager, MeshletMesh, MeshletMesh3d};
 use crate::{
-    Material, MaterialBindingId, MeshFlags, MeshTransforms, MeshUniform, NotShadowCaster,
-    NotShadowReceiver, PreviousGlobalTransform, RenderMaterialBindings, RenderMaterialInstances,
-    StandardMaterial,
+    material::DUMMY_MESH_MATERIAL, Material, MaterialBindingId, MeshFlags, MeshTransforms,
+    MeshUniform, NotShadowCaster, NotShadowReceiver, PreviousGlobalTransform,
+    RenderMaterialBindings, RenderMaterialInstances,
 };
-use bevy_asset::{AssetEvent, AssetId, AssetServer, Assets, UntypedAssetId};
+use bevy_asset::{AssetEvent, AssetServer, Assets, UntypedAssetId};
 use bevy_ecs::{
     entity::{Entities, Entity, EntityHashMap},
     event::EventReader,
@@ -113,16 +113,15 @@ impl InstanceManager {
         };
 
         let mesh_material = mesh_material_ids.mesh_material(instance);
-        let mesh_material_binding_id =
-            if mesh_material != AssetId::<StandardMaterial>::invalid().untyped() {
-                render_material_bindings
-                    .get(&mesh_material)
-                    .cloned()
-                    .unwrap_or_default()
-            } else {
-                // Use a dummy binding ID if the mesh has no material
-                MaterialBindingId::default()
-            };
+        let mesh_material_binding_id = if mesh_material != DUMMY_MESH_MATERIAL.untyped() {
+            render_material_bindings
+                .get(&mesh_material)
+                .cloned()
+                .unwrap_or_default()
+        } else {
+            // Use a dummy binding ID if the mesh has no material
+            MaterialBindingId::default()
+        };
 
         let mesh_uniform = MeshUniform::new(
             &transforms,

crates/bevy_pbr/src/render/mesh.rs

@@ -196,7 +196,9 @@ impl Plugin for MeshRenderPlugin {
                 .init_resource::<RenderMaterialInstances>()
                 .configure_sets(
                     ExtractSchedule,
-                    ExtractMeshesSet.after(view::extract_visibility_ranges),
+                    ExtractMeshesSet
+                        .after(view::extract_visibility_ranges)
+                        .after(late_sweep_material_instances),
                 )
                 .add_systems(
                     ExtractSchedule,
@@ -1131,19 +1133,18 @@ impl RenderMeshInstanceGpuBuilder {
         // yet loaded. In that case, add the mesh to
         // `meshes_to_reextract_next_frame` and bail.
         let mesh_material = mesh_material_ids.mesh_material(entity);
-        let mesh_material_binding_id =
-            if mesh_material != AssetId::<StandardMaterial>::invalid().untyped() {
-                match render_material_bindings.get(&mesh_material) {
-                    Some(binding_id) => *binding_id,
-                    None => {
-                        meshes_to_reextract_next_frame.insert(entity);
-                        return None;
-                    }
+        let mesh_material_binding_id = if mesh_material != DUMMY_MESH_MATERIAL.untyped() {
+            match render_material_bindings.get(&mesh_material) {
+                Some(binding_id) => *binding_id,
+                None => {
+                    meshes_to_reextract_next_frame.insert(entity);
+                    return None;
                 }
-            } else {
-                // Use a dummy material binding ID.
-                MaterialBindingId::default()
-            };
+            }
+        } else {
+            // Use a dummy material binding ID.
+            MaterialBindingId::default()
+        };
         self.shared.material_bindings_index = mesh_material_binding_id;
 
         let lightmap_slot = match render_lightmaps.render_lightmaps.get(&entity) {