diff --git a/Cargo.toml b/Cargo.toml index 7bd99fc3a2aea..a25c5c72d3fef 100644 --- a/Cargo.toml +++ b/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" diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index 81f1c83609046..cb1c10e9ed693 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/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( } } +/// 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>, + pub skin_uniforms: Res<'w, SkinUniforms>, +} + pub fn specialize_material_meshes( + params: SpecializeParams, render_meshes: Res>, render_materials: Res>>, render_mesh_instances: Res, @@ -822,12 +831,10 @@ pub fn specialize_material_meshes( ), views: Query<(&ExtractedView, &RenderVisibleEntities)>, view_key_cache: Res, - entity_specialization_ticks: Res>, view_specialization_ticks: Res, mut specialized_material_pipeline_cache: ResMut>, mut pipelines: ResMut>>, pipeline: Res>, - pipeline_cache: Res, ticks: SystemChangeTick, ) where M::Data: PartialEq + Eq + Hash + Clone, @@ -859,7 +866,10 @@ pub fn specialize_material_meshes( .or_default(); for (_, visible_entity) in visible_entities.iter::() { - 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( 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( .get_extra_data(material.binding.slot) .clone(), }; - let pipeline_id = pipelines.specialize(&pipeline_cache, &pipeline, key, &mesh.layout); + let pipeline_id = + pipelines.specialize(¶ms.pipeline_cache, &pipeline, key, &mesh.layout); let pipeline_id = match pipeline_id { Ok(id) => id, Err(err) => { diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index ea60f42667815..bd5dbed8a2bb7 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/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( + params: SpecializeParams, render_meshes: Res>, render_materials: Res>>, render_mesh_instances: Res, @@ -924,17 +924,13 @@ pub fn specialize_prepass_material_meshes( ticks, prepass_pipeline, mut pipelines, - pipeline_cache, view_specialization_ticks, - entity_specialization_ticks, ): ( ResMut>, SystemChangeTick, Res>, ResMut>>, - Res, Res, - Res>, ), ) where M: Material, @@ -962,7 +958,10 @@ pub fn specialize_prepass_material_meshes( .or_default(); for (_, visible_entity) in visible_entities.iter::() { - 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( 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( } let pipeline_id = pipelines.specialize( - &pipeline_cache, + ¶ms.pipeline_cache, &prepass_pipeline, MaterialPipelineKey { mesh_key, diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index c17b9d9355287..ca7d345963605 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -1718,6 +1718,7 @@ pub fn check_views_lights_need_specialization( } pub fn specialize_shadows( + params: SpecializeParams, prepass_pipeline: Res>, ( render_meshes, @@ -1734,7 +1735,6 @@ pub fn specialize_shadows( ), shadow_render_phases: Res>, mut pipelines: ResMut>>, - pipeline_cache: Res, render_lightmaps: Res, view_lights: Query<(Entity, &ViewLightEntities), With>, view_light_entities: Query<(&LightEntity, &ExtractedView)>, @@ -1747,7 +1747,6 @@ pub fn specialize_shadows( light_key_cache: Res, mut specialized_material_pipeline_cache: ResMut>, light_specialization_ticks: Res, - entity_specialization_ticks: Res>, ticks: SystemChangeTick, ) where M::Data: PartialEq + Eq + Hash + Clone, @@ -1809,7 +1808,10 @@ pub fn specialize_shadows( .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( 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( _ => MeshPipelineKey::NONE, }; let pipeline_id = pipelines.specialize( - &pipeline_cache, + ¶ms.pipeline_cache, &prepass_pipeline, MaterialPipelineKey { mesh_key, diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs index 433df17c54e4e..9b757e77983eb 100644 --- a/crates/bevy_pbr/src/render/mesh.rs +++ b/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, - skin_uniforms: Res, morph_indices: Res, ) { - 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, vertex_attributes: &mut Vec, 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, diff --git a/crates/bevy_pbr/src/render/skin.rs b/crates/bevy_pbr/src/render/skin.rs index bec846a038e6a..5495bbd955b2d 100644 --- a/crates/bevy_pbr/src/render/skin.rs +++ b/crates/bevy_pbr/src/render/skin.rs @@ -167,9 +167,9 @@ impl SkinUniforms { }) } - /// Returns an iterator over all skins in the scene. - pub fn all_skins(&self) -> impl Iterator { - self.skin_uniform_info.keys() + /// Returns true if the given entity has a skin. + pub fn contains(&self, skin: MainEntity) -> bool { + self.skin_uniform_info.contains_key(&skin) } } @@ -229,12 +229,25 @@ pub fn prepare_skins( } else { BufferUsages::STORAGE } | BufferUsages::COPY_DST; + uniform.current_buffer = render_device.create_buffer(&BufferDescriptor { label: Some("skin uniform buffer"), usage: buffer_usages, size: new_size, mapped_at_creation: false, }); + uniform.prev_buffer = render_device.create_buffer(&BufferDescriptor { + label: Some("skin uniform buffer"), + usage: buffer_usages, + size: new_size, + mapped_at_creation: false, + }); + // TODO: This is wrong. We should be using last frame's data for prev_buffer. + render_queue.write_buffer( + &uniform.prev_buffer, + 0, + bytemuck::must_cast_slice(&uniform.current_staging_buffer[..]), + ); } // Write the data from `uniform.current_staging_buffer` into diff --git a/tests/3d/invalid_skinned_mesh.rs b/tests/3d/invalid_skinned_mesh.rs new file mode 100644 index 0000000000000..dfa467c3df4e9 --- /dev/null +++ b/tests/3d/invalid_skinned_mesh.rs @@ -0,0 +1,219 @@ +//! Test that the renderer can handle invalid skinned meshes, and meshes that +//! have skinning attributes but aren't instantiated as skinned. + +use bevy::{ + core_pipeline::motion_blur::MotionBlur, + math::ops, + prelude::*, + render::{ + camera::ScalingMode, + mesh::{ + skinning::{SkinnedMesh, SkinnedMeshInverseBindposes}, + Indices, PrimitiveTopology, VertexAttributeValues, + }, + render_asset::RenderAssetUsages, + }, +}; +use std::f32::consts::TAU; + +fn main() { + App::new() + .add_plugins(DefaultPlugins) + .insert_resource(AmbientLight { + brightness: 20_000.0, + ..default() + }) + .add_systems(Startup, (setup_environment, setup_meshes)) + .add_systems(Update, update_animated_joints) + .run(); +} + +fn setup_environment( + mut commands: Commands, + mut mesh_assets: ResMut>, + mut material_assets: ResMut>, +) { + let description = "(left to right)\n\ + 0: Normal skinned mesh.\n\ + 1: Mesh asset is missing joint index and joint weight attributes.\n\ + 2: One joint entity has been deleted.\n\ + 3: Mesh entity is missing SkinnedMesh component."; + + commands.spawn(( + Text::new(description), + Node { + position_type: PositionType::Absolute, + top: Val::Px(12.0), + left: Val::Px(12.0), + ..default() + }, + )); + + commands.spawn(( + Camera3d::default(), + Transform::from_xyz(0.0, 0.0, 1.0).looking_at(Vec3::new(0.0, 0.0, 0.0), Vec3::Y), + Projection::Orthographic(OrthographicProjection { + scaling_mode: ScalingMode::AutoMin { + min_width: 19.0, + min_height: 6.0, + }, + ..OrthographicProjection::default_3d() + }), + // Add motion blur so that we can check if it's working for skinned + // meshes. This also exercises the renderer's prepass path. + MotionBlur { + // Use an unrealistically large shutter angle so that motion blur is clearly visible. + shutter_angle: 3.0, + samples: 2, + #[cfg(all(feature = "webgl2", target_arch = "wasm32", not(feature = "webgpu")))] + _webgl2_padding: Default::default(), + }, + // MSAA and MotionBlur together are not compatible on WebGL. + #[cfg(all(feature = "webgl2", target_arch = "wasm32", not(feature = "webgpu")))] + Msaa::Off, + )); + + // Add a directional light to make sure we exercise the renderer's shadow path. + commands.spawn(( + Transform::from_xyz(1.0, 1.0, 3.0).looking_at(Vec3::ZERO, Vec3::Y), + DirectionalLight { + shadows_enabled: true, + ..default() + }, + )); + + // Add a plane behind the meshes so that we can see if shadows are working. + commands.spawn(( + Transform::from_xyz(0.0, 0.0, -1.0), + Mesh3d(mesh_assets.add(Plane3d::default().mesh().size(100.0, 100.0).normal(Dir3::Z))), + MeshMaterial3d(material_assets.add(StandardMaterial { + base_color: Color::srgb(0.05, 0.05, 0.15), + reflectance: 0.2, + ..default() + })), + )); +} + +fn setup_meshes( + mut commands: Commands, + mut mesh_assets: ResMut>, + mut material_assets: ResMut>, + mut inverse_bindposes_assets: ResMut>, +) { + // Create a mesh with two rectangles. + let unskinned_mesh = Mesh::new( + PrimitiveTopology::TriangleList, + RenderAssetUsages::default(), + ) + .with_inserted_attribute( + Mesh::ATTRIBUTE_POSITION, + vec![ + [-0.3, -0.3, 0.0], + [0.3, -0.3, 0.0], + [-0.3, 0.3, 0.0], + [0.3, 0.3, 0.0], + [-0.4, 0.8, 0.0], + [0.4, 0.8, 0.0], + [-0.4, 1.8, 0.0], + [0.4, 1.8, 0.0], + ], + ) + .with_inserted_attribute(Mesh::ATTRIBUTE_NORMAL, vec![[0.0, 0.0, 1.0]; 8]) + .with_inserted_indices(Indices::U16(vec![0, 1, 3, 0, 3, 2, 4, 5, 7, 4, 7, 6])); + + // Copy the mesh and add skinning attributes that bind each rectangle to a joint. + let skinned_mesh = unskinned_mesh + .clone() + .with_inserted_attribute( + Mesh::ATTRIBUTE_JOINT_INDEX, + VertexAttributeValues::Uint16x4(vec![ + [0, 0, 0, 0], + [0, 0, 0, 0], + [0, 0, 0, 0], + [0, 0, 0, 0], + [1, 0, 0, 0], + [1, 0, 0, 0], + [1, 0, 0, 0], + [1, 0, 0, 0], + ]), + ) + .with_inserted_attribute( + Mesh::ATTRIBUTE_JOINT_WEIGHT, + vec![[1.00, 0.00, 0.0, 0.0]; 8], + ); + + let unskinned_mesh_handle = mesh_assets.add(unskinned_mesh); + let skinned_mesh_handle = mesh_assets.add(skinned_mesh); + + let inverse_bindposes_handle = inverse_bindposes_assets.add(vec![ + Mat4::IDENTITY, + Mat4::from_translation(Vec3::new(0.0, -1.3, 0.0)), + ]); + + let mesh_material_handle = material_assets.add(StandardMaterial::default()); + + let background_material_handle = material_assets.add(StandardMaterial { + base_color: Color::srgb(0.05, 0.15, 0.05), + reflectance: 0.2, + ..default() + }); + + for mesh_index in 0..4 { + let transform = Transform::from_xyz(((mesh_index as f32) - 1.5) * 4.5, 0.0, 0.0); + + let joint_0 = commands.spawn(transform).id(); + + let joint_1 = commands + .spawn(( + ChildOf { parent: joint_0 }, + AnimatedJoint, + Transform::IDENTITY, + )) + .id(); + + // Optionally delete one joint entity. + if mesh_index == 2 { + commands.entity(joint_1).despawn(); + } + + // Optionally use the mesh without skinning attributes. + let mesh_handle = match mesh_index { + 1 => &unskinned_mesh_handle, + _ => &skinned_mesh_handle, + }; + + let mut entity_commands = commands.spawn(( + Mesh3d(mesh_handle.clone()), + MeshMaterial3d(mesh_material_handle.clone()), + transform, + )); + + // Optionally add the SkinnedMesh component. + if mesh_index != 3 { + entity_commands.insert(SkinnedMesh { + inverse_bindposes: inverse_bindposes_handle.clone(), + joints: vec![joint_0, joint_1], + }); + } + + // Add a square behind the mesh to distinguish it from the other meshes. + commands.spawn(( + Transform::from_xyz(transform.translation.x, transform.translation.y, -0.8), + Mesh3d(mesh_assets.add(Plane3d::default().mesh().size(4.3, 4.3).normal(Dir3::Z))), + MeshMaterial3d(background_material_handle.clone()), + )); + } +} + +#[derive(Component)] +struct AnimatedJoint; + +fn update_animated_joints(time: Res