From 3da0ef048ecaddfad2b8441918a5a8842098b5a9 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 9 Oct 2024 21:10:01 +0000 Subject: [PATCH] Remove the `Component` trait implementation from `Handle` (#15796) # Objective - Closes #15716 - Closes #15718 ## Solution - Replace `Handle` with a new `MeshletMesh3d` component - As expected there were some random things that needed fixing: - A couple tests were storing handles just to prevent them from being dropped I believe, which seems to have been unnecessary in some. - The `SpriteBundle` still had a `Handle` field. I've removed this. - Tests in `bevy_sprite` incorrectly added a `Handle` field outside of the `Sprite` component. - A few examples were still inserting `Handle`s, switched those to their corresponding wrappers. - 2 examples that were still querying for `Handle` were changed to query `Sprite` ## Testing - I've verified that the changed example work now ## Migration Guide `Handle` can no longer be used as a `Component`. All existing Bevy types using this pattern have been wrapped in their own semantically meaningful type. You should do the same for any custom `Handle` components your project needs. The `Handle` component is now `MeshletMesh3d`. The `WithMeshletMesh` type alias has been removed. Use `With` instead. --- crates/bevy_animation/src/animation_event.rs | 2 +- crates/bevy_asset/src/handle.rs | 5 +-- crates/bevy_asset/src/lib.rs | 8 +--- crates/bevy_gltf/src/loader.rs | 10 +++-- crates/bevy_pbr/src/meshlet/asset.rs | 2 +- .../bevy_pbr/src/meshlet/instance_manager.rs | 6 +-- crates/bevy_pbr/src/meshlet/mod.rs | 37 +++++++++++++++---- crates/bevy_pbr/src/prepass/mod.rs | 4 +- crates/bevy_sprite/src/bundle.rs | 4 -- crates/bevy_sprite/src/lib.rs | 26 ++++++------- examples/3d/clearcoat.rs | 4 +- examples/3d/meshlet.rs | 24 ++++++------ examples/animation/eased_motion.rs | 2 +- examples/asset/alter_sprite.rs | 18 ++++----- .../shader/compute_shader_game_of_life.rs | 10 ++--- examples/shader/custom_shader_instancing.rs | 2 +- examples/shader/specialized_mesh_pipeline.rs | 2 +- 17 files changed, 87 insertions(+), 79 deletions(-) diff --git a/crates/bevy_animation/src/animation_event.rs b/crates/bevy_animation/src/animation_event.rs index 825ad8e362f49..0339122ee57b1 100644 --- a/crates/bevy_animation/src/animation_event.rs +++ b/crates/bevy_animation/src/animation_event.rs @@ -58,7 +58,7 @@ pub(crate) fn trigger_animation_event( /// let mut player = AnimationPlayer::default(); /// player.play(animation_index).repeat(); /// -/// commands.spawn((graphs.add(graph), player)); +/// commands.spawn((AnimationGraphHandle(graphs.add(graph)), player)); /// } /// # /// # bevy_ecs::system::assert_is_system(setup_animation); diff --git a/crates/bevy_asset/src/handle.rs b/crates/bevy_asset/src/handle.rs index 4ecc24c29431b..6e4dec39ce25a 100644 --- a/crates/bevy_asset/src/handle.rs +++ b/crates/bevy_asset/src/handle.rs @@ -3,7 +3,6 @@ use crate::{ UntypedAssetId, }; use alloc::sync::Arc; -use bevy_ecs::prelude::*; use bevy_reflect::{std_traits::ReflectDefault, Reflect, TypePath}; use core::{ any::TypeId, @@ -122,8 +121,8 @@ impl core::fmt::Debug for StrongHandle { /// of the [`Handle`] are dropped. /// /// [`Handle::Strong`] also provides access to useful [`Asset`] metadata, such as the [`AssetPath`] (if it exists). -#[derive(Component, Reflect)] -#[reflect(Default, Component, Debug, Hash, PartialEq)] +#[derive(Reflect)] +#[reflect(Default, Debug, Hash, PartialEq)] pub enum Handle { /// A "strong" reference to a live (or loading) [`Asset`]. If a [`Handle`] is [`Handle::Strong`], the [`Asset`] will be kept /// alive until the [`Handle`] is dropped. Strong handles also provide access to additional asset metadata. diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 5ffbf61549ffd..9e92b1507d8fe 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -900,7 +900,6 @@ mod tests { let asset_server = app.world().resource::().clone(); let handle: Handle = asset_server.load(a_path); let a_id = handle.id(); - let entity = app.world_mut().spawn(handle).id(); app.update(); { let a_text = get::(app.world(), a_id); @@ -1090,7 +1089,8 @@ mod tests { a.text = "Changed".to_string(); } - app.world_mut().despawn(entity); + drop(handle); + app.update(); assert_eq!( app.world().resource::>().len(), @@ -1225,7 +1225,6 @@ mod tests { ); } - app.world_mut().spawn(handle); gate_opener.open(a_path); gate_opener.open(b_path); gate_opener.open(c_path); @@ -1345,7 +1344,6 @@ mod tests { let asset_server = app.world().resource::().clone(); let handle: Handle = asset_server.load(a_path); let a_id = handle.id(); - app.world_mut().spawn(handle); gate_opener.open(a_path); run_app_until(&mut app, |world| { @@ -1746,8 +1744,6 @@ mod tests { let a_handle: Handle = asset_server.load(a_path); let a_id = a_handle.id(); - app.world_mut().spawn(a_handle); - run_app_until(&mut app, |world| { let tracker = world.resource::(); match tracker.finished_asset { diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index f63f2fddba674..6d224a11c651d 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -2229,7 +2229,7 @@ mod test { AssetApp, AssetPlugin, AssetServer, Assets, Handle, LoadState, }; use bevy_core::TaskPoolPlugin; - use bevy_ecs::world::World; + use bevy_ecs::{system::Resource, world::World}; use bevy_log::LogPlugin; use bevy_render::mesh::{skinning::SkinnedMeshInverseBindposes, MeshPlugin}; use bevy_scene::ScenePlugin; @@ -2270,6 +2270,10 @@ mod test { } fn load_gltf_into_app(gltf_path: &str, gltf: &str) -> App { + #[expect(unused)] + #[derive(Resource)] + struct GltfHandle(Handle); + let dir = Dir::default(); dir.insert_asset_text(Path::new(gltf_path), gltf); let mut app = test_app(dir); @@ -2277,7 +2281,7 @@ mod test { let asset_server = app.world().resource::().clone(); let handle: Handle = asset_server.load(gltf_path.to_string()); let handle_id = handle.id(); - app.world_mut().spawn(handle.clone()); + app.insert_resource(GltfHandle(handle)); app.update(); run_app_until(&mut app, |_world| { let load_state = asset_server.get_load_state(handle_id).unwrap(); @@ -2509,7 +2513,6 @@ mod test { let asset_server = app.world().resource::().clone(); let handle: Handle = asset_server.load(gltf_path); let handle_id = handle.id(); - app.world_mut().spawn(handle.clone()); app.update(); run_app_until(&mut app, |_world| { let load_state = asset_server.get_load_state(handle_id).unwrap(); @@ -2552,7 +2555,6 @@ mod test { let asset_server = app.world().resource::().clone(); let handle: Handle = asset_server.load(gltf_path); let handle_id = handle.id(); - app.world_mut().spawn(handle.clone()); app.update(); run_app_until(&mut app, |_world| { let load_state = asset_server.get_load_state(handle_id).unwrap(); diff --git a/crates/bevy_pbr/src/meshlet/asset.rs b/crates/bevy_pbr/src/meshlet/asset.rs index 433f421a527cb..4e3edb32a959d 100644 --- a/crates/bevy_pbr/src/meshlet/asset.rs +++ b/crates/bevy_pbr/src/meshlet/asset.rs @@ -36,7 +36,7 @@ pub const MESHLET_MESH_ASSET_VERSION: u64 = 1; /// * Materials must use the [`crate::Material::meshlet_mesh_fragment_shader`] method (and similar variants for prepass/deferred shaders) /// which requires certain shader patterns that differ from the regular material shaders. /// -/// See also [`super::MaterialMeshletMeshBundle`] and [`super::MeshletPlugin`]. +/// See also [`super::MeshletMesh3d`] and [`super::MeshletPlugin`]. #[derive(Asset, TypePath, Clone)] pub struct MeshletMesh { /// Quantized and bitstream-packed vertex positions for meshlet vertices. diff --git a/crates/bevy_pbr/src/meshlet/instance_manager.rs b/crates/bevy_pbr/src/meshlet/instance_manager.rs index 4c609848a600e..768864fe6b610 100644 --- a/crates/bevy_pbr/src/meshlet/instance_manager.rs +++ b/crates/bevy_pbr/src/meshlet/instance_manager.rs @@ -1,9 +1,9 @@ -use super::{meshlet_mesh_manager::MeshletMeshManager, MeshletMesh}; +use super::{meshlet_mesh_manager::MeshletMeshManager, MeshletMesh, MeshletMesh3d}; use crate::{ Material, MeshFlags, MeshTransforms, MeshUniform, NotShadowCaster, NotShadowReceiver, PreviousGlobalTransform, RenderMaterialInstances, }; -use bevy_asset::{AssetEvent, AssetServer, Assets, Handle, UntypedAssetId}; +use bevy_asset::{AssetEvent, AssetServer, Assets, UntypedAssetId}; use bevy_ecs::{ entity::{Entities, Entity, EntityHashMap}, event::EventReader, @@ -168,7 +168,7 @@ pub fn extract_meshlet_mesh_entities( SystemState<( Query<( Entity, - &Handle, + &MeshletMesh3d, &GlobalTransform, Option<&PreviousGlobalTransform>, Option<&RenderLayers>, diff --git a/crates/bevy_pbr/src/meshlet/mod.rs b/crates/bevy_pbr/src/meshlet/mod.rs index 17793af5d637a..80d5f7aa59bfc 100644 --- a/crates/bevy_pbr/src/meshlet/mod.rs +++ b/crates/bevy_pbr/src/meshlet/mod.rs @@ -1,3 +1,4 @@ +#![expect(deprecated)] //! Render high-poly 3d meshes using an efficient GPU-driven method. See [`MeshletPlugin`] and [`MeshletMesh`] for details. mod asset; @@ -57,19 +58,23 @@ use self::{ }; use crate::{graph::NodePbr, Material, MeshMaterial3d}; use bevy_app::{App, Plugin, PostUpdate}; -use bevy_asset::{load_internal_asset, AssetApp, Handle}; +use bevy_asset::{load_internal_asset, AssetApp, AssetId, Handle}; use bevy_core_pipeline::{ core_3d::graph::{Core3d, Node3d}, prepass::{DeferredPrepass, MotionVectorPrepass, NormalPrepass}, }; +use bevy_derive::{Deref, DerefMut}; use bevy_ecs::{ bundle::Bundle, + component::Component, entity::Entity, prelude::With, query::Has, + reflect::ReflectComponent, schedule::IntoSystemConfigs, system::{Commands, Query}, }; +use bevy_reflect::{std_traits::ReflectDefault, Reflect}; use bevy_render::{ render_graph::{RenderGraphApp, ViewNodeRunner}, render_resource::Shader, @@ -83,6 +88,7 @@ use bevy_render::{ }; use bevy_transform::components::{GlobalTransform, Transform}; use bevy_utils::tracing::error; +use derive_more::From; const MESHLET_BINDINGS_SHADER_HANDLE: Handle = Handle::weak_from_u128(1325134235233421); const MESHLET_MESH_MATERIAL_SHADER_HANDLE: Handle = @@ -206,7 +212,7 @@ impl Plugin for MeshletPlugin { .register_asset_loader(MeshletMeshLoader) .add_systems( PostUpdate, - check_visibility::.in_set(VisibilitySystems::CheckVisibility), + check_visibility::>.in_set(VisibilitySystems::CheckVisibility), ); } @@ -284,10 +290,31 @@ impl Plugin for MeshletPlugin { } } +#[derive(Component, Clone, Debug, Default, Deref, DerefMut, Reflect, PartialEq, Eq, From)] +#[reflect(Component, Default)] +#[require(Transform, Visibility)] +pub struct MeshletMesh3d(pub Handle); + +impl From for AssetId { + fn from(mesh: MeshletMesh3d) -> Self { + mesh.id() + } +} + +impl From<&MeshletMesh3d> for AssetId { + fn from(mesh: &MeshletMesh3d) -> Self { + mesh.id() + } +} + /// A component bundle for entities with a [`MeshletMesh`] and a [`Material`]. #[derive(Bundle, Clone)] +#[deprecated( + since = "0.15.0", + note = "Use the `MeshletMesh3d` and `MeshMaterial3d` components instead. Inserting them will now also insert the other components required by them automatically." +)] pub struct MaterialMeshletMeshBundle { - pub meshlet_mesh: Handle, + pub meshlet_mesh: MeshletMesh3d, pub material: MeshMaterial3d, pub transform: Transform, pub global_transform: GlobalTransform, @@ -313,10 +340,6 @@ impl Default for MaterialMeshletMeshBundle { } } -/// A convenient alias for `With>`, for use with -/// [`bevy_render::view::VisibleEntities`]. -pub type WithMeshletMesh = With>; - fn configure_meshlet_views( mut views_3d: Query<( Entity, diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index f3f6fcbb04d8e..4ac34878df6ce 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -35,7 +35,7 @@ use bevy_utils::tracing::error; #[cfg(feature = "meshlet")] use crate::meshlet::{ prepare_material_meshlet_meshes_prepass, queue_material_meshlet_meshes, InstanceManager, - MeshletMesh, + MeshletMesh3d, }; use crate::*; @@ -221,7 +221,7 @@ pub struct PreviousGlobalTransform(pub Affine3A); #[cfg(not(feature = "meshlet"))] type PreviousMeshFilter = With; #[cfg(feature = "meshlet")] -type PreviousMeshFilter = Or<(With, With>)>; +type PreviousMeshFilter = Or<(With, With)>; pub fn update_mesh_previous_global_transforms( mut commands: Commands, diff --git a/crates/bevy_sprite/src/bundle.rs b/crates/bevy_sprite/src/bundle.rs index fbbdf2efe372c..f05bc7e79885a 100644 --- a/crates/bevy_sprite/src/bundle.rs +++ b/crates/bevy_sprite/src/bundle.rs @@ -1,10 +1,8 @@ #![expect(deprecated)] use crate::Sprite; -use bevy_asset::Handle; use bevy_ecs::bundle::Bundle; use bevy_render::{ sync_world::SyncToRenderWorld, - texture::Image, view::{InheritedVisibility, ViewVisibility, Visibility}, }; use bevy_transform::components::{GlobalTransform, Transform}; @@ -28,8 +26,6 @@ pub struct SpriteBundle { pub transform: Transform, /// The absolute transform of the sprite. This should generally not be written to directly. pub global_transform: GlobalTransform, - /// A reference-counted handle to the image asset to be drawn. - pub texture: Handle, /// User indication of whether an entity is visible pub visibility: Visibility, /// Inherited visibility of an entity. diff --git a/crates/bevy_sprite/src/lib.rs b/crates/bevy_sprite/src/lib.rs index 62634f859910a..659344ec9b8e1 100644 --- a/crates/bevy_sprite/src/lib.rs +++ b/crates/bevy_sprite/src/lib.rs @@ -299,13 +299,11 @@ mod test { // Add entities let entity = app .world_mut() - .spawn(( - Sprite { - custom_size: Some(Vec2::ZERO), - ..default() - }, - image_handle, - )) + .spawn(Sprite { + custom_size: Some(Vec2::ZERO), + image: image_handle, + ..default() + }) .id(); // Create initial AABB @@ -364,14 +362,12 @@ mod test { // Add entities let entity = app .world_mut() - .spawn(( - Sprite { - rect: Some(Rect::new(0., 0., 0.5, 1.)), - anchor: Anchor::TopRight, - ..default() - }, - image_handle, - )) + .spawn(Sprite { + rect: Some(Rect::new(0., 0., 0.5, 1.)), + anchor: Anchor::TopRight, + image: image_handle, + ..default() + }) .id(); // Create AABB diff --git a/examples/3d/clearcoat.rs b/examples/3d/clearcoat.rs index 5685d4fb5a4cb..067e7e2e717fb 100644 --- a/examples/3d/clearcoat.rs +++ b/examples/3d/clearcoat.rs @@ -98,7 +98,7 @@ fn spawn_car_paint_sphere( commands .spawn(( Mesh3d(sphere.clone()), - materials.add(StandardMaterial { + MeshMaterial3d(materials.add(StandardMaterial { clearcoat: 1.0, clearcoat_perceptual_roughness: 0.1, normal_map_texture: Some(asset_server.load_with_settings( @@ -109,7 +109,7 @@ fn spawn_car_paint_sphere( perceptual_roughness: 0.5, base_color: BLUE.into(), ..default() - }), + })), Transform::from_xyz(-1.0, 1.0, 0.0).with_scale(Vec3::splat(SPHERE_SCALE)), )) .insert(ExampleSphere); diff --git a/examples/3d/meshlet.rs b/examples/3d/meshlet.rs index 5a9e04019428a..72da8c968b4a5 100644 --- a/examples/3d/meshlet.rs +++ b/examples/3d/meshlet.rs @@ -7,7 +7,7 @@ mod camera_controller; use bevy::{ pbr::{ - experimental::meshlet::{MaterialMeshletMeshBundle, MeshletPlugin}, + experimental::meshlet::{MeshletMesh3d, MeshletPlugin}, CascadeShadowConfigBuilder, DirectionalLightShadowMap, }, prelude::*, @@ -84,9 +84,9 @@ fn setup( let debug_material = debug_materials.add(MeshletDebugMaterial::default()); for x in -2..=2 { - commands.spawn(MaterialMeshletMeshBundle { - meshlet_mesh: meshlet_mesh_handle.clone(), - material: MeshMaterial3d(standard_materials.add(StandardMaterial { + commands.spawn(( + MeshletMesh3d(meshlet_mesh_handle.clone()), + MeshMaterial3d(standard_materials.add(StandardMaterial { base_color: match x { -2 => Srgba::hex("#dc2626").unwrap().into(), -1 => Srgba::hex("#ea580c").unwrap().into(), @@ -98,22 +98,20 @@ fn setup( perceptual_roughness: (x + 2) as f32 / 4.0, ..default() })), - transform: Transform::default() + Transform::default() .with_scale(Vec3::splat(0.2)) .with_translation(Vec3::new(x as f32 / 2.0, 0.0, -0.3)), - ..default() - }); + )); } for x in -2..=2 { - commands.spawn(MaterialMeshletMeshBundle { - meshlet_mesh: meshlet_mesh_handle.clone(), - material: debug_material.clone().into(), - transform: Transform::default() + commands.spawn(( + MeshletMesh3d(meshlet_mesh_handle.clone()), + MeshMaterial3d(debug_material.clone()), + Transform::default() .with_scale(Vec3::splat(0.2)) .with_rotation(Quat::from_rotation_y(PI)) .with_translation(Vec3::new(x as f32 / 2.0, 0.0, 0.3)), - ..default() - }); + )); } commands.spawn(( diff --git a/examples/animation/eased_motion.rs b/examples/animation/eased_motion.rs index 38b1d2afc60d9..cbe4c5d993851 100644 --- a/examples/animation/eased_motion.rs +++ b/examples/animation/eased_motion.rs @@ -43,7 +43,7 @@ fn setup( Transform::from_translation(vec3(-6., 2., 0.)), animation_target_name, animation_player, - animation_graph, + AnimationGraphHandle(animation_graph), )) .id(); diff --git a/examples/asset/alter_sprite.rs b/examples/asset/alter_sprite.rs index 786aab857f2de..5dc0c26802e35 100644 --- a/examples/asset/alter_sprite.rs +++ b/examples/asset/alter_sprite.rs @@ -105,22 +105,20 @@ fn spawn_text(mut commands: Commands) { }, )) .with_children(|parent| { + parent.spawn(Text::new("Space: swap the right sprite's image handle")); parent.spawn(Text::new( - "Space: swap image texture paths by mutating a Handle", - )); - parent.spawn(Text::new( - "Return: mutate the image Asset itself, changing all copies of it", + "Return: modify the image Asset of the left sprite, affecting all uses of it", )); }); } fn alter_handle( asset_server: Res, - mut right_bird: Query<(&mut Bird, &mut Handle), Without>, + mut right_bird: Query<(&mut Bird, &mut Sprite), Without>, ) { // Image handles, like other parts of the ECS, can be queried as mutable and modified at // runtime. We only spawned one bird without the `Left` marker component. - let Ok((mut bird, mut handle)) = right_bird.get_single_mut() else { + let Ok((mut bird, mut sprite)) = right_bird.get_single_mut() else { return; }; @@ -130,18 +128,18 @@ fn alter_handle( // Modify the handle associated with the Bird on the right side. Note that we will only // have to load the same path from storage media once: repeated attempts will re-use the // asset. - *handle = asset_server.load(bird.get_texture_path()); + sprite.image = asset_server.load(bird.get_texture_path()); } -fn alter_asset(mut images: ResMut>, left_bird: Query<&Handle, With>) { +fn alter_asset(mut images: ResMut>, left_bird: Query<&Sprite, With>) { // It's convenient to retrieve the asset handle stored with the bird on the left. However, // we could just as easily have retained this in a resource or a dedicated component. - let Ok(handle) = left_bird.get_single() else { + let Ok(sprite) = left_bird.get_single() else { return; }; // Obtain a mutable reference to the Image asset. - let Some(image) = images.get_mut(handle) else { + let Some(image) = images.get_mut(&sprite.image) else { return; }; diff --git a/examples/shader/compute_shader_game_of_life.rs b/examples/shader/compute_shader_game_of_life.rs index e27c2c88cea57..70ce948515dc8 100644 --- a/examples/shader/compute_shader_game_of_life.rs +++ b/examples/shader/compute_shader_game_of_life.rs @@ -84,12 +84,12 @@ fn setup(mut commands: Commands, mut images: ResMut>) { } // Switch texture to display every frame to show the one that was written to most recently. -fn switch_textures(images: Res, mut displayed: Query<&mut Handle>) { - let mut displayed = displayed.single_mut(); - if *displayed == images.texture_a { - *displayed = images.texture_b.clone_weak(); +fn switch_textures(images: Res, mut displayed: Query<&mut Sprite>) { + let mut sprite = displayed.single_mut(); + if sprite.image == images.texture_a { + sprite.image = images.texture_b.clone_weak(); } else { - *displayed = images.texture_a.clone_weak(); + sprite.image = images.texture_a.clone_weak(); } } diff --git a/examples/shader/custom_shader_instancing.rs b/examples/shader/custom_shader_instancing.rs index 1f9852d320b01..839d0ea2a13d7 100644 --- a/examples/shader/custom_shader_instancing.rs +++ b/examples/shader/custom_shader_instancing.rs @@ -47,7 +47,7 @@ fn main() { fn setup(mut commands: Commands, mut meshes: ResMut>) { commands.spawn(( - meshes.add(Cuboid::new(0.5, 0.5, 0.5)), + Mesh3d(meshes.add(Cuboid::new(0.5, 0.5, 0.5))), SpatialBundle::INHERITED_IDENTITY, InstanceMaterialData( (1..=10) diff --git a/examples/shader/specialized_mesh_pipeline.rs b/examples/shader/specialized_mesh_pipeline.rs index cab2c6e18c66e..cc8cae101d853 100644 --- a/examples/shader/specialized_mesh_pipeline.rs +++ b/examples/shader/specialized_mesh_pipeline.rs @@ -79,7 +79,7 @@ fn setup(mut commands: Commands, mut meshes: ResMut>) { // with our specialized pipeline CustomRenderedEntity, // We need to add the mesh handle to the entity - meshes.add(mesh.clone()), + Mesh3d(meshes.add(mesh.clone())), // This bundle's components are needed for something to be rendered SpatialBundle { transform: Transform::from_xyz(x, y, 0.0),