From 4ac528a57905620784026f937761ccbfbb411a65 Mon Sep 17 00:00:00 2001 From: akimakinai <105044389+akimakinai@users.noreply.github.com> Date: Tue, 15 Oct 2024 22:54:09 +0900 Subject: [PATCH 1/8] Despawn unused light-view entity (#15902) # Objective - Fixes #15897 ## Solution - Despawn light view entities when they go unused or when the corresponding view is not alive. ## Testing - `scene_viewer` example no longer prints "The preprocessing index buffer wasn't present" warning - modified an example to try toggling shadows for all kinds of light: https://gist.github.com/akimakinai/ddb0357191f5052b654370699d2314cf --- crates/bevy_pbr/src/render/light.rs | 128 ++++++++++++++++++++-------- 1 file changed, 91 insertions(+), 37 deletions(-) diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index 315563752aa48..9cc3fec765e68 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -4,7 +4,7 @@ use bevy_color::ColorToComponents; use bevy_core_pipeline::core_3d::{Camera3d, CORE_3D_DEPTH_FORMAT}; use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ - entity::{EntityHashMap, EntityHashSet}, + entity::{EntityHash, EntityHashMap, EntityHashSet}, prelude::*, system::lifetimeless::Read, }; @@ -459,7 +459,9 @@ fn create_render_visible_mesh_entities( } #[derive(Component, Default, Deref, DerefMut)] -pub struct LightViewEntities(Vec); +/// Component automatically attached to a light entity to track light-view entities +/// for each view. +pub struct LightViewEntities(EntityHashMap>); // TODO: using required component pub(crate) fn add_light_view_entities( @@ -477,9 +479,11 @@ pub(crate) fn remove_light_view_entities( mut commands: Commands, ) { if let Ok(entities) = query.get(trigger.entity()) { - for e in entities.0.iter().copied() { - if let Some(mut v) = commands.get_entity(e) { - v.despawn(); + for v in entities.0.values() { + for e in v.iter().copied() { + if let Some(mut v) = commands.get_entity(e) { + v.despawn(); + } } } } @@ -731,7 +735,8 @@ pub fn prepare_lights( let point_light_count = point_lights .iter() .filter(|light| light.1.spot_light_angles.is_none()) - .count(); + .count() + .min(max_texture_cubes); let point_light_volumetric_enabled_count = point_lights .iter() @@ -759,6 +764,12 @@ pub fn prepare_lights( .count() .min(max_texture_array_layers / MAX_CASCADES_PER_LIGHT); + let spot_light_count = point_lights + .iter() + .filter(|(_, light, _)| light.spot_light_angles.is_some()) + .count() + .min(max_texture_array_layers - directional_shadow_enabled_count * MAX_CASCADES_PER_LIGHT); + let spot_light_volumetric_enabled_count = point_lights .iter() .filter(|(_, light, _)| light.volumetric && light.spot_light_angles.is_some()) @@ -956,9 +967,12 @@ pub fn prepare_lights( live_shadow_mapping_lights.clear(); - let mut dir_light_view_offset = 0; + let mut live_views = EntityHashSet::with_capacity_and_hasher(views_count, EntityHash); + // set up light data for each view - for (offset, (entity, extracted_view, clusters, maybe_layers)) in views.iter().enumerate() { + for (entity, extracted_view, clusters, maybe_layers) in views.iter() { + live_views.insert(entity); + let point_light_depth_texture = texture_cache.get( &render_device, TextureDescriptor { @@ -1032,9 +1046,19 @@ pub fn prepare_lights( for &(light_entity, light, (point_light_frusta, _)) in point_lights .iter() // Lights are sorted, shadow enabled lights are first - .take(point_light_shadow_maps_count) - .filter(|(_, light, _)| light.shadows_enabled) + .take(point_light_count) { + let Ok(mut light_view_entities) = light_view_entities.get_mut(light_entity) else { + continue; + }; + + if !light.shadows_enabled { + if let Some(entities) = light_view_entities.remove(&entity) { + despawn_entities(&mut commands, entities); + } + continue; + } + let light_index = *global_light_meta .entity_to_index .get(&light_entity) @@ -1044,14 +1068,10 @@ pub fn prepare_lights( // and ignore rotation because we want the shadow map projections to align with the axes let view_translation = GlobalTransform::from_translation(light.transform.translation()); - let Ok(mut light_entities) = light_view_entities.get_mut(light_entity) else { - continue; - }; - // for each face of a cube and each view we spawn a light entity - while light_entities.len() < 6 * (offset + 1) { - light_entities.push(commands.spawn_empty().id()); - } + let light_view_entities = light_view_entities + .entry(entity) + .or_insert_with(|| (0..6).map(|_| commands.spawn_empty().id()).collect()); let cube_face_projection = Mat4::perspective_infinite_reverse_rh( core::f32::consts::FRAC_PI_2, @@ -1062,7 +1082,7 @@ pub fn prepare_lights( for (face_index, ((view_rotation, frustum), view_light_entity)) in cube_face_rotations .iter() .zip(&point_light_frusta.unwrap().frusta) - .zip(light_entities.iter().skip(6 * offset).copied()) + .zip(light_view_entities.iter().copied()) .enumerate() { let depth_texture_view = @@ -1119,16 +1139,23 @@ pub fn prepare_lights( for (light_index, &(light_entity, light, (_, spot_light_frustum))) in point_lights .iter() .skip(point_light_count) - .take(spot_light_shadow_maps_count) + .take(spot_light_count) .enumerate() { - let spot_world_from_view = spot_light_world_from_view(&light.transform); - let spot_world_from_view = spot_world_from_view.into(); - let Ok(mut light_view_entities) = light_view_entities.get_mut(light_entity) else { continue; }; + if !light.shadows_enabled { + if let Some(entities) = light_view_entities.remove(&entity) { + despawn_entities(&mut commands, entities); + } + continue; + } + + let spot_world_from_view = spot_light_world_from_view(&light.transform); + let spot_world_from_view = spot_world_from_view.into(); + let angle = light.spot_light_angles.expect("lights should be sorted so that \ [point_light_count..point_light_count + spot_light_shadow_maps_count] are spot lights").1; let spot_projection = spot_light_clip_from_view(angle, light.shadow_map_near_z); @@ -1147,11 +1174,11 @@ pub fn prepare_lights( array_layer_count: Some(1u32), }); - while light_view_entities.len() < offset + 1 { - light_view_entities.push(commands.spawn_empty().id()); - } + let light_view_entities = light_view_entities + .entry(entity) + .or_insert_with(|| vec![commands.spawn_empty().id()]); - let view_light_entity = light_view_entities[offset]; + let view_light_entity = light_view_entities[0]; commands.entity(view_light_entity).insert(( ShadowView { @@ -1194,14 +1221,21 @@ pub fn prepare_lights( let Ok(mut light_view_entities) = light_view_entities.get_mut(light_entity) else { continue; }; + // Check if the light intersects with the view. if !view_layers.intersects(&light.render_layers) { gpu_light.skip = 1u32; + if let Some(entities) = light_view_entities.remove(&entity) { + despawn_entities(&mut commands, entities); + } continue; } // Only deal with cascades when shadows are enabled. if (gpu_light.flags & DirectionalLightFlags::SHADOWS_ENABLED.bits()) == 0u32 { + if let Some(entities) = light_view_entities.remove(&entity) { + despawn_entities(&mut commands, entities); + } continue; } @@ -1222,18 +1256,19 @@ pub fn prepare_lights( .zip(frusta) .zip(&light.cascade_shadow_config.bounds); - while light_view_entities.len() < dir_light_view_offset + iter.len() { - light_view_entities.push(commands.spawn_empty().id()); + let light_view_entities = light_view_entities.entry(entity).or_insert_with(|| { + (0..iter.len()) + .map(|_| commands.spawn_empty().id()) + .collect() + }); + if light_view_entities.len() != iter.len() { + let entities = core::mem::take(light_view_entities); + despawn_entities(&mut commands, entities); + light_view_entities.extend((0..iter.len()).map(|_| commands.spawn_empty().id())); } - for (cascade_index, (((cascade, frustum), bound), view_light_entity)) in iter - .zip( - light_view_entities - .iter() - .skip(dir_light_view_offset) - .copied(), - ) - .enumerate() + for (cascade_index, (((cascade, frustum), bound), view_light_entity)) in + iter.zip(light_view_entities.iter().copied()).enumerate() { gpu_lights.directional_lights[light_index].cascades[cascade_index] = GpuDirectionalCascade { @@ -1292,7 +1327,6 @@ pub fn prepare_lights( shadow_render_phases.insert_or_clear(view_light_entity); live_shadow_mapping_lights.insert(view_light_entity); - dir_light_view_offset += 1; } } @@ -1360,9 +1394,29 @@ pub fn prepare_lights( )); } + // Despawn light-view entities for views that no longer exist + for mut entities in &mut light_view_entities { + for (_, light_view_entities) in + entities.extract_if(|entity, _| !live_views.contains(entity)) + { + despawn_entities(&mut commands, light_view_entities); + } + } + shadow_render_phases.retain(|entity, _| live_shadow_mapping_lights.contains(entity)); } +fn despawn_entities(commands: &mut Commands, entities: Vec) { + if entities.is_empty() { + return; + } + commands.queue(move |world: &mut World| { + for entity in entities { + world.despawn(entity); + } + }); +} + /// For each shadow cascade, iterates over all the meshes "visible" from it and /// adds them to [`BinnedRenderPhase`]s or [`SortedRenderPhase`]s as /// appropriate. From de08fb2afa5e8da1d910ed6be424649b6fbe9f0e Mon Sep 17 00:00:00 2001 From: Brett Striker Date: Tue, 15 Oct 2024 10:06:17 -0400 Subject: [PATCH 2/8] [bevy_ui/layout] Add tests, missing asserts, and missing debug fields for UiSurface (#12803) This is 3 of 5 iterative PR's that affect bevy_ui/layout - [x] Blocked by https://github.com/bevyengine/bevy/pull/12801 - [x] Blocked by https://github.com/bevyengine/bevy/pull/12802 --- # Objective - Add tests to `UiSurface` - Add missing asserts in `_assert_send_sync_ui_surface_impl_safe` - Add missing Debug field print for `camera_entity_to_taffy` ## Solution - Adds tests to `UiSurface` - Adds missing asserts in `_assert_send_sync_ui_surface_impl_safe` - Adds missing impl Debug field print for `camera_entity_to_taffy` --------- Co-authored-by: Alice Cecile --- crates/bevy_ui/src/layout/mod.rs | 65 +++- crates/bevy_ui/src/layout/ui_surface.rs | 410 ++++++++++++++++++++++++ crates/bevy_ui/src/lib.rs | 18 +- 3 files changed, 486 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 492f1c1e0f2d4..504e2e02074b6 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -55,6 +55,16 @@ impl LayoutContext { } } +#[cfg(test)] +impl LayoutContext { + pub const TEST_CONTEXT: Self = Self { + scale_factor: 1.0, + physical_size: Vec2::new(1000.0, 1000.0), + min_size: 0.0, + max_size: 1000.0, + }; +} + impl Default for LayoutContext { fn default() -> Self { Self::DEFAULT @@ -513,7 +523,7 @@ mod tests { prelude::*, ui_layout_system, update::update_target_camera_system, - ContentSize, + ContentSize, LayoutContext, }; #[test] @@ -1234,4 +1244,57 @@ mod tests { ui_schedule.run(&mut world); } + + #[test] + fn test_ui_surface_compute_camera_layout() { + use bevy_ecs::prelude::ResMut; + + let (mut world, ..) = setup_ui_test_world(); + + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + + struct TestSystemParam { + camera_entity: Entity, + root_node_entity: Entity, + } + + fn test_system( + params: In, + mut ui_surface: ResMut, + #[cfg(feature = "bevy_text")] mut computed_text_block_query: Query< + &mut bevy_text::ComputedTextBlock, + >, + #[cfg(feature = "bevy_text")] mut font_system: ResMut, + ) { + ui_surface.upsert_node( + &LayoutContext::TEST_CONTEXT, + params.root_node_entity, + &Style::default(), + None, + ); + + ui_surface.compute_camera_layout( + params.camera_entity, + UVec2::new(800, 600), + #[cfg(feature = "bevy_text")] + &mut computed_text_block_query, + #[cfg(feature = "bevy_text")] + &mut font_system.0, + ); + } + + let _ = world.run_system_once_with( + TestSystemParam { + camera_entity, + root_node_entity, + }, + test_system, + ); + + let ui_surface = world.resource::(); + + let taffy_node = ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + assert!(ui_surface.taffy.layout(*taffy_node).is_ok()); + } } diff --git a/crates/bevy_ui/src/layout/ui_surface.rs b/crates/bevy_ui/src/layout/ui_surface.rs index 4ffd2b467538a..24c9965d14ead 100644 --- a/crates/bevy_ui/src/layout/ui_surface.rs +++ b/crates/bevy_ui/src/layout/ui_surface.rs @@ -33,6 +33,8 @@ pub struct UiSurface { fn _assert_send_sync_ui_surface_impl_safe() { fn _assert_send_sync() {} _assert_send_sync::>(); + _assert_send_sync::>>(); + _assert_send_sync::>>(); _assert_send_sync::>(); _assert_send_sync::(); } @@ -41,7 +43,9 @@ impl fmt::Debug for UiSurface { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_struct("UiSurface") .field("entity_to_taffy", &self.entity_to_taffy) + .field("camera_entity_to_taffy", &self.camera_entity_to_taffy) .field("camera_roots", &self.camera_roots) + .field("taffy_children_scratch", &self.taffy_children_scratch) .finish() } } @@ -312,3 +316,409 @@ fn get_text_buffer<'a>( }; Some(computed.into_inner()) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ContentSize, FixedMeasure}; + use bevy_math::Vec2; + use taffy::TraversePartialTree; + + /// Checks if the parent of the `user_root_node` in a `RootNodePair` + /// is correctly assigned as the `implicit_viewport_node`. + fn is_root_node_pair_valid( + taffy_tree: &TaffyTree, + root_node_pair: &RootNodePair, + ) -> bool { + taffy_tree.parent(root_node_pair.user_root_node) + == Some(root_node_pair.implicit_viewport_node) + } + + /// Tries to get the root node pair for a given root node entity with the specified camera entity + fn get_root_node_pair_exact( + ui_surface: &UiSurface, + root_node_entity: Entity, + camera_entity: Entity, + ) -> Option<&RootNodePair> { + let root_node_pairs = ui_surface.camera_roots.get(&camera_entity)?; + let root_node_taffy = ui_surface.entity_to_taffy.get(&root_node_entity)?; + root_node_pairs + .iter() + .find(|&root_node_pair| root_node_pair.user_root_node == *root_node_taffy) + } + + #[test] + fn test_initialization() { + let ui_surface = UiSurface::default(); + assert!(ui_surface.entity_to_taffy.is_empty()); + assert!(ui_surface.camera_entity_to_taffy.is_empty()); + assert!(ui_surface.camera_roots.is_empty()); + assert_eq!(ui_surface.taffy.total_node_count(), 0); + } + + #[test] + fn test_upsert() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + // standard upsert + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + + // should be inserted into taffy + assert_eq!(ui_surface.taffy.total_node_count(), 1); + assert!(ui_surface.entity_to_taffy.contains_key(&root_node_entity)); + + // test duplicate insert 1 + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + + // node count should not have increased + assert_eq!(ui_surface.taffy.total_node_count(), 1); + + // assign root node to camera + ui_surface.set_camera_children(camera_entity, vec![root_node_entity].into_iter()); + + // each root node will create 2 taffy nodes + assert_eq!(ui_surface.taffy.total_node_count(), 2); + + // root node pair should now exist + let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node pair"); + assert!(is_root_node_pair_valid(&ui_surface.taffy, root_node_pair)); + + // test duplicate insert 2 + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + + // node count should not have increased + assert_eq!(ui_surface.taffy.total_node_count(), 2); + + // root node pair should be unaffected + let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node pair"); + assert!(is_root_node_pair_valid(&ui_surface.taffy, root_node_pair)); + } + + #[test] + fn test_get_root_node_pair_exact() { + /// Attempts to find the camera entity that holds a reference to the given root node entity + fn get_associated_camera_entity( + ui_surface: &UiSurface, + root_node_entity: Entity, + ) -> Option { + for (&camera_entity, root_node_map) in ui_surface.camera_entity_to_taffy.iter() { + if root_node_map.contains_key(&root_node_entity) { + return Some(camera_entity); + } + } + None + } + + /// Attempts to find the root node pair corresponding to the given root node entity + fn get_root_node_pair( + ui_surface: &UiSurface, + root_node_entity: Entity, + ) -> Option<&RootNodePair> { + let camera_entity = get_associated_camera_entity(ui_surface, root_node_entity)?; + get_root_node_pair_exact(ui_surface, root_node_entity, camera_entity) + } + + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + + // assign root node to camera + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + assert_eq!( + get_associated_camera_entity(&ui_surface, root_node_entity), + Some(camera_entity) + ); + assert_eq!( + get_associated_camera_entity(&ui_surface, Entity::from_raw(2)), + None + ); + + let root_node_pair = get_root_node_pair(&ui_surface, root_node_entity); + assert!(root_node_pair.is_some()); + assert_eq!( + Some(root_node_pair.unwrap().user_root_node).as_ref(), + ui_surface.entity_to_taffy.get(&root_node_entity) + ); + + assert_eq!( + get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity), + root_node_pair + ); + } + + #[allow(unreachable_code)] + #[test] + fn test_remove_camera_entities() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + + // assign root node to camera + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + assert!(ui_surface + .camera_entity_to_taffy + .contains_key(&camera_entity)); + assert!(ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity)); + assert!(ui_surface.camera_roots.contains_key(&camera_entity)); + let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node pair"); + assert!(ui_surface + .camera_roots + .get(&camera_entity) + .unwrap() + .contains(root_node_pair)); + + ui_surface.remove_camera_entities([camera_entity]); + + // should not affect `entity_to_taffy` + assert!(ui_surface.entity_to_taffy.contains_key(&root_node_entity)); + + // `camera_roots` and `camera_entity_to_taffy` should no longer contain entries for `camera_entity` + assert!(!ui_surface + .camera_entity_to_taffy + .contains_key(&camera_entity)); + + return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) + + assert!(!ui_surface.camera_roots.contains_key(&camera_entity)); + + // root node pair should be removed + let root_node_pair = get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity); + assert_eq!(root_node_pair, None); + } + + #[allow(unreachable_code)] + #[test] + fn test_remove_entities() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + assert!(ui_surface.entity_to_taffy.contains_key(&root_node_entity)); + assert!(ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity)); + let root_node_pair = + get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity).unwrap(); + assert!(ui_surface + .camera_roots + .get(&camera_entity) + .unwrap() + .contains(root_node_pair)); + + ui_surface.remove_entities([root_node_entity]); + assert!(!ui_surface.entity_to_taffy.contains_key(&root_node_entity)); + + return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) + + assert!(!ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity)); + assert!(!ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity)); + assert!(ui_surface + .camera_roots + .get(&camera_entity) + .unwrap() + .is_empty()); + } + + #[test] + fn test_try_update_measure() { + let mut ui_surface = UiSurface::default(); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + let mut content_size = ContentSize::default(); + content_size.set(NodeMeasure::Fixed(FixedMeasure { size: Vec2::ONE })); + let measure_func = content_size.measure.take().unwrap(); + assert!(ui_surface + .update_node_context(root_node_entity, measure_func) + .is_some()); + } + + #[test] + fn test_update_children() { + let mut ui_surface = UiSurface::default(); + let root_node_entity = Entity::from_raw(1); + let child_entity = Entity::from_raw(2); + let style = Style::default(); + + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, child_entity, &style, None); + + ui_surface.update_children(root_node_entity, vec![child_entity].into_iter()); + + let parent_node = *ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + let child_node = *ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + assert_eq!(ui_surface.taffy.parent(child_node), Some(parent_node)); + } + + #[allow(unreachable_code)] + #[test] + fn test_set_camera_children() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let child_entity = Entity::from_raw(2); + let style = Style::default(); + + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, child_entity, &style, None); + + let root_taffy_node = *ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + let child_taffy = *ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + + // set up the relationship manually + ui_surface + .taffy + .add_child(root_taffy_node, child_taffy) + .unwrap(); + + ui_surface.set_camera_children(camera_entity, [root_node_entity].into_iter()); + + assert!( + ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity), + "root node not associated with camera" + ); + assert!( + !ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&child_entity), + "child of root node should not be associated with camera" + ); + + let _root_node_pair = + get_root_node_pair_exact(&ui_surface, root_node_entity, camera_entity) + .expect("expected root node pair"); + + assert_eq!(ui_surface.taffy.parent(child_taffy), Some(root_taffy_node)); + let root_taffy_children = ui_surface.taffy.children(root_taffy_node).unwrap(); + assert!( + root_taffy_children.contains(&child_taffy), + "root node is not a parent of child node" + ); + assert_eq!( + ui_surface.taffy.child_count(root_taffy_node), + 1, + "expected root node child count to be 1" + ); + + // clear camera's root nodes + ui_surface.set_camera_children(camera_entity, Vec::::new().into_iter()); + + return; // TODO: can't pass the test if we continue - not implemented (remove allow(unreachable_code)) + + assert!( + !ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity), + "root node should have been unassociated with camera" + ); + assert!( + !ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&child_entity), + "child of root node should not be associated with camera" + ); + + let root_taffy_children = ui_surface.taffy.children(root_taffy_node).unwrap(); + assert!( + root_taffy_children.contains(&child_taffy), + "root node is not a parent of child node" + ); + assert_eq!( + ui_surface.taffy.child_count(root_taffy_node), + 1, + "expected root node child count to be 1" + ); + + // re-associate root node with camera + ui_surface.set_camera_children(camera_entity, vec![root_node_entity].into_iter()); + + assert!( + ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&root_node_entity), + "root node should have been re-associated with camera" + ); + assert!( + !ui_surface + .camera_entity_to_taffy + .get(&camera_entity) + .unwrap() + .contains_key(&child_entity), + "child of root node should not be associated with camera" + ); + + let child_taffy = ui_surface.entity_to_taffy.get(&child_entity).unwrap(); + let root_taffy_children = ui_surface.taffy.children(root_taffy_node).unwrap(); + assert!( + root_taffy_children.contains(child_taffy), + "root node is not a parent of child node" + ); + assert_eq!( + ui_surface.taffy.child_count(root_taffy_node), + 1, + "expected root node child count to be 1" + ); + } + + #[test] + #[cfg(not(feature = "bevy_text"))] + fn test_compute_camera_layout() { + let mut ui_surface = UiSurface::default(); + let camera_entity = Entity::from_raw(0); + let root_node_entity = Entity::from_raw(1); + let style = Style::default(); + + ui_surface.upsert_node(&LayoutContext::TEST_CONTEXT, root_node_entity, &style, None); + + ui_surface.compute_camera_layout(camera_entity, UVec2::new(800, 600)); + + let taffy_node = ui_surface.entity_to_taffy.get(&root_node_entity).unwrap(); + assert!(ui_surface.taffy.layout(*taffy_node).is_ok()); + } +} diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs index 007959402ba8b..1cdd48aa5a1cc 100644 --- a/crates/bevy_ui/src/lib.rs +++ b/crates/bevy_ui/src/lib.rs @@ -46,6 +46,7 @@ use widget::UiImageSize; /// /// This includes the most common types in this crate, re-exported for your convenience. pub mod prelude { + #[cfg(feature = "bevy_text")] #[allow(deprecated)] #[doc(hidden)] pub use crate::widget::TextBundle; @@ -178,17 +179,22 @@ impl Plugin for UiPlugin { ui_focus_system.in_set(UiSystem::Focus).after(InputSystem), ); + let ui_layout_system_config = ui_layout_system + .in_set(UiSystem::Layout) + .before(TransformSystem::TransformPropagate); + + #[cfg(feature = "bevy_text")] + let ui_layout_system_config = ui_layout_system_config + // Text and Text2D operate on disjoint sets of entities + .ambiguous_with(bevy_text::update_text2d_layout) + .ambiguous_with(bevy_text::detect_text_needs_rerender::); + app.add_systems( PostUpdate, ( check_visibility::.in_set(VisibilitySystems::CheckVisibility), update_target_camera_system.in_set(UiSystem::Prepare), - ui_layout_system - .in_set(UiSystem::Layout) - .before(TransformSystem::TransformPropagate) - // Text and Text2D operate on disjoint sets of entities - .ambiguous_with(bevy_text::detect_text_needs_rerender::) - .ambiguous_with(bevy_text::update_text2d_layout), + ui_layout_system_config, ui_stack_system .in_set(UiSystem::Stack) // the systems don't care about stack index From af93e78b72c1e435e73c54c9d0ac6260dc170762 Mon Sep 17 00:00:00 2001 From: Lucas Date: Tue, 15 Oct 2024 16:14:44 +0200 Subject: [PATCH 3/8] Fix overflow panic on Stopwatch at Duration::MAX (#15927) See #15924 for more details close #15924 from the issue, this code panic: ```rust use bevy::time::Stopwatch; use std::time::Duration; fn main() { let second = Duration::from_secs(1); let mut stopwatch = Stopwatch::new(); // lot of time has passed... or a timer with Duration::MAX that was artificially set has "finished": // timer.set_elapsed(timer.remaining()); stopwatch.set_elapsed(Duration::MAX); // panic stopwatch.tick(second); let mut stopwatch = Stopwatch::new(); stopwatch.set_elapsed(Duration::MAX - second); // this doesnt panic as its still one off the max stopwatch.tick(second); // this panic stopwatch.tick(second); } ``` with this PR changes, the code now doesn't panic. have a good day ! --- crates/bevy_time/src/stopwatch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_time/src/stopwatch.rs b/crates/bevy_time/src/stopwatch.rs index c2ecfb2f62453..352749848d29c 100644 --- a/crates/bevy_time/src/stopwatch.rs +++ b/crates/bevy_time/src/stopwatch.rs @@ -129,7 +129,7 @@ impl Stopwatch { /// ``` pub fn tick(&mut self, delta: Duration) -> &Self { if !self.paused() { - self.elapsed += delta; + self.elapsed = self.elapsed.saturating_add(delta); } self } From 812e599f77a1e5ddad0fc82f4eb691cfbdd4b2b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= Date: Tue, 15 Oct 2024 17:21:28 +0200 Subject: [PATCH 4/8] don't clip text that is rotated (#15925) # Objective - Fixes #15922 , #15853 - Don't clip text that is rotated by some angles ## Solution - Compare to the absolute size before clipping --- crates/bevy_ui/src/render/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index e3be4a5bdb7dc..c80ba79182080 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -1158,9 +1158,9 @@ pub fn prepare_uinodes( let transformed_rect_size = glyph.transform.transform_vector3(rect_size); if positions_diff[0].x - positions_diff[1].x - >= transformed_rect_size.x + >= transformed_rect_size.x.abs() || positions_diff[1].y - positions_diff[2].y - >= transformed_rect_size.y + >= transformed_rect_size.y.abs() { continue; } From 15440c189b6998238a910c873dee5920e342ef96 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Tue, 15 Oct 2024 11:06:34 -0700 Subject: [PATCH 5/8] Move `SUPPORTED_FILE_EXTENSIONS` to `ImageLoader` and remove unsupported formats. (#15917) # Objective Fixes #15730. ## Solution As part of #15586, we made a constant to store all the supported image formats. However since the `ImageFormat` does actually include Hdr and OpenExr, it also included the `"hdr"` and `"exr"` file extensions. These are supported by separate loaders though: `HdrTextureLoader` and `ExrTextureLoader`. This led to a warning about duplicate asset loaders. Therefore, instead of having the constant for `ImageFormat`, I made the constant just for `ImageLoader`. This lets us correctly remove `"hdr"` and `"exr"` from the image formats supported by `ImageLoader`, returning us to having a single asset loader for every image format. Note: we could have just removed `hdr` and `exr` from `ImageFormat::SUPPORTED_FILE_EXTENSIONS`, but this would be very confusing. Then the list of `ImageFormat`s would not match the list of supported formats! ## Testing - I ran the `sprite` example and got no warning! I also replaced the sprite in that example with an HDR file and everything worked as expected. --- crates/bevy_image/src/image.rs | 141 -------------------------- crates/bevy_image/src/image_loader.rs | 131 +++++++++++++++++++++++- crates/bevy_render/src/texture/mod.rs | 6 +- 3 files changed, 133 insertions(+), 145 deletions(-) diff --git a/crates/bevy_image/src/image.rs b/crates/bevy_image/src/image.rs index 436af1f504174..71c98ed7720a5 100644 --- a/crates/bevy_image/src/image.rs +++ b/crates/bevy_image/src/image.rs @@ -78,147 +78,6 @@ macro_rules! feature_gate { } impl ImageFormat { - /// Number of image formats, used for computing other constants. - const COUNT: usize = { - let mut count = 0; - #[cfg(feature = "avif")] - { - count += 1; - } - #[cfg(feature = "basis-universal")] - { - count += 1; - } - #[cfg(feature = "bmp")] - { - count += 1; - } - #[cfg(feature = "dds")] - { - count += 1; - } - #[cfg(feature = "ff")] - { - count += 1; - } - #[cfg(feature = "gif")] - { - count += 1; - } - #[cfg(feature = "exr")] - { - count += 1; - } - #[cfg(feature = "hdr")] - { - count += 1; - } - #[cfg(feature = "ico")] - { - count += 1; - } - #[cfg(feature = "jpeg")] - { - count += 1; - } - #[cfg(feature = "ktx2")] - { - count += 1; - } - #[cfg(feature = "pnm")] - { - count += 1; - } - #[cfg(feature = "png")] - { - count += 1; - } - #[cfg(feature = "qoi")] - { - count += 1; - } - #[cfg(feature = "tga")] - { - count += 1; - } - #[cfg(feature = "tiff")] - { - count += 1; - } - #[cfg(feature = "webp")] - { - count += 1; - } - count - }; - - /// Full list of supported formats. - pub const SUPPORTED: &'static [ImageFormat] = &[ - #[cfg(feature = "avif")] - ImageFormat::Avif, - #[cfg(feature = "basis-universal")] - ImageFormat::Basis, - #[cfg(feature = "bmp")] - ImageFormat::Bmp, - #[cfg(feature = "dds")] - ImageFormat::Dds, - #[cfg(feature = "ff")] - ImageFormat::Farbfeld, - #[cfg(feature = "gif")] - ImageFormat::Gif, - #[cfg(feature = "exr")] - ImageFormat::OpenExr, - #[cfg(feature = "hdr")] - ImageFormat::Hdr, - #[cfg(feature = "ico")] - ImageFormat::Ico, - #[cfg(feature = "jpeg")] - ImageFormat::Jpeg, - #[cfg(feature = "ktx2")] - ImageFormat::Ktx2, - #[cfg(feature = "png")] - ImageFormat::Png, - #[cfg(feature = "pnm")] - ImageFormat::Pnm, - #[cfg(feature = "qoi")] - ImageFormat::Qoi, - #[cfg(feature = "tga")] - ImageFormat::Tga, - #[cfg(feature = "tiff")] - ImageFormat::Tiff, - #[cfg(feature = "webp")] - ImageFormat::WebP, - ]; - - /// Total count of file extensions, for computing supported file extensions list. - const COUNT_FILE_EXTENSIONS: usize = { - let mut count = 0; - let mut idx = 0; - while idx < ImageFormat::COUNT { - count += ImageFormat::SUPPORTED[idx].to_file_extensions().len(); - idx += 1; - } - count - }; - - /// Gets the list of file extensions for all formats. - pub const SUPPORTED_FILE_EXTENSIONS: &'static [&'static str] = &{ - let mut exts = [""; ImageFormat::COUNT_FILE_EXTENSIONS]; - let mut ext_idx = 0; - let mut fmt_idx = 0; - while fmt_idx < ImageFormat::COUNT { - let mut off = 0; - let fmt_exts = ImageFormat::SUPPORTED[fmt_idx].to_file_extensions(); - while off < fmt_exts.len() { - exts[ext_idx] = fmt_exts[off]; - off += 1; - ext_idx += 1; - } - fmt_idx += 1; - } - exts - }; - /// Gets the file extensions for a given format. pub const fn to_file_extensions(&self) -> &'static [&'static str] { match self { diff --git a/crates/bevy_image/src/image_loader.rs b/crates/bevy_image/src/image_loader.rs index f22c6a729d380..397b0ba7275cf 100644 --- a/crates/bevy_image/src/image_loader.rs +++ b/crates/bevy_image/src/image_loader.rs @@ -12,6 +12,135 @@ pub struct ImageLoader { } impl ImageLoader { + /// Number of image formats, used for computing other constants. + const COUNT: usize = { + let mut count = 0; + #[cfg(feature = "avif")] + { + count += 1; + } + #[cfg(feature = "basis-universal")] + { + count += 1; + } + #[cfg(feature = "bmp")] + { + count += 1; + } + #[cfg(feature = "dds")] + { + count += 1; + } + #[cfg(feature = "ff")] + { + count += 1; + } + #[cfg(feature = "gif")] + { + count += 1; + } + #[cfg(feature = "ico")] + { + count += 1; + } + #[cfg(feature = "jpeg")] + { + count += 1; + } + #[cfg(feature = "ktx2")] + { + count += 1; + } + #[cfg(feature = "pnm")] + { + count += 1; + } + #[cfg(feature = "png")] + { + count += 1; + } + #[cfg(feature = "qoi")] + { + count += 1; + } + #[cfg(feature = "tga")] + { + count += 1; + } + #[cfg(feature = "tiff")] + { + count += 1; + } + #[cfg(feature = "webp")] + { + count += 1; + } + count + }; + + /// Full list of supported formats. + pub const SUPPORTED_FORMATS: &'static [ImageFormat] = &[ + #[cfg(feature = "avif")] + ImageFormat::Avif, + #[cfg(feature = "basis-universal")] + ImageFormat::Basis, + #[cfg(feature = "bmp")] + ImageFormat::Bmp, + #[cfg(feature = "dds")] + ImageFormat::Dds, + #[cfg(feature = "ff")] + ImageFormat::Farbfeld, + #[cfg(feature = "gif")] + ImageFormat::Gif, + #[cfg(feature = "ico")] + ImageFormat::Ico, + #[cfg(feature = "jpeg")] + ImageFormat::Jpeg, + #[cfg(feature = "ktx2")] + ImageFormat::Ktx2, + #[cfg(feature = "png")] + ImageFormat::Png, + #[cfg(feature = "pnm")] + ImageFormat::Pnm, + #[cfg(feature = "qoi")] + ImageFormat::Qoi, + #[cfg(feature = "tga")] + ImageFormat::Tga, + #[cfg(feature = "tiff")] + ImageFormat::Tiff, + #[cfg(feature = "webp")] + ImageFormat::WebP, + ]; + + /// Total count of file extensions, for computing supported file extensions list. + const COUNT_FILE_EXTENSIONS: usize = { + let mut count = 0; + let mut idx = 0; + while idx < Self::COUNT { + count += Self::SUPPORTED_FORMATS[idx].to_file_extensions().len(); + idx += 1; + } + count + }; + + /// Gets the list of file extensions for all formats. + pub const SUPPORTED_FILE_EXTENSIONS: &'static [&'static str] = &{ + let mut exts = [""; Self::COUNT_FILE_EXTENSIONS]; + let mut ext_idx = 0; + let mut fmt_idx = 0; + while fmt_idx < Self::COUNT { + let mut off = 0; + let fmt_exts = Self::SUPPORTED_FORMATS[fmt_idx].to_file_extensions(); + while off < fmt_exts.len() { + exts[ext_idx] = fmt_exts[off]; + off += 1; + ext_idx += 1; + } + fmt_idx += 1; + } + exts + }; + /// Creates a new image loader that supports the provided formats. pub fn new(supported_compressed_formats: CompressedImageFormats) -> Self { Self { @@ -105,7 +234,7 @@ impl AssetLoader for ImageLoader { } fn extensions(&self) -> &[&str] { - ImageFormat::SUPPORTED_FILE_EXTENSIONS + Self::SUPPORTED_FILE_EXTENSIONS } } diff --git a/crates/bevy_render/src/texture/mod.rs b/crates/bevy_render/src/texture/mod.rs index 1fd999a9a1dac..351a356be24a1 100644 --- a/crates/bevy_render/src/texture/mod.rs +++ b/crates/bevy_render/src/texture/mod.rs @@ -111,13 +111,13 @@ impl Plugin for ImagePlugin { ); } - if !ImageFormat::SUPPORTED_FILE_EXTENSIONS.is_empty() { - app.preregister_asset_loader::(ImageFormat::SUPPORTED_FILE_EXTENSIONS); + if !ImageLoader::SUPPORTED_FILE_EXTENSIONS.is_empty() { + app.preregister_asset_loader::(ImageLoader::SUPPORTED_FILE_EXTENSIONS); } } fn finish(&self, app: &mut App) { - if !ImageFormat::SUPPORTED.is_empty() { + if !ImageLoader::SUPPORTED_FORMATS.is_empty() { let supported_compressed_formats = match app.world().get_resource::() { Some(render_device) => { CompressedImageFormats::from_features(render_device.features()) From 424e56318494ade4ed0f0e202e624515958a870f Mon Sep 17 00:00:00 2001 From: Rob Parrett Date: Tue, 15 Oct 2024 11:32:51 -0700 Subject: [PATCH 6/8] Make `contributors` example deterministic in CI (#15901) # Objective - Make the example deterministic when run with CI, so that the [screenshot comparison](https://thebevyflock.github.io/bevy-example-runner/) is stable - Preserve the "truly random on each run" behavior so that every page load in the example showcase shows a different contributor first ## Solution - Fall back to the static default contributor list in CI - Store contributors in a `Vec` so that we can show repeats of the fallback contributor list, giving the appearance of lots of overlapping contributors in CI - Use a shared seeded RNG throughout the app - Give contributor birds a `z` value so that their depth is stable - Remove the shuffle, which was redundant because contributors are first collected into a hashmap - `chain` the systems so that the physics is deterministic from run to run ## Testing ```bash echo '(setup: (fixed_frame_time: Some(0.05)), events: [(100, Screenshot), (500, AppExit)])' > config.ron CI_TESTING_CONFIG=config.ron cargo run --example contributors --features=bevy_ci_testing mv screenshot-100.png screenshot-100-a.png CI_TESTING_CONFIG=config.ron cargo run --example contributors --features=bevy_ci_testing diff screenshot-100.png screenshot-100-a.png ``` ## Alternatives I'd also be fine with removing this example from the list of examples that gets screenshot-tested in CI. Coverage from other 2d examples is probably adequate. --------- Co-authored-by: Alice Cecile --- examples/games/contributors.rs | 77 +++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 25 deletions(-) diff --git a/examples/games/contributors.rs b/examples/games/contributors.rs index 545f8babb7b82..2d653d496a01a 100644 --- a/examples/games/contributors.rs +++ b/examples/games/contributors.rs @@ -1,7 +1,8 @@ //! This example displays each contributor to the bevy source code as a bouncing bevy-ball. use bevy::{math::bounding::Aabb2d, prelude::*, utils::HashMap}; -use rand::{prelude::SliceRandom, Rng}; +use rand::{Rng, SeedableRng}; +use rand_chacha::ChaCha8Rng; use std::{ env::VarError, hash::{DefaultHasher, Hash, Hasher}, @@ -13,13 +14,14 @@ fn main() { App::new() .add_plugins(DefaultPlugins) .init_resource::() + .init_resource::() .add_systems(Startup, (setup_contributor_selection, setup)) - .add_systems(Update, (gravity, movement, collisions, selection)) + // Systems are chained for determinism only + .add_systems(Update, (gravity, movement, collisions, selection).chain()) .run(); } -// Store contributors with their commit count in a collection that preserves the uniqueness -type Contributors = HashMap; +type Contributors = Vec<(String, usize)>; #[derive(Resource)] struct ContributorSelection { @@ -55,26 +57,34 @@ struct Velocity { rotation: f32, } +// We're using a shared seeded RNG here to make this example deterministic for testing purposes. +// This isn't strictly required in practical use unless you need your app to be deterministic. +#[derive(Resource, Deref, DerefMut)] +struct SharedRng(ChaCha8Rng); +impl Default for SharedRng { + fn default() -> Self { + Self(ChaCha8Rng::seed_from_u64(10223163112)) + } +} + const GRAVITY: f32 = 9.821 * 100.0; const SPRITE_SIZE: f32 = 75.0; const SELECTED: Hsla = Hsla::hsl(0.0, 0.9, 0.7); const DESELECTED: Hsla = Hsla::new(0.0, 0.3, 0.2, 0.92); +const SELECTED_Z_OFFSET: f32 = 100.0; + const SHOWCASE_TIMER_SECS: f32 = 3.0; const CONTRIBUTORS_LIST: &[&str] = &["Carter Anderson", "And Many More"]; -fn setup_contributor_selection(mut commands: Commands, asset_server: Res) { - // Load contributors from the git history log or use default values from - // the constant array. Contributors are stored in a HashMap with their - // commit count. - let contribs = contributors().unwrap_or_else(|_| { - CONTRIBUTORS_LIST - .iter() - .map(|name| (name.to_string(), 1)) - .collect() - }); +fn setup_contributor_selection( + mut commands: Commands, + asset_server: Res, + mut rng: ResMut, +) { + let contribs = contributors_or_fallback(); let texture_handle = asset_server.load("branding/icon.png"); @@ -83,11 +93,12 @@ fn setup_contributor_selection(mut commands: Commands, asset_server: Res, mut velocity_query: Query<&mut Velocity>) { fn collisions( window: Single<&Window>, mut query: Query<(&mut Velocity, &mut Transform), With>, + mut rng: ResMut, ) { let window_size = window.size(); @@ -253,8 +263,6 @@ fn collisions( let max_bounce_height = (window_size.y - SPRITE_SIZE * 2.0).max(0.0); let min_bounce_height = max_bounce_height * 0.4; - let mut rng = rand::thread_rng(); - for (mut velocity, mut transform) in &mut query { // Clamp the translation to not go out of the bounds if transform.translation.y < collision_area.min.y { @@ -333,7 +341,26 @@ fn contributors() -> Result { }, ); - Ok(contributors) + Ok(contributors.into_iter().collect()) +} + +/// Get the contributors list, or fall back to a default value if +/// it's unavailable or we're in CI +fn contributors_or_fallback() -> Contributors { + let get_default = || { + CONTRIBUTORS_LIST + .iter() + .cycle() + .take(1000) + .map(|name| (name.to_string(), 1)) + .collect() + }; + + if cfg!(feature = "bevy_ci_testing") { + return get_default(); + } + + contributors().unwrap_or_else(|_| get_default()) } /// Give each unique contributor name a particular hue that is stable between runs. From c1a4b8276293ee8083bf72feda68b5750cc01441 Mon Sep 17 00:00:00 2001 From: Joona Aalto Date: Tue, 15 Oct 2024 22:47:40 +0300 Subject: [PATCH 7/8] Revert default mesh materials (#15930) # Objective Closes #15799. Many rendering people and maintainers are in favor of reverting default mesh materials added in #15524, especially as the migration to required component is already large and heavily breaking. ## Solution Revert default mesh materials, and adjust docs accordingly. - Remove `extract_default_materials` - Remove `clear_material_instances`, and move the logic back into `extract_mesh_materials` - Remove `HasMaterial2d` and `HasMaterial3d` - Change default material handles back to pink instead of white - 2D uses `Color::srgb(1.0, 0.0, 1.0)`, while 3D uses `Color::srgb(1.0, 0.0, 0.5)`. Not sure if this is intended. There is now no indication at all about missing materials for `Mesh2d` and `Mesh3d`. Having a mesh without a material renders nothing. ## Testing I ran `2d_shapes`, `mesh2d_manual`, and `3d_shapes`, with and without mesh material components. --- crates/bevy_pbr/src/lib.rs | 13 +--- crates/bevy_pbr/src/material.rs | 26 +------ crates/bevy_pbr/src/mesh_material.rs | 44 +---------- crates/bevy_render/src/mesh/components.rs | 10 +-- .../bevy_sprite/src/mesh2d/color_material.rs | 21 +---- crates/bevy_sprite/src/mesh2d/material.rs | 76 +------------------ examples/2d/mesh2d_manual.rs | 6 +- 7 files changed, 18 insertions(+), 178 deletions(-) diff --git a/crates/bevy_pbr/src/lib.rs b/crates/bevy_pbr/src/lib.rs index 997a0b99caac3..da63545eed6f0 100644 --- a/crates/bevy_pbr/src/lib.rs +++ b/crates/bevy_pbr/src/lib.rs @@ -422,13 +422,13 @@ impl Plugin for PbrPlugin { app.add_plugins(DeferredPbrLightingPlugin); } - // Initialize the default material. + // Initialize the default material handle. app.world_mut() .resource_mut::>() .insert( &Handle::::default(), StandardMaterial { - base_color: Color::WHITE, + base_color: Color::srgb(1.0, 0.0, 0.5), ..Default::default() }, ); @@ -439,14 +439,7 @@ impl Plugin for PbrPlugin { // Extract the required data from the main world render_app - .add_systems( - ExtractSchedule, - ( - extract_clusters, - extract_lights, - extract_default_materials.after(clear_material_instances::), - ), - ) + .add_systems(ExtractSchedule, (extract_clusters, extract_lights)) .add_systems( Render, ( diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index 2a06aee19c20f..916b5a4676476 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -270,7 +270,6 @@ where fn build(&self, app: &mut App) { app.init_asset::() .register_type::>() - .register_type::() .add_plugins(RenderAssetPlugin::>::default()); if let Some(render_app) = app.get_sub_app_mut(RenderApp) { @@ -283,10 +282,7 @@ where .add_render_command::>() .add_render_command::>() .init_resource::>>() - .add_systems( - ExtractSchedule, - (clear_material_instances::, extract_mesh_materials::).chain(), - ) + .add_systems(ExtractSchedule, extract_mesh_materials::) .add_systems( Render, queue_material_meshes:: @@ -550,16 +546,12 @@ pub const fn screen_space_specular_transmission_pipeline_key( } } -pub(super) fn clear_material_instances( - mut material_instances: ResMut>, -) { - material_instances.clear(); -} - fn extract_mesh_materials( mut material_instances: ResMut>, query: Extract)>>, ) { + material_instances.clear(); + for (entity, view_visibility, material) in &query { if view_visibility.get() { material_instances.insert(entity.into(), material.id()); @@ -567,18 +559,6 @@ fn extract_mesh_materials( } } -/// Extracts default materials for 3D meshes with no [`MeshMaterial3d`]. -pub(super) fn extract_default_materials( - mut material_instances: ResMut>, - query: Extract, Without)>>, -) { - for (entity, view_visibility) in &query { - if view_visibility.get() { - material_instances.insert(entity.into(), AssetId::default()); - } - } -} - /// For each view, iterates over all the meshes visible from that view and adds /// them to [`BinnedRenderPhase`]s or [`SortedRenderPhase`]s as appropriate. #[allow(clippy::too_many_arguments)] diff --git a/crates/bevy_pbr/src/mesh_material.rs b/crates/bevy_pbr/src/mesh_material.rs index c737c991b808b..84eaf7cffa79a 100644 --- a/crates/bevy_pbr/src/mesh_material.rs +++ b/crates/bevy_pbr/src/mesh_material.rs @@ -5,7 +5,7 @@ use bevy_ecs::{component::Component, reflect::ReflectComponent}; use bevy_reflect::{std_traits::ReflectDefault, Reflect}; use derive_more::derive::From; -/// A [material](Material) for a [`Mesh3d`]. +/// A [material](Material) used for rendering a [`Mesh3d`]. /// /// See [`Material`] for general information about 3D materials and how to implement your own materials. /// @@ -36,41 +36,8 @@ use derive_more::derive::From; /// )); /// } /// ``` -/// -/// ## Default Material -/// -/// Meshes without a [`MeshMaterial3d`] are rendered with a default [`StandardMaterial`]. -/// This material can be overridden by inserting a custom material for the default asset handle. -/// -/// ``` -/// # use bevy_pbr::{Material, MeshMaterial3d, StandardMaterial}; -/// # use bevy_ecs::prelude::*; -/// # use bevy_render::mesh::{Mesh, Mesh3d}; -/// # use bevy_color::Color; -/// # use bevy_asset::{Assets, Handle}; -/// # use bevy_math::primitives::Capsule3d; -/// # -/// fn setup( -/// mut commands: Commands, -/// mut meshes: ResMut>, -/// mut materials: ResMut>, -/// ) { -/// // Optional: Insert a custom default material. -/// materials.insert( -/// &Handle::::default(), -/// StandardMaterial::from(Color::srgb(1.0, 0.0, 1.0)), -/// ); -/// -/// // Spawn a capsule with no material. -/// // The mesh will be rendered with the default material. -/// commands.spawn(Mesh3d(meshes.add(Capsule3d::default()))); -/// } -/// ``` -/// -/// [`StandardMaterial`]: crate::StandardMaterial #[derive(Component, Clone, Debug, Deref, DerefMut, Reflect, PartialEq, Eq, From)] #[reflect(Component, Default)] -#[require(HasMaterial3d)] pub struct MeshMaterial3d(pub Handle); impl Default for MeshMaterial3d { @@ -90,12 +57,3 @@ impl From<&MeshMaterial3d> for AssetId { material.id() } } - -/// A component that marks an entity as having a [`MeshMaterial3d`]. -/// [`Mesh3d`] entities without this component are rendered with a [default material]. -/// -/// [`Mesh3d`]: bevy_render::mesh::Mesh3d -/// [default material]: crate::MeshMaterial3d#default-material -#[derive(Component, Clone, Debug, Default, Reflect)] -#[reflect(Component, Default)] -pub struct HasMaterial3d; diff --git a/crates/bevy_render/src/mesh/components.rs b/crates/bevy_render/src/mesh/components.rs index cb3d46afa4e6b..2e712c8054b48 100644 --- a/crates/bevy_render/src/mesh/components.rs +++ b/crates/bevy_render/src/mesh/components.rs @@ -6,13 +6,10 @@ use bevy_reflect::{std_traits::ReflectDefault, Reflect}; use bevy_transform::components::Transform; use derive_more::derive::From; -/// A component for rendering 2D meshes, typically with a [`MeshMaterial2d`] using a [`ColorMaterial`]. -/// -/// Meshes without a [`MeshMaterial2d`] will be rendered with a [default material]. +/// A component for 2D meshes. Requires a [`MeshMaterial2d`] to be rendered, commonly using a [`ColorMaterial`]. /// /// [`MeshMaterial2d`]: /// [`ColorMaterial`]: -/// [default material]: /// /// # Example /// @@ -53,13 +50,10 @@ impl From<&Mesh2d> for AssetId { } } -/// A component for rendering 3D meshes, typically with a [`MeshMaterial3d`] using a [`StandardMaterial`]. -/// -/// Meshes without a [`MeshMaterial3d`] will be rendered with a [default material]. +/// A component for 3D meshes. Requires a [`MeshMaterial3d`] to be rendered, commonly using a [`StandardMaterial`]. /// /// [`MeshMaterial3d`]: /// [`StandardMaterial`]: -/// [default material]: /// /// # Example /// diff --git a/crates/bevy_sprite/src/mesh2d/color_material.rs b/crates/bevy_sprite/src/mesh2d/color_material.rs index f2e5aab14e5ea..3b4ce7641627a 100644 --- a/crates/bevy_sprite/src/mesh2d/color_material.rs +++ b/crates/bevy_sprite/src/mesh2d/color_material.rs @@ -1,20 +1,15 @@ #![expect(deprecated)] -use crate::{ - clear_material_2d_instances, extract_default_materials_2d, AlphaMode2d, Material2d, - Material2dPlugin, MaterialMesh2dBundle, -}; +use crate::{AlphaMode2d, Material2d, Material2dPlugin, MaterialMesh2dBundle}; use bevy_app::{App, Plugin}; use bevy_asset::{load_internal_asset, Asset, AssetApp, Assets, Handle}; use bevy_color::{Alpha, Color, ColorToComponents, LinearRgba}; -use bevy_ecs::schedule::IntoSystemConfigs; use bevy_math::Vec4; use bevy_reflect::prelude::*; use bevy_render::{ render_asset::RenderAssets, render_resource::*, texture::{GpuImage, Image}, - ExtractSchedule, RenderApp, }; pub const COLOR_MATERIAL_SHADER_HANDLE: Handle = @@ -35,26 +30,16 @@ impl Plugin for ColorMaterialPlugin { app.add_plugins(Material2dPlugin::::default()) .register_asset_reflect::(); - // Initialize the default material. + // Initialize the default material handle. app.world_mut() .resource_mut::>() .insert( &Handle::::default(), ColorMaterial { - color: Color::WHITE, + color: Color::srgb(1.0, 0.0, 1.0), ..Default::default() }, ); - - let Some(render_app) = app.get_sub_app_mut(RenderApp) else { - return; - }; - - // Extract default materials for entities with no material. - render_app.add_systems( - ExtractSchedule, - extract_default_materials_2d.after(clear_material_2d_instances::), - ); } } diff --git a/crates/bevy_sprite/src/mesh2d/material.rs b/crates/bevy_sprite/src/mesh2d/material.rs index 3a78622b2bcea..cfef74be398f3 100644 --- a/crates/bevy_sprite/src/mesh2d/material.rs +++ b/crates/bevy_sprite/src/mesh2d/material.rs @@ -43,8 +43,6 @@ use bevy_utils::tracing::error; use core::{hash::Hash, marker::PhantomData}; use derive_more::derive::From; -use super::ColorMaterial; - /// Materials are used alongside [`Material2dPlugin`], [`Mesh2d`], and [`MeshMaterial2d`] /// to spawn entities that are rendered with a specific [`Material2d`] type. They serve as an easy to use high level /// way to render [`Mesh2d`] entities with custom shader logic. @@ -151,7 +149,7 @@ pub trait Material2d: AsBindGroup + Asset + Clone + Sized { } } -/// A [material](Material2d) for a [`Mesh2d`]. +/// A [material](Material2d) used for rendering a [`Mesh2d`]. /// /// See [`Material2d`] for general information about 2D materials and how to implement your own materials. /// @@ -179,40 +177,8 @@ pub trait Material2d: AsBindGroup + Asset + Clone + Sized { /// ``` /// /// [`MeshMaterial2d`]: crate::MeshMaterial2d -/// [`ColorMaterial`]: crate::ColorMaterial -/// -/// ## Default Material -/// -/// Meshes without a [`MeshMaterial2d`] are rendered with a default [`ColorMaterial`]. -/// This material can be overridden by inserting a custom material for the default asset handle. -/// -/// ``` -/// # use bevy_sprite::ColorMaterial; -/// # use bevy_ecs::prelude::*; -/// # use bevy_render::mesh::{Mesh, Mesh2d}; -/// # use bevy_color::Color; -/// # use bevy_asset::{Assets, Handle}; -/// # use bevy_math::primitives::Circle; -/// # -/// fn setup( -/// mut commands: Commands, -/// mut meshes: ResMut>, -/// mut materials: ResMut>, -/// ) { -/// // Optional: Insert a custom default material. -/// materials.insert( -/// &Handle::::default(), -/// ColorMaterial::from(Color::srgb(1.0, 0.0, 1.0)), -/// ); -/// -/// // Spawn a circle with no material. -/// // The mesh will be rendered with the default material. -/// commands.spawn(Mesh2d(meshes.add(Circle::new(50.0)))); -/// } -/// ``` #[derive(Component, Clone, Debug, Deref, DerefMut, Reflect, PartialEq, Eq, From)] #[reflect(Component, Default)] -#[require(HasMaterial2d)] pub struct MeshMaterial2d(pub Handle); impl Default for MeshMaterial2d { @@ -233,14 +199,6 @@ impl From<&MeshMaterial2d> for AssetId { } } -/// A component that marks an entity as having a [`MeshMaterial2d`]. -/// [`Mesh2d`] entities without this component are rendered with a [default material]. -/// -/// [default material]: crate::MeshMaterial2d#default-material -#[derive(Component, Clone, Debug, Default, Reflect)] -#[reflect(Component, Default)] -pub struct HasMaterial2d; - /// Sets how a 2d material's base color alpha channel is used for transparency. /// Currently, this only works with [`Mesh2d`]. Sprites are always transparent. /// @@ -284,7 +242,6 @@ where fn build(&self, app: &mut App) { app.init_asset::() .register_type::>() - .register_type::() .add_plugins(RenderAssetPlugin::>::default()); if let Some(render_app) = app.get_sub_app_mut(RenderApp) { @@ -294,14 +251,7 @@ where .add_render_command::>() .init_resource::>() .init_resource::>>() - .add_systems( - ExtractSchedule, - ( - clear_material_2d_instances::, - extract_mesh_materials_2d::, - ) - .chain(), - ) + .add_systems(ExtractSchedule, extract_mesh_materials_2d::) .add_systems( Render, queue_material2d_meshes:: @@ -327,16 +277,12 @@ impl Default for RenderMaterial2dInstances { } } -pub(crate) fn clear_material_2d_instances( - mut material_instances: ResMut>, -) { - material_instances.clear(); -} - fn extract_mesh_materials_2d( mut material_instances: ResMut>, query: Extract), With>>, ) { + material_instances.clear(); + for (entity, view_visibility, material) in &query { if view_visibility.get() { material_instances.insert(entity.into(), material.id()); @@ -344,20 +290,6 @@ fn extract_mesh_materials_2d( } } -/// Extracts default materials for 2D meshes with no [`MeshMaterial2d`]. -pub(crate) fn extract_default_materials_2d( - mut material_instances: ResMut>, - query: Extract, Without)>>, -) { - let default_material: AssetId = Handle::::default().id(); - - for (entity, view_visibility) in &query { - if view_visibility.get() { - material_instances.insert(entity.into(), default_material); - } - } -} - /// Render pipeline data for a given [`Material2d`] #[derive(Resource)] pub struct Material2dPipeline { diff --git a/examples/2d/mesh2d_manual.rs b/examples/2d/mesh2d_manual.rs index 758084fae35c6..b68470f0eda66 100644 --- a/examples/2d/mesh2d_manual.rs +++ b/examples/2d/mesh2d_manual.rs @@ -30,8 +30,8 @@ use bevy::{ Extract, Render, RenderApp, RenderSet, }, sprite::{ - extract_mesh2d, DrawMesh2d, HasMaterial2d, Material2dBindGroupId, Mesh2dPipeline, - Mesh2dPipelineKey, Mesh2dTransforms, MeshFlags, RenderMesh2dInstance, SetMesh2dBindGroup, + extract_mesh2d, DrawMesh2d, Material2dBindGroupId, Mesh2dPipeline, Mesh2dPipelineKey, + Mesh2dTransforms, MeshFlags, RenderMesh2dInstance, SetMesh2dBindGroup, SetMesh2dViewBindGroup, }, }; @@ -120,10 +120,8 @@ fn star( commands.spawn(Camera2d); } -// Require `HasMaterial2d` to indicate that no placeholder material should be rendeed. /// A marker component for colored 2d meshes #[derive(Component, Default)] -#[require(HasMaterial2d)] pub struct ColoredMesh2d; /// Custom pipeline for 2d meshes with vertex colors From a09104b62ce860daad9ced214470f352471c051c Mon Sep 17 00:00:00 2001 From: Matty Date: Tue, 15 Oct 2024 15:55:36 -0400 Subject: [PATCH 8/8] Infer `StableInterpolate` on tuples (#15931) # Objective Make `StableInterpolate` "just work" on tuples whose parts are each `StableInterpolate` types. These types arise notably through `Curve::zip` (or just through explicit mapping of a similar form). It would otherwise be kind of frustrating to stumble upon such a thing and then realize that, e.g., automatic resampling just doesn't work, even though there is a very "obvious" way to do it. ## Solution Infer `StableInterpolate` on tuples of up to size 11. I can make that number bigger, if desired. Unfortunately, I don't think that our standard "fake variadics" tools actually work for this; the anonymous field accessors of tuples are `:tt` for purposes of macro expansion, which means that you can't simplify away the identifiers by doing something clever like using recursion (which would work if they were `:expr`). Maybe someone who knows some incredibly dark magic could chime in with a better solution. The expanded impls look like this: ```rust impl< T0: StableInterpolate, T1: StableInterpolate, T2: StableInterpolate, T3: StableInterpolate, T4: StableInterpolate, > StableInterpolate for (T0, T1, T2, T3, T4) { fn interpolate_stable(&self, other: &Self, t: f32) -> Self { ( ::interpolate_stable(&self.0, &other.0, t), ::interpolate_stable(&self.1, &other.1, t), ::interpolate_stable(&self.2, &other.2, t), ::interpolate_stable(&self.3, &other.3, t), ::interpolate_stable(&self.4, &other.4, t), ) } } ``` ## Testing Expanded macros; it compiles. ## Future Make a version of the fake variadics workflow that supports this kind of thing. --- crates/bevy_math/src/common_traits.rs | 78 +++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/crates/bevy_math/src/common_traits.rs b/crates/bevy_math/src/common_traits.rs index c015f73113c95..a6aeb04683983 100644 --- a/crates/bevy_math/src/common_traits.rs +++ b/crates/bevy_math/src/common_traits.rs @@ -309,3 +309,81 @@ impl StableInterpolate for Dir3A { self.slerp(*other, t) } } + +macro_rules! impl_stable_interpolate_tuple { + ($(($T:ident, $n:tt)),*) => { + impl<$($T: StableInterpolate),*> StableInterpolate for ($($T,)*) { + fn interpolate_stable(&self, other: &Self, t: f32) -> Self { + ( + $( + <$T as StableInterpolate>::interpolate_stable(&self.$n, &other.$n, t), + )* + ) + } + } + }; +} + +// (See `macro_metavar_expr`, which might make this better.) +// This currently implements `StableInterpolate` for tuples of up to 11 elements. +impl_stable_interpolate_tuple!((T, 0)); +impl_stable_interpolate_tuple!((T0, 0), (T1, 1)); +impl_stable_interpolate_tuple!((T0, 0), (T1, 1), (T2, 2)); +impl_stable_interpolate_tuple!((T0, 0), (T1, 1), (T2, 2), (T3, 3)); +impl_stable_interpolate_tuple!((T0, 0), (T1, 1), (T2, 2), (T3, 3), (T4, 4)); +impl_stable_interpolate_tuple!((T0, 0), (T1, 1), (T2, 2), (T3, 3), (T4, 4), (T5, 5)); +impl_stable_interpolate_tuple!( + (T0, 0), + (T1, 1), + (T2, 2), + (T3, 3), + (T4, 4), + (T5, 5), + (T6, 6) +); +impl_stable_interpolate_tuple!( + (T0, 0), + (T1, 1), + (T2, 2), + (T3, 3), + (T4, 4), + (T5, 5), + (T6, 6), + (T7, 7) +); +impl_stable_interpolate_tuple!( + (T0, 0), + (T1, 1), + (T2, 2), + (T3, 3), + (T4, 4), + (T5, 5), + (T6, 6), + (T7, 7), + (T8, 8) +); +impl_stable_interpolate_tuple!( + (T0, 0), + (T1, 1), + (T2, 2), + (T3, 3), + (T4, 4), + (T5, 5), + (T6, 6), + (T7, 7), + (T8, 8), + (T9, 9) +); +impl_stable_interpolate_tuple!( + (T0, 0), + (T1, 1), + (T2, 2), + (T3, 3), + (T4, 4), + (T5, 5), + (T6, 6), + (T7, 7), + (T8, 8), + (T9, 9), + (T10, 10) +);