From 8ba495f42ee343f0af65de69395d841845cc1731 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sun, 5 Oct 2025 19:04:36 -0700 Subject: [PATCH 1/4] Fix a crash that occurs when an off-screen entity's material type changes and the material then becomes visible. PR #20993 attempted to fix a crash that would occur with the following sequence of events: 1. An entity's material changes from type A to type B. (The material *type* must change, not just the material asset.) 2. The `extract_entities_needs_specialization` system runs and adds a new specialization change tick to the entity. 3. The `extract_entities_needs_specialization` system runs and removes the specialization change tick. 4. We crash in rendering because no specialization change tick was present for the entity. Unfortunately, that PR used the presence of the entity in `RenderMaterialInstances` to detect whether the entity is safe to delete from the specialization change tick table. This is incorrect for meshes that change material types while not visible, because only visible meshes are present in `RenderMaterialInstances`. So the above race can still occur if the mesh changes materials while off-screen, which will lead to a crash when the mesh becomes visible. This PR fixes the issue by dividing the process of adding new specialization ticks and the process of removing old specialization ticks into two systems. First, all specialization ticks for all materials are updated; alongside that, we store the *material instance tick*, which is a tick that's updated once per frame. After that, we run `sweep_entities_needing_specialization`, which processes the `RemovedComponents` list for each material and prunes dead entities from the table if and only if their material instance ticks haven't changed since the last frame. This ensures that the above race can't happen, regardless of whether the meshes are present in `RenderMaterialInstances`. Having to have two separate specialization ticks, one being the standard Bevy system tick and one being a special material instance tick that's updated once per frame, is unfortunate, but it seemed to me like the least bad option. --- crates/bevy_pbr/src/material.rs | 141 +++++++++++++++++++++++----- crates/bevy_pbr/src/prepass/mod.rs | 9 +- crates/bevy_pbr/src/render/light.rs | 6 +- 3 files changed, 129 insertions(+), 27 deletions(-) diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index af4067e08bd93..e54ad85d1ec2a 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -283,7 +283,7 @@ impl Plugin for MaterialsPlugin { app.add_plugins((PrepassPipelinePlugin, PrepassPlugin::new(self.debug_flags))); if let Some(render_app) = app.get_sub_app_mut(RenderApp) { render_app - .init_resource::() + .init_resource::() .init_resource::() .init_resource::>() .init_resource::() @@ -387,9 +387,16 @@ where early_sweep_material_instances:: .after(MaterialExtractionSystems) .before(late_sweep_material_instances), + // See the comments in + // `sweep_entities_needing_specialization` for an + // explanation of why the systems are ordered this way. extract_entities_needs_specialization:: + .in_set(MaterialExtractEntitiesNeedingSpecializationSystems), + sweep_entities_needing_specialization:: + .after(MaterialExtractEntitiesNeedingSpecializationSystems) + .after(MaterialExtractionSystems) .after(extract_cameras) - .after(MaterialExtractionSystems), + .before(late_sweep_material_instances), ), ); } @@ -604,6 +611,11 @@ pub struct RenderMaterialInstance { #[derive(SystemSet, Clone, PartialEq, Eq, Debug, Hash)] pub struct MaterialExtractionSystems; +/// A [`SystemSet`] that contains all `extract_entities_needs_specialization` +/// systems. +#[derive(SystemSet, Clone, PartialEq, Eq, Debug, Hash)] +pub struct MaterialExtractEntitiesNeedingSpecializationSystems; + /// Deprecated alias for [`MaterialExtractionSystems`]. #[deprecated(since = "0.17.0", note = "Renamed to `MaterialExtractionSystems`.")] pub type ExtractMaterialsSet = MaterialExtractionSystems; @@ -777,8 +789,40 @@ pub(crate) fn late_sweep_material_instances( pub fn extract_entities_needs_specialization( entities_needing_specialization: Extract>>, - material_instances: Res, - mut entity_specialization_ticks: ResMut, + mut entity_specialization_ticks: ResMut, + render_material_instances: Res, + ticks: SystemChangeTick, +) where + M: Material, +{ + for entity in entities_needing_specialization.iter() { + // Update the entity's specialization tick with this run's tick + entity_specialization_ticks.insert( + (*entity).into(), + EntitySpecializationTicks { + system_tick: ticks.this_run(), + material_instances_tick: render_material_instances.current_change_tick, + }, + ); + } +} + +/// A system that runs after all instances of +/// [`extract_entities_needs_specialization`] in order to delete specialization +/// ticks for entities that are no longer renderable. +/// +/// We delete entities from the [`EntitySpecializationTicksTable`] *after* +/// updating it with newly-discovered renderable entities in order to handle the +/// case in which a single entity changes material types. If we naïvely removed +/// entities from that table when their [`MeshMaterial3d`] components were +/// removed, and an entity changed material types, we might end up adding a new +/// set of [`EntitySpecializationTicks`] for the new material and then deleting +/// it upon detecting the removed component for the old material. Deferring +/// [`sweep_entities_needing_specialization`] to the end allows us to detect the +/// case in which another material type updated the entity specialization ticks +/// this frame and avoid deleting it if so. +pub fn sweep_entities_needing_specialization( + mut entity_specialization_ticks: ResMut, mut removed_mesh_material_components: Extract>>, mut specialized_material_pipeline_cache: ResMut, mut specialized_prepass_material_pipeline_cache: Option< @@ -787,8 +831,8 @@ pub fn extract_entities_needs_specialization( mut specialized_shadow_material_pipeline_cache: Option< ResMut, >, + render_material_instances: Res, views: Query<&ExtractedView>, - ticks: SystemChangeTick, ) where M: Material, { @@ -796,15 +840,22 @@ pub fn extract_entities_needs_specialization( // the same frame, thus will appear both in the removed components list and have been added to // the `EntitiesNeedingSpecialization` collection by triggering the `Changed` filter // - // Additionally, we need to make sure that we are careful about materials that could have changed - // type, e.g. from a `StandardMaterial` to a `CustomMaterial`, as this will also appear in the - // removed components list. As such, we make sure that this system runs after `MaterialExtractionSystems` - // so that the `RenderMaterialInstances` bookkeeping has already been done, and we can check if the entity - // still has a valid material instance. + // Additionally, we need to make sure that we are careful about materials + // that could have changed type, e.g. from a `StandardMaterial` to a + // `CustomMaterial`, as this will also appear in the removed components + // list. As such, we make sure that this system runs after + // `extract_entities_needs_specialization` so that the entity specialization + // tick bookkeeping has already been done, and we can check if the entity's + // tick was updated this frame. for entity in removed_mesh_material_components.read() { - if material_instances - .instances - .contains_key(&MainEntity::from(entity)) + // If the entity's specialization tick was updated this frame, that + // means that that entity changed materials this frame. Don't remove the + // entity from the table in that case. + if entity_specialization_ticks + .get(&MainEntity::from(entity)) + .is_some_and(|ticks| { + ticks.material_instances_tick == render_material_instances.current_change_tick + }) { continue; } @@ -830,11 +881,6 @@ pub fn extract_entities_needs_specialization( } } } - - for entity in entities_needing_specialization.iter() { - // Update the entity's specialization tick with this run's tick - entity_specialization_ticks.insert((*entity).into(), ticks.this_run()); - } } #[derive(Resource, Deref, DerefMut, Clone, Debug)] @@ -853,10 +899,58 @@ impl Default for EntitiesNeedingSpecialization { } } +/// Stores ticks specifying the last time Bevy specialized the pipelines of each +/// entity. +/// +/// Every entity that has a mesh and material must be present in this table, +/// even if that mesh isn't visible. #[derive(Resource, Deref, DerefMut, Default, Clone, Debug)] -pub struct EntitySpecializationTicks { +pub struct EntitySpecializationTicksTable { + /// A mapping from each main entity to ticks that specify the last time this + /// entity's pipeline was specialized. + /// + /// Every entity that has a mesh and material must be present in this table, + /// even if that mesh isn't visible. #[deref] - pub entities: MainEntityHashMap, + pub entities: MainEntityHashMap, +} + +/// Ticks that specify the last time an entity's pipeline was specialized. +/// +/// We need two different types of ticks here for a subtle reason. First, we +/// need the [`Self::system_tick`], which maps to Bevy's [`SystemChangeTick`], +/// because that's what we use in [`specialize_material_meshes`] to check +/// whether pipelines need specialization. But we also need +/// [`Self::material_instances_tick`], which maps to the +/// [`RenderMaterialInstances::current_change_tick`]. That's because the latter +/// only changes once per frame, which is a guarantee we need to handle the +/// following case: +/// +/// 1. The app removes material A from a mesh and replaces it with material B. +/// Both A and B are of different [`Material`] types entirely. +/// +/// 2. [`extract_entities_needs_specialization`] runs for material B and marks +/// the mesh as up to date by recording the current tick. +/// +/// 3. [`sweep_entities_needing_specialization`] runs for material A and checks +/// to ensure it's safe to remove the [`EntitySpecializationTicks`] for the mesh +/// from the [`EntitySpecializationTicksTable`]. To do this, it needs to know +/// whether [`extract_entities_needs_specialization`] for some *different* +/// material (in this case, material B) ran earlier in the frame and updated the +/// change tick, and to skip removing the [`EntitySpecializationTicks`] if so. +/// It can't reliably use the [`Self::system_tick`] to determine this because +/// the [`SystemChangeTick`] can be updated multiple times in the same frame. +/// Instead, it needs a type of tick that's updated only once per frame, after +/// all materials' versions of [`sweep_entities_needing_specialization`] have +/// run. The [`RenderMaterialInstances`] tick satisfies this criterion, and so +/// that's what [`sweep_entities_needing_specialization`] uses. +#[derive(Clone, Copy, Debug)] +pub struct EntitySpecializationTicks { + /// The standard Bevy system tick. + pub system_tick: Tick, + /// The tick in [`RenderMaterialInstances`], which is updated in + /// `late_sweep_material_instances`. + pub material_instances_tick: Tick, } /// Stores the [`SpecializedMaterialViewPipelineCache`] for each view. @@ -923,7 +1017,7 @@ pub fn specialize_material_meshes( ), views: Query<(&ExtractedView, &RenderVisibleEntities)>, view_key_cache: Res, - entity_specialization_ticks: Res, + entity_specialization_ticks: Res, view_specialization_ticks: Res, mut specialized_material_pipeline_cache: ResMut, mut pipelines: ResMut>, @@ -966,7 +1060,10 @@ pub fn specialize_material_meshes( else { continue; }; - let entity_tick = entity_specialization_ticks.get(visible_entity).unwrap(); + let entity_tick = entity_specialization_ticks + .get(visible_entity) + .unwrap() + .system_tick; let last_specialized_tick = view_specialized_material_pipeline_cache .get(visible_entity) .map(|(tick, _)| *tick); diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 581b8af288660..36136b943548d 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -4,7 +4,7 @@ use crate::{ alpha_mode_pipeline_key, binding_arrays_are_usable, buffer_layout, collect_meshes_for_gpu_building, init_material_pipeline, set_mesh_motion_vector_flags, setup_morph_and_skinning_defs, skin, DeferredDrawFunction, DeferredFragmentShader, - DeferredVertexShader, DrawMesh, EntitySpecializationTicks, ErasedMaterialPipelineKey, + DeferredVertexShader, DrawMesh, EntitySpecializationTicksTable, ErasedMaterialPipelineKey, MaterialPipeline, MaterialProperties, MeshLayouts, MeshPipeline, MeshPipelineKey, OpaqueRendererMethod, PreparedMaterial, PrepassDrawFunction, PrepassFragmentShader, PrepassVertexShader, RenderLightmaps, RenderMaterialInstances, RenderMeshInstanceFlags, @@ -844,7 +844,7 @@ pub fn specialize_prepass_material_meshes( ResMut>, Res, Res, - Res, + Res, ), ) { for (extracted_view, visible_entities, msaa, motion_vector_prepass, deferred_prepass) in &views @@ -877,7 +877,10 @@ pub fn specialize_prepass_material_meshes( else { continue; }; - let entity_tick = entity_specialization_ticks.get(visible_entity).unwrap(); + let entity_tick = entity_specialization_ticks + .get(visible_entity) + .unwrap() + .system_tick; let last_specialized_tick = view_specialized_material_pipeline_cache .get(visible_entity) .map(|(tick, _)| *tick); diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index 8bce0f4556bb6..b2f83c947ae62 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -1767,7 +1767,7 @@ pub fn specialize_shadows( light_key_cache: Res, mut specialized_material_pipeline_cache: ResMut, light_specialization_ticks: Res, - entity_specialization_ticks: Res, + entity_specialization_ticks: Res, ticks: SystemChangeTick, ) { // Record the retained IDs of all shadow views so that we can expire old @@ -1844,7 +1844,9 @@ pub fn specialize_shadows( .map(|(tick, _)| *tick); let needs_specialization = last_specialized_tick.is_none_or(|tick| { view_tick.is_newer_than(tick, ticks.this_run()) - || entity_tick.is_newer_than(tick, ticks.this_run()) + || entity_tick + .system_tick + .is_newer_than(tick, ticks.this_run()) }); if !needs_specialization { continue; From 8df961018f55f9f663b48717cef42389c5bc8d10 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sun, 5 Oct 2025 21:58:05 -0700 Subject: [PATCH 2/4] Doc formatting police --- crates/bevy_pbr/src/material.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index e54ad85d1ec2a..768a6b0f7fba1 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -933,17 +933,17 @@ pub struct EntitySpecializationTicksTable { /// the mesh as up to date by recording the current tick. /// /// 3. [`sweep_entities_needing_specialization`] runs for material A and checks -/// to ensure it's safe to remove the [`EntitySpecializationTicks`] for the mesh -/// from the [`EntitySpecializationTicksTable`]. To do this, it needs to know -/// whether [`extract_entities_needs_specialization`] for some *different* -/// material (in this case, material B) ran earlier in the frame and updated the -/// change tick, and to skip removing the [`EntitySpecializationTicks`] if so. -/// It can't reliably use the [`Self::system_tick`] to determine this because -/// the [`SystemChangeTick`] can be updated multiple times in the same frame. -/// Instead, it needs a type of tick that's updated only once per frame, after -/// all materials' versions of [`sweep_entities_needing_specialization`] have -/// run. The [`RenderMaterialInstances`] tick satisfies this criterion, and so -/// that's what [`sweep_entities_needing_specialization`] uses. +/// to ensure it's safe to remove the [`EntitySpecializationTicks`] for the mesh +/// from the [`EntitySpecializationTicksTable`]. To do this, it needs to know +/// whether [`extract_entities_needs_specialization`] for some *different* +/// material (in this case, material B) ran earlier in the frame and updated the +/// change tick, and to skip removing the [`EntitySpecializationTicks`] if so. +/// It can't reliably use the [`Self::system_tick`] to determine this because +/// the [`SystemChangeTick`] can be updated multiple times in the same frame. +/// Instead, it needs a type of tick that's updated only once per frame, after +/// all materials' versions of [`sweep_entities_needing_specialization`] have +/// run. The [`RenderMaterialInstances`] tick satisfies this criterion, and so +/// that's what [`sweep_entities_needing_specialization`] uses. #[derive(Clone, Copy, Debug)] pub struct EntitySpecializationTicks { /// The standard Bevy system tick. From b04355ffbff1bbd308ebd72b662077ae64b69b25 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sun, 5 Oct 2025 22:55:03 -0700 Subject: [PATCH 3/4] Un-rename `EntitySpecializationTicks` to avoid breaking compatibility in a point release. This also fixes the `manual_material` example (which was actually broken before, as it never had PR #20993 applied). --- crates/bevy_pbr/src/material.rs | 36 ++++++------- crates/bevy_pbr/src/prepass/mod.rs | 4 +- crates/bevy_pbr/src/render/light.rs | 2 +- .../bevy_sprite_render/src/mesh2d/material.rs | 10 ++-- examples/3d/manual_material.rs | 54 +++++++++++++++++-- 5 files changed, 76 insertions(+), 30 deletions(-) diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index 768a6b0f7fba1..3fe645b549a68 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -283,7 +283,7 @@ impl Plugin for MaterialsPlugin { app.add_plugins((PrepassPipelinePlugin, PrepassPlugin::new(self.debug_flags))); if let Some(render_app) = app.get_sub_app_mut(RenderApp) { render_app - .init_resource::() + .init_resource::() .init_resource::() .init_resource::>() .init_resource::() @@ -765,7 +765,7 @@ fn early_sweep_material_instances( /// 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. -pub(crate) fn late_sweep_material_instances( +pub fn late_sweep_material_instances( mut material_instances: ResMut, mut removed_meshes_query: Extract>, ) { @@ -789,7 +789,7 @@ pub(crate) fn late_sweep_material_instances( pub fn extract_entities_needs_specialization( entities_needing_specialization: Extract>>, - mut entity_specialization_ticks: ResMut, + mut entity_specialization_ticks: ResMut, render_material_instances: Res, ticks: SystemChangeTick, ) where @@ -799,7 +799,7 @@ pub fn extract_entities_needs_specialization( // Update the entity's specialization tick with this run's tick entity_specialization_ticks.insert( (*entity).into(), - EntitySpecializationTicks { + EntitySpecializationTickPair { system_tick: ticks.this_run(), material_instances_tick: render_material_instances.current_change_tick, }, @@ -811,18 +811,18 @@ pub fn extract_entities_needs_specialization( /// [`extract_entities_needs_specialization`] in order to delete specialization /// ticks for entities that are no longer renderable. /// -/// We delete entities from the [`EntitySpecializationTicksTable`] *after* +/// We delete entities from the [`EntitySpecializationTicks`] table *after* /// updating it with newly-discovered renderable entities in order to handle the /// case in which a single entity changes material types. If we naïvely removed /// entities from that table when their [`MeshMaterial3d`] components were /// removed, and an entity changed material types, we might end up adding a new -/// set of [`EntitySpecializationTicks`] for the new material and then deleting -/// it upon detecting the removed component for the old material. Deferring -/// [`sweep_entities_needing_specialization`] to the end allows us to detect the -/// case in which another material type updated the entity specialization ticks -/// this frame and avoid deleting it if so. +/// set of [`EntitySpecializationTickPair`] for the new material and then +/// deleting it upon detecting the removed component for the old material. +/// Deferring [`sweep_entities_needing_specialization`] to the end allows us to +/// detect the case in which another material type updated the entity +/// specialization ticks this frame and avoid deleting it if so. pub fn sweep_entities_needing_specialization( - mut entity_specialization_ticks: ResMut, + mut entity_specialization_ticks: ResMut, mut removed_mesh_material_components: Extract>>, mut specialized_material_pipeline_cache: ResMut, mut specialized_prepass_material_pipeline_cache: Option< @@ -905,14 +905,14 @@ impl Default for EntitiesNeedingSpecialization { /// Every entity that has a mesh and material must be present in this table, /// even if that mesh isn't visible. #[derive(Resource, Deref, DerefMut, Default, Clone, Debug)] -pub struct EntitySpecializationTicksTable { +pub struct EntitySpecializationTicks { /// A mapping from each main entity to ticks that specify the last time this /// entity's pipeline was specialized. /// /// Every entity that has a mesh and material must be present in this table, /// even if that mesh isn't visible. #[deref] - pub entities: MainEntityHashMap, + pub entities: MainEntityHashMap, } /// Ticks that specify the last time an entity's pipeline was specialized. @@ -933,11 +933,11 @@ pub struct EntitySpecializationTicksTable { /// the mesh as up to date by recording the current tick. /// /// 3. [`sweep_entities_needing_specialization`] runs for material A and checks -/// to ensure it's safe to remove the [`EntitySpecializationTicks`] for the mesh -/// from the [`EntitySpecializationTicksTable`]. To do this, it needs to know +/// to ensure it's safe to remove the [`EntitySpecializationTickPair`] for the mesh +/// from the [`EntitySpecializationTicks`]. To do this, it needs to know /// whether [`extract_entities_needs_specialization`] for some *different* /// material (in this case, material B) ran earlier in the frame and updated the -/// change tick, and to skip removing the [`EntitySpecializationTicks`] if so. +/// change tick, and to skip removing the [`EntitySpecializationTickPair`] if so. /// It can't reliably use the [`Self::system_tick`] to determine this because /// the [`SystemChangeTick`] can be updated multiple times in the same frame. /// Instead, it needs a type of tick that's updated only once per frame, after @@ -945,7 +945,7 @@ pub struct EntitySpecializationTicksTable { /// run. The [`RenderMaterialInstances`] tick satisfies this criterion, and so /// that's what [`sweep_entities_needing_specialization`] uses. #[derive(Clone, Copy, Debug)] -pub struct EntitySpecializationTicks { +pub struct EntitySpecializationTickPair { /// The standard Bevy system tick. pub system_tick: Tick, /// The tick in [`RenderMaterialInstances`], which is updated in @@ -1017,7 +1017,7 @@ pub fn specialize_material_meshes( ), views: Query<(&ExtractedView, &RenderVisibleEntities)>, view_key_cache: Res, - entity_specialization_ticks: Res, + entity_specialization_ticks: Res, view_specialization_ticks: Res, mut specialized_material_pipeline_cache: ResMut, mut pipelines: ResMut>, diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 36136b943548d..ca7cc5c66e602 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -4,7 +4,7 @@ use crate::{ alpha_mode_pipeline_key, binding_arrays_are_usable, buffer_layout, collect_meshes_for_gpu_building, init_material_pipeline, set_mesh_motion_vector_flags, setup_morph_and_skinning_defs, skin, DeferredDrawFunction, DeferredFragmentShader, - DeferredVertexShader, DrawMesh, EntitySpecializationTicksTable, ErasedMaterialPipelineKey, + DeferredVertexShader, DrawMesh, EntitySpecializationTicks, ErasedMaterialPipelineKey, MaterialPipeline, MaterialProperties, MeshLayouts, MeshPipeline, MeshPipelineKey, OpaqueRendererMethod, PreparedMaterial, PrepassDrawFunction, PrepassFragmentShader, PrepassVertexShader, RenderLightmaps, RenderMaterialInstances, RenderMeshInstanceFlags, @@ -844,7 +844,7 @@ pub fn specialize_prepass_material_meshes( ResMut>, Res, Res, - Res, + Res, ), ) { for (extracted_view, visible_entities, msaa, motion_vector_prepass, deferred_prepass) in &views diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index b2f83c947ae62..904be3164ec60 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -1767,7 +1767,7 @@ pub fn specialize_shadows( light_key_cache: Res, mut specialized_material_pipeline_cache: ResMut, light_specialization_ticks: Res, - entity_specialization_ticks: Res, + entity_specialization_ticks: Res, ticks: SystemChangeTick, ) { // Record the retained IDs of all shadow views so that we can expire old diff --git a/crates/bevy_sprite_render/src/mesh2d/material.rs b/crates/bevy_sprite_render/src/mesh2d/material.rs index a96c1e01a37e1..6ee6f0fccc66d 100644 --- a/crates/bevy_sprite_render/src/mesh2d/material.rs +++ b/crates/bevy_sprite_render/src/mesh2d/material.rs @@ -283,7 +283,7 @@ where if let Some(render_app) = app.get_sub_app_mut(RenderApp) { render_app - .init_resource::>() + .init_resource::>() .init_resource::>() .add_render_command::>() .add_render_command::>() @@ -566,7 +566,7 @@ pub const fn tonemapping_pipeline_key(tonemapping: Tonemapping) -> Mesh2dPipelin pub fn extract_entities_needs_specialization( entities_needing_specialization: Extract>>, - mut entity_specialization_ticks: ResMut>, + mut entity_specialization_ticks: ResMut>, mut removed_mesh_material_components: Extract>>, mut specialized_material2d_pipeline_cache: ResMut>, views: Query<&MainEntity, With>, @@ -608,13 +608,13 @@ impl Default for EntitiesNeedingSpecialization { } #[derive(Clone, Resource, Deref, DerefMut, Debug)] -pub struct EntitySpecializationTicks { +pub struct EntitySpecializationTickPair { #[deref] pub entities: MainEntityHashMap, _marker: PhantomData, } -impl Default for EntitySpecializationTicks { +impl Default for EntitySpecializationTickPair { fn default() -> Self { Self { entities: MainEntityHashMap::default(), @@ -702,7 +702,7 @@ pub fn specialize_material2d_meshes( alpha_mask_render_phases: Res>, views: Query<(&MainEntity, &ExtractedView, &RenderVisibleEntities)>, view_key_cache: Res, - entity_specialization_ticks: Res>, + entity_specialization_ticks: Res>, view_specialization_ticks: Res, ticks: SystemChangeTick, mut specialized_material_pipeline_cache: ResMut>, diff --git a/examples/3d/manual_material.rs b/examples/3d/manual_material.rs index 8d90fe9ad79f5..8eb7f34062c10 100644 --- a/examples/3d/manual_material.rs +++ b/examples/3d/manual_material.rs @@ -8,8 +8,10 @@ use bevy::{ SystemChangeTick, SystemParamItem, }, pbr::{ - DrawMaterial, EntitiesNeedingSpecialization, EntitySpecializationTicks, - MaterialBindGroupAllocator, MaterialBindGroupAllocators, MaterialDrawFunction, + late_sweep_material_instances, DrawMaterial, EntitiesNeedingSpecialization, + EntitySpecializationTickPair, EntitySpecializationTicks, MaterialBindGroupAllocator, + MaterialBindGroupAllocators, MaterialDrawFunction, + MaterialExtractEntitiesNeedingSpecializationSystems, MaterialExtractionSystems, MaterialFragmentShader, MaterialProperties, PreparedMaterial, RenderMaterialBindings, RenderMaterialInstance, RenderMaterialInstances, SpecializedMaterialPipelineCache, }, @@ -66,7 +68,12 @@ impl Plugin for ImageMaterialPlugin { ExtractSchedule, ( extract_image_materials, - extract_image_materials_needing_specialization, + extract_image_materials_needing_specialization + .in_set(MaterialExtractEntitiesNeedingSpecializationSystems), + sweep_image_materials_needing_specialization + .after(MaterialExtractEntitiesNeedingSpecializationSystems) + .after(MaterialExtractionSystems) + .before(late_sweep_material_instances), ), ); } @@ -285,6 +292,7 @@ fn extract_image_materials_needing_specialization( mut entity_specialization_ticks: ResMut, mut removed_mesh_material_components: Extract>, mut specialized_material_pipeline_cache: ResMut, + render_material_instances: Res, views: Query<&ExtractedView>, ticks: SystemChangeTick, ) { @@ -304,6 +312,44 @@ fn extract_image_materials_needing_specialization( for entity in entities_needing_specialization.iter() { // Update the entity's specialization tick with this run's tick - entity_specialization_ticks.insert((*entity).into(), ticks.this_run()); + entity_specialization_ticks.insert( + (*entity).into(), + EntitySpecializationTickPair { + system_tick: ticks.this_run(), + material_instances_tick: render_material_instances.current_change_tick, + }, + ); + } +} + +fn sweep_image_materials_needing_specialization( + mut entity_specialization_ticks: ResMut, + mut removed_mesh_material_components: Extract>, + mut specialized_material_pipeline_cache: ResMut, + render_material_instances: Res, + views: Query<&ExtractedView>, +) { + // Clean up any despawned entities, we do this first in case the removed material was re-added + // the same frame, thus will appear both in the removed components list and have been added to + // the `EntitiesNeedingSpecialization` collection by triggering the `Changed` filter + for entity in removed_mesh_material_components.read() { + if entity_specialization_ticks + .get(&MainEntity::from(entity)) + .is_some_and(|ticks| { + ticks.material_instances_tick == render_material_instances.current_change_tick + }) + { + continue; + } + + entity_specialization_ticks.remove(&MainEntity::from(entity)); + + for view in views { + if let Some(cache) = + specialized_material_pipeline_cache.get_mut(&view.retained_view_entity) + { + cache.remove(&MainEntity::from(entity)); + } + } } } From 2426625d848dd2ff4220c9f886043b359f4d6df1 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sun, 5 Oct 2025 23:16:12 -0700 Subject: [PATCH 4/4] Doc check police --- crates/bevy_pbr/src/material.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index 3fe645b549a68..711e9e17c78dd 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -762,7 +762,7 @@ fn early_sweep_material_instances( /// Removes mesh materials from [`RenderMaterialInstances`] when their /// [`ViewVisibility`] components are removed. /// -/// This runs after all invocations of [`early_sweep_material_instances`] and is +/// 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. pub fn late_sweep_material_instances(