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

Cargo.toml

@@ -4098,6 +4098,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

@@ -827,7 +827,8 @@ pub fn specialize_material_meshes<M: Material>(
     mut specialized_material_pipeline_cache: ResMut<SpecializedMaterialPipelineCache<M>>,
     mut pipelines: ResMut<SpecializedMeshPipelines<MaterialPipeline<M>>>,
     pipeline: Res<MaterialPipeline<M>>,
-    pipeline_cache: Res<PipelineCache>,
+    (pipeline_cache, skin_uniforms): (Res<PipelineCache>, Res<SkinUniforms>),
+
     ticks: SystemChangeTick,
 ) where
     M::Data: PartialEq + Eq + Hash + Clone,
@@ -910,14 +911,17 @@ pub fn specialize_material_meshes<M: Material>(
                 mesh_key |= MeshPipelineKey::VISIBILITY_RANGE_DITHER;
             }
 
+            let is_skinned = 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)

crates/bevy_pbr/src/prepass/mod.rs

@@ -7,7 +7,7 @@ use crate::{
     EntitySpecializationTicks, Material, MaterialPipeline, MaterialPipelineKey, MeshLayouts,
     MeshPipeline, MeshPipelineKey, OpaqueRendererMethod, PreparedMaterial, RenderLightmaps,
     RenderMaterialInstances, RenderMeshInstanceFlags, RenderMeshInstances, RenderPhaseType,
-    SetMaterialBindGroup, SetMeshBindGroup, ShadowView, StandardMaterial,
+    SetMaterialBindGroup, SetMeshBindGroup, ShadowView, SkinUniforms, 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,
@@ -901,6 +900,7 @@ pub fn specialize_prepass_material_meshes<M>(
     render_visibility_ranges: Res<RenderVisibilityRanges>,
     material_bind_group_allocator: Res<MaterialBindGroupAllocator<M>>,
     view_key_cache: Res<ViewKeyPrepassCache>,
+    skin_uniforms: Res<SkinUniforms>,
     views: Query<(
         &ExtractedView,
         &RenderVisibleEntities,
@@ -1042,14 +1042,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 = 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)

crates/bevy_pbr/src/render/light.rs

@@ -1747,7 +1747,10 @@ 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>>,
+    (entity_specialization_ticks, skin_uniforms): (
+        Res<EntitySpecializationTicks<M>>,
+        Res<SkinUniforms>,
+    ),
     ticks: SystemChangeTick,
 ) where
     M::Data: PartialEq + Eq + Hash + Clone,
@@ -1862,6 +1865,10 @@ pub fn specialize_shadows<M: Material>(
                     mesh_key |= MeshPipelineKey::LIGHTMAPPED;
                 }
 
+                if skin_uniforms.contains(visible_entity) {
+                    mesh_key |= MeshPipelineKey::SKINNED;
+                }
+
                 mesh_key |= match material.properties.alpha_mode {
                     AlphaMode::Mask(_)
                     | AlphaMode::Blend

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.
-fn set_mesh_motion_vector_flags(
+/// 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.
+pub 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,13 @@ pub fn prepare_mesh_bind_groups(
     }
 }
 
+/// This should match the `is_skinned` function in `skin.rs`, which takes a
+/// `Mesh` instead of a `MeshVertexBufferLayoutRef`.
+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,