Fix panic when a mesh has skinning attributes but no SkinnedMesh component

Cargo.toml

@@ -4102,6 +4102,14 @@ description = "Demonstrates specular tints and maps"
 category = "3D Rendering"
 wasm = true
 
+[[example]]
+name = "invalid_skinned_mesh"
+path = "tests/3d/invalid_skinned_mesh.rs"
+doc-scrape-examples = true
+
+[package.metadata.example.invalid_skinned_mesh]
+hidden = true
+
 [profile.wasm-release]
 inherits = "release"
 opt-level = "z"

crates/bevy_pbr/src/material.rs

@@ -21,7 +21,7 @@ use bevy_core_pipeline::{
 };
 use bevy_derive::{Deref, DerefMut};
 use bevy_ecs::component::Tick;
-use bevy_ecs::system::SystemChangeTick;
+use bevy_ecs::system::{SystemChangeTick, SystemParam};
 use bevy_ecs::{
     prelude::*,
     system::{
@@ -800,7 +800,16 @@ pub fn check_entities_needing_specialization<M>(
     }
 }
 
+/// Parameters shared between specialize systems.
+#[derive(SystemParam)]
+pub struct SpecializeParams<'w, M: Material> {
+    pub pipeline_cache: Res<'w, PipelineCache>,
+    pub entity_specialization_ticks: Res<'w, EntitySpecializationTicks<M>>,
+    pub skin_uniforms: Res<'w, SkinUniforms>,
+}
+
 pub fn specialize_material_meshes<M: Material>(
+    params: SpecializeParams<M>,
     render_meshes: Res<RenderAssets<RenderMesh>>,
     render_materials: Res<RenderAssets<PreparedMaterial<M>>>,
     render_mesh_instances: Res<RenderMeshInstances>,
@@ -822,12 +831,10 @@ pub fn specialize_material_meshes<M: Material>(
     ),
     views: Query<(&ExtractedView, &RenderVisibleEntities)>,
     view_key_cache: Res<ViewKeyCache>,
-    entity_specialization_ticks: Res<EntitySpecializationTicks<M>>,
     view_specialization_ticks: Res<ViewSpecializationTicks>,
     mut specialized_material_pipeline_cache: ResMut<SpecializedMaterialPipelineCache<M>>,
     mut pipelines: ResMut<SpecializedMeshPipelines<MaterialPipeline<M>>>,
     pipeline: Res<MaterialPipeline<M>>,
-    pipeline_cache: Res<PipelineCache>,
     ticks: SystemChangeTick,
 ) where
     M::Data: PartialEq + Eq + Hash + Clone,
@@ -859,7 +866,10 @@ pub fn specialize_material_meshes<M: Material>(
             .or_default();
 
         for (_, visible_entity) in visible_entities.iter::<Mesh3d>() {
-            let entity_tick = entity_specialization_ticks.get(visible_entity).unwrap();
+            let entity_tick = params
+                .entity_specialization_ticks
+                .get(visible_entity)
+                .unwrap();
             let last_specialized_tick = view_specialized_material_pipeline_cache
                 .get(visible_entity)
                 .map(|(tick, _)| *tick);
@@ -910,14 +920,17 @@ pub fn specialize_material_meshes<M: Material>(
                 mesh_key |= MeshPipelineKey::VISIBILITY_RANGE_DITHER;
             }
 
+            let is_skinned = params.skin_uniforms.contains(*visible_entity);
+
+            if is_skinned {
+                mesh_key |= MeshPipelineKey::SKINNED;
+            }
+
             if view_key.contains(MeshPipelineKey::MOTION_VECTOR_PREPASS) {
-                // If the previous frame have skins or morph targets, note that.
-                if mesh_instance
-                    .flags
-                    .contains(RenderMeshInstanceFlags::HAS_PREVIOUS_SKIN)
-                {
+                if is_skinned {
                     mesh_key |= MeshPipelineKey::HAS_PREVIOUS_SKIN;
                 }
+                // If the previous frame has morph targets, note that.
                 if mesh_instance
                     .flags
                     .contains(RenderMeshInstanceFlags::HAS_PREVIOUS_MORPH)
@@ -932,7 +945,8 @@ pub fn specialize_material_meshes<M: Material>(
                     .get_extra_data(material.binding.slot)
                     .clone(),
             };
-            let pipeline_id = pipelines.specialize(&pipeline_cache, &pipeline, key, &mesh.layout);
+            let pipeline_id =
+                pipelines.specialize(&params.pipeline_cache, &pipeline, key, &mesh.layout);
             let pipeline_id = match pipeline_id {
                 Ok(id) => id,
                 Err(err) => {

crates/bevy_pbr/src/prepass/mod.rs

@@ -3,11 +3,11 @@ mod prepass_bindings;
 use crate::{
     alpha_mode_pipeline_key, binding_arrays_are_usable, buffer_layout,
     collect_meshes_for_gpu_building, material_bind_groups::MaterialBindGroupAllocator,
-    queue_material_meshes, setup_morph_and_skinning_defs, skin, DrawMesh,
-    EntitySpecializationTicks, Material, MaterialPipeline, MaterialPipelineKey, MeshLayouts,
-    MeshPipeline, MeshPipelineKey, OpaqueRendererMethod, PreparedMaterial, RenderLightmaps,
-    RenderMaterialInstances, RenderMeshInstanceFlags, RenderMeshInstances, RenderPhaseType,
-    SetMaterialBindGroup, SetMeshBindGroup, ShadowView, StandardMaterial,
+    queue_material_meshes, setup_morph_and_skinning_defs, skin, DrawMesh, Material,
+    MaterialPipeline, MaterialPipelineKey, MeshLayouts, MeshPipeline, MeshPipelineKey,
+    OpaqueRendererMethod, PreparedMaterial, RenderLightmaps, RenderMaterialInstances,
+    RenderMeshInstanceFlags, RenderMeshInstances, RenderPhaseType, SetMaterialBindGroup,
+    SetMeshBindGroup, ShadowView, SpecializeParams, StandardMaterial,
 };
 use bevy_app::{App, Plugin, PreUpdate};
 use bevy_render::{
@@ -589,7 +589,6 @@ where
 
         let bind_group = setup_morph_and_skinning_defs(
             &self.mesh_layouts,
-            layout,
             5,
             &key.mesh_key,
             &mut shader_defs,
@@ -893,6 +892,7 @@ pub fn check_prepass_views_need_specialization(
 }
 
 pub fn specialize_prepass_material_meshes<M>(
+    params: SpecializeParams<M>,
     render_meshes: Res<RenderAssets<RenderMesh>>,
     render_materials: Res<RenderAssets<PreparedMaterial<M>>>,
     render_mesh_instances: Res<RenderMeshInstances>,
@@ -924,17 +924,13 @@ pub fn specialize_prepass_material_meshes<M>(
         ticks,
         prepass_pipeline,
         mut pipelines,
-        pipeline_cache,
         view_specialization_ticks,
-        entity_specialization_ticks,
     ): (
         ResMut<SpecializedPrepassMaterialPipelineCache<M>>,
         SystemChangeTick,
         Res<PrepassPipeline<M>>,
         ResMut<SpecializedMeshPipelines<PrepassPipeline<M>>>,
-        Res<PipelineCache>,
         Res<ViewPrepassSpecializationTicks>,
-        Res<EntitySpecializationTicks<M>>,
     ),
 ) where
     M: Material,
@@ -962,7 +958,10 @@ pub fn specialize_prepass_material_meshes<M>(
             .or_default();
 
         for (_, visible_entity) in visible_entities.iter::<Mesh3d>() {
-            let entity_tick = entity_specialization_ticks.get(visible_entity).unwrap();
+            let entity_tick = params
+                .entity_specialization_ticks
+                .get(visible_entity)
+                .unwrap();
             let last_specialized_tick = view_specialized_material_pipeline_cache
                 .get(visible_entity)
                 .map(|(tick, _)| *tick);
@@ -1042,14 +1041,17 @@ pub fn specialize_prepass_material_meshes<M>(
                 mesh_key |= MeshPipelineKey::VISIBILITY_RANGE_DITHER;
             }
 
-            // If the previous frame has skins or morph targets, note that.
+            let is_skinned = params.skin_uniforms.contains(*visible_entity);
+
+            if is_skinned {
+                mesh_key |= MeshPipelineKey::SKINNED;
+            }
+
             if motion_vector_prepass.is_some() {
-                if mesh_instance
-                    .flags
-                    .contains(RenderMeshInstanceFlags::HAS_PREVIOUS_SKIN)
-                {
+                if is_skinned {
                     mesh_key |= MeshPipelineKey::HAS_PREVIOUS_SKIN;
                 }
+                // If the previous frame has morph targets, note that.
                 if mesh_instance
                     .flags
                     .contains(RenderMeshInstanceFlags::HAS_PREVIOUS_MORPH)
@@ -1059,7 +1061,7 @@ pub fn specialize_prepass_material_meshes<M>(
             }
 
             let pipeline_id = pipelines.specialize(
-                &pipeline_cache,
+                &params.pipeline_cache,
                 &prepass_pipeline,
                 MaterialPipelineKey {
                     mesh_key,

crates/bevy_pbr/src/render/light.rs

@@ -1718,6 +1718,7 @@ pub fn check_views_lights_need_specialization(
 }
 
 pub fn specialize_shadows<M: Material>(
+    params: SpecializeParams<M>,
     prepass_pipeline: Res<PrepassPipeline<M>>,
     (
         render_meshes,
@@ -1734,7 +1735,6 @@ pub fn specialize_shadows<M: Material>(
     ),
     shadow_render_phases: Res<ViewBinnedRenderPhases<Shadow>>,
     mut pipelines: ResMut<SpecializedMeshPipelines<PrepassPipeline<M>>>,
-    pipeline_cache: Res<PipelineCache>,
     render_lightmaps: Res<RenderLightmaps>,
     view_lights: Query<(Entity, &ViewLightEntities), With<ExtractedView>>,
     view_light_entities: Query<(&LightEntity, &ExtractedView)>,
@@ -1747,7 +1747,6 @@ pub fn specialize_shadows<M: Material>(
     light_key_cache: Res<LightKeyCache>,
     mut specialized_material_pipeline_cache: ResMut<SpecializedShadowMaterialPipelineCache<M>>,
     light_specialization_ticks: Res<LightSpecializationTicks>,
-    entity_specialization_ticks: Res<EntitySpecializationTicks<M>>,
     ticks: SystemChangeTick,
 ) where
     M::Data: PartialEq + Eq + Hash + Clone,
@@ -1809,7 +1808,10 @@ pub fn specialize_shadows<M: Material>(
                 .or_default();
 
             for (_, visible_entity) in visible_entities.iter().copied() {
-                let entity_tick = entity_specialization_ticks.get(&visible_entity).unwrap();
+                let entity_tick = params
+                    .entity_specialization_ticks
+                    .get(&visible_entity)
+                    .unwrap();
                 let last_specialized_tick = view_specialized_material_pipeline_cache
                     .get(&visible_entity)
                     .map(|(tick, _)| *tick);
@@ -1862,6 +1864,10 @@ pub fn specialize_shadows<M: Material>(
                     mesh_key |= MeshPipelineKey::LIGHTMAPPED;
                 }
 
+                if params.skin_uniforms.contains(visible_entity) {
+                    mesh_key |= MeshPipelineKey::SKINNED;
+                }
+
                 mesh_key |= match material.properties.alpha_mode {
                     AlphaMode::Mask(_)
                     | AlphaMode::Blend
@@ -1871,7 +1877,7 @@ pub fn specialize_shadows<M: Material>(
                     _ => MeshPipelineKey::NONE,
                 };
                 let pipeline_id = pipelines.specialize(
-                    &pipeline_cache,
+                    &params.pipeline_cache,
                     &prepass_pipeline,
                     MaterialPipelineKey {
                         mesh_key,

crates/bevy_pbr/src/render/mesh.rs

@@ -699,12 +699,9 @@ bitflags::bitflags! {
         /// The mesh had a transform last frame and so is eligible for motion
         /// vector computation.
         const HAS_PREVIOUS_TRANSFORM  = 1 << 2;
-        /// The mesh had a skin last frame and so that skin should be taken into
-        /// account for motion vector computation.
-        const HAS_PREVIOUS_SKIN       = 1 << 3;
         /// The mesh had morph targets last frame and so they should be taken
         /// into account for motion vector computation.
-        const HAS_PREVIOUS_MORPH      = 1 << 4;
+        const HAS_PREVIOUS_MORPH      = 1 << 3;
     }
 }
 
@@ -1629,29 +1626,22 @@ fn extract_mesh_for_gpu_building(
     );
 }
 
-/// A system that sets the [`RenderMeshInstanceFlags`] for each mesh based on
-/// whether the previous frame had skins and/or morph targets.
+/// A system that sets [`RenderMeshInstanceFlags::HAS_PREVIOUS_MORPH`] for each
+/// mesh.
 ///
 /// Ordinarily, [`RenderMeshInstanceFlags`] are set during the extraction phase.
-/// However, we can't do that for the flags related to skins and morph targets
-/// because the previous frame's skin and morph targets are the responsibility
-/// of [`extract_skins`] and [`extract_morphs`] respectively. We want to run
-/// those systems in parallel with mesh extraction for performance, so we need
-/// to defer setting of these mesh instance flags to after extraction, which
-/// this system does. An alternative to having skin- and morph-target-related
-/// data in [`RenderMeshInstanceFlags`] would be to have
-/// [`crate::material::queue_material_meshes`] check the skin and morph target
-/// tables for each mesh, but that would be too slow in the hot mesh queuing
-/// loop.
+/// However, we can't do that for the flags related to morph targets because the
+/// previous frame's morph targets are the responsibility of [`extract_morphs`].
+/// We want to run that system in parallel with mesh extraction for performance,
+/// so we need to defer setting of the mesh instance flag to after extraction,
+/// which this system does. An alternative to having morph target related data
+/// in [`RenderMeshInstanceFlags`] would be to have
+/// [`crate::material::queue_material_meshes`] check the morph target tables for
+/// each mesh, but that would be too slow in the hot mesh queuing loop.
 fn set_mesh_motion_vector_flags(
     mut render_mesh_instances: ResMut<RenderMeshInstances>,
-    skin_uniforms: Res<SkinUniforms>,
     morph_indices: Res<MorphIndices>,
 ) {
-    for &entity in skin_uniforms.all_skins() {
-        render_mesh_instances
-            .insert_mesh_instance_flags(entity, RenderMeshInstanceFlags::HAS_PREVIOUS_SKIN);
-    }
     for &entity in morph_indices.prev.keys() {
         render_mesh_instances
             .insert_mesh_instance_flags(entity, RenderMeshInstanceFlags::HAS_PREVIOUS_MORPH);
@@ -2092,10 +2082,11 @@ bitflags::bitflags! {
         const IRRADIANCE_VOLUME                 = 1 << 15;
         const VISIBILITY_RANGE_DITHER           = 1 << 16;
         const SCREEN_SPACE_REFLECTIONS          = 1 << 17;
-        const HAS_PREVIOUS_SKIN                 = 1 << 18;
-        const HAS_PREVIOUS_MORPH                = 1 << 19;
-        const OIT_ENABLED                       = 1 << 20;
-        const DISTANCE_FOG                      = 1 << 21;
+        const SKINNED                           = 1 << 18;
+        const HAS_PREVIOUS_SKIN                 = 1 << 19;
+        const HAS_PREVIOUS_MORPH                = 1 << 20;
+        const OIT_ENABLED                       = 1 << 21;
+        const DISTANCE_FOG                      = 1 << 22;
         const LAST_FLAG                         = Self::DISTANCE_FOG.bits();
 
         // Bitfields
@@ -2218,19 +2209,15 @@ const_assert_eq!(
     0
 );
 
-fn is_skinned(layout: &MeshVertexBufferLayoutRef) -> bool {
-    layout.0.contains(Mesh::ATTRIBUTE_JOINT_INDEX)
-        && layout.0.contains(Mesh::ATTRIBUTE_JOINT_WEIGHT)
-}
 pub fn setup_morph_and_skinning_defs(
     mesh_layouts: &MeshLayouts,
-    layout: &MeshVertexBufferLayoutRef,
     offset: u32,
     key: &MeshPipelineKey,
     shader_defs: &mut Vec<ShaderDefVal>,
     vertex_attributes: &mut Vec<VertexAttributeDescriptor>,
     skins_use_uniform_buffers: bool,
 ) -> BindGroupLayout {
+    let is_skinned = key.intersects(MeshPipelineKey::SKINNED);
     let is_morphed = key.intersects(MeshPipelineKey::MORPH_TARGETS);
     let is_lightmapped = key.intersects(MeshPipelineKey::LIGHTMAPPED);
     let motion_vector_prepass = key.intersects(MeshPipelineKey::MOTION_VECTOR_PREPASS);
@@ -2246,7 +2233,7 @@ pub fn setup_morph_and_skinning_defs(
     };
 
     match (
-        is_skinned(layout),
+        is_skinned,
         is_morphed,
         is_lightmapped,
         motion_vector_prepass,
@@ -2351,7 +2338,6 @@ impl SpecializedMeshPipeline for MeshPipeline {
 
         bind_group_layout.push(setup_morph_and_skinning_defs(
             &self.mesh_layouts,
-            layout,
             6,
             &key,
             &mut shader_defs,
@@ -2789,6 +2775,11 @@ pub fn prepare_mesh_bind_groups(
     }
 }
 
+fn is_skinned(layout: &MeshVertexBufferLayoutRef) -> bool {
+    layout.0.contains(Mesh::ATTRIBUTE_JOINT_INDEX)
+        && layout.0.contains(Mesh::ATTRIBUTE_JOINT_WEIGHT)
+}
+
 /// Creates the per-mesh bind groups for each type of mesh, for a single phase.
 fn prepare_mesh_bind_groups_for_phase(
     model: BindingResource,