Skip to content

Commit

Permalink
Remove the Component trait implementation from Handle (#15796)
Browse files Browse the repository at this point in the history
# Objective

- Closes #15716
- Closes #15718

## Solution

- Replace `Handle<MeshletMesh>` 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<Image>` field. I've removed
this.
- Tests in `bevy_sprite` incorrectly added a `Handle<Image>` 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<Image>` 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<MeshletMesh>` component is now `MeshletMesh3d`.

The `WithMeshletMesh` type alias has been removed. Use
`With<MeshletMesh3d>` instead.
  • Loading branch information
tim-blackbird authored Oct 9, 2024
1 parent a6be9b4 commit 3da0ef0
Show file tree
Hide file tree
Showing 17 changed files with 87 additions and 79 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_animation/src/animation_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions crates/bevy_asset/src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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: Asset> {
/// 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.
Expand Down
8 changes: 2 additions & 6 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,6 @@ mod tests {
let asset_server = app.world().resource::<AssetServer>().clone();
let handle: Handle<CoolText> = asset_server.load(a_path);
let a_id = handle.id();
let entity = app.world_mut().spawn(handle).id();
app.update();
{
let a_text = get::<CoolText>(app.world(), a_id);
Expand Down Expand Up @@ -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::<Assets<CoolText>>().len(),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1345,7 +1344,6 @@ mod tests {
let asset_server = app.world().resource::<AssetServer>().clone();
let handle: Handle<CoolText> = 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| {
Expand Down Expand Up @@ -1746,8 +1744,6 @@ mod tests {
let a_handle: Handle<CoolText> = 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::<ErrorTracker>();
match tracker.finished_asset {
Expand Down
10 changes: 6 additions & 4 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -2270,14 +2270,18 @@ mod test {
}

fn load_gltf_into_app(gltf_path: &str, gltf: &str) -> App {
#[expect(unused)]
#[derive(Resource)]
struct GltfHandle(Handle<Gltf>);

let dir = Dir::default();
dir.insert_asset_text(Path::new(gltf_path), gltf);
let mut app = test_app(dir);
app.update();
let asset_server = app.world().resource::<AssetServer>().clone();
let handle: Handle<Gltf> = 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();
Expand Down Expand Up @@ -2509,7 +2513,6 @@ mod test {
let asset_server = app.world().resource::<AssetServer>().clone();
let handle: Handle<Gltf> = 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();
Expand Down Expand Up @@ -2552,7 +2555,6 @@ mod test {
let asset_server = app.world().resource::<AssetServer>().clone();
let handle: Handle<Gltf> = 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();
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/meshlet/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_pbr/src/meshlet/instance_manager.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -168,7 +168,7 @@ pub fn extract_meshlet_mesh_entities(
SystemState<(
Query<(
Entity,
&Handle<MeshletMesh>,
&MeshletMesh3d,
&GlobalTransform,
Option<&PreviousGlobalTransform>,
Option<&RenderLayers>,
Expand Down
37 changes: 30 additions & 7 deletions crates/bevy_pbr/src/meshlet/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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<Shader> = Handle::weak_from_u128(1325134235233421);
const MESHLET_MESH_MATERIAL_SHADER_HANDLE: Handle<Shader> =
Expand Down Expand Up @@ -206,7 +212,7 @@ impl Plugin for MeshletPlugin {
.register_asset_loader(MeshletMeshLoader)
.add_systems(
PostUpdate,
check_visibility::<WithMeshletMesh>.in_set(VisibilitySystems::CheckVisibility),
check_visibility::<With<MeshletMesh3d>>.in_set(VisibilitySystems::CheckVisibility),
);
}

Expand Down Expand Up @@ -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<MeshletMesh>);

impl From<MeshletMesh3d> for AssetId<MeshletMesh> {
fn from(mesh: MeshletMesh3d) -> Self {
mesh.id()
}
}

impl From<&MeshletMesh3d> for AssetId<MeshletMesh> {
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<M: Material> {
pub meshlet_mesh: Handle<MeshletMesh>,
pub meshlet_mesh: MeshletMesh3d,
pub material: MeshMaterial3d<M>,
pub transform: Transform,
pub global_transform: GlobalTransform,
Expand All @@ -313,10 +340,6 @@ impl<M: Material> Default for MaterialMeshletMeshBundle<M> {
}
}

/// A convenient alias for `With<Handle<MeshletMesh>>`, for use with
/// [`bevy_render::view::VisibleEntities`].
pub type WithMeshletMesh = With<Handle<MeshletMesh>>;

fn configure_meshlet_views(
mut views_3d: Query<(
Entity,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/prepass/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;

Expand Down Expand Up @@ -221,7 +221,7 @@ pub struct PreviousGlobalTransform(pub Affine3A);
#[cfg(not(feature = "meshlet"))]
type PreviousMeshFilter = With<Mesh3d>;
#[cfg(feature = "meshlet")]
type PreviousMeshFilter = Or<(With<Mesh3d>, With<Handle<MeshletMesh>>)>;
type PreviousMeshFilter = Or<(With<Mesh3d>, With<MeshletMesh3d>)>;

pub fn update_mesh_previous_global_transforms(
mut commands: Commands,
Expand Down
4 changes: 0 additions & 4 deletions crates/bevy_sprite/src/bundle.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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<Image>,
/// User indication of whether an entity is visible
pub visibility: Visibility,
/// Inherited visibility of an entity.
Expand Down
26 changes: 11 additions & 15 deletions crates/bevy_sprite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions examples/3d/clearcoat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
Expand Down
24 changes: 11 additions & 13 deletions examples/3d/meshlet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod camera_controller;

use bevy::{
pbr::{
experimental::meshlet::{MaterialMeshletMeshBundle, MeshletPlugin},
experimental::meshlet::{MeshletMesh3d, MeshletPlugin},
CascadeShadowConfigBuilder, DirectionalLightShadowMap,
},
prelude::*,
Expand Down Expand Up @@ -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(),
Expand All @@ -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((
Expand Down
2 changes: 1 addition & 1 deletion examples/animation/eased_motion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn setup(
Transform::from_translation(vec3(-6., 2., 0.)),
animation_target_name,
animation_player,
animation_graph,
AnimationGraphHandle(animation_graph),
))
.id();

Expand Down
Loading

0 comments on commit 3da0ef0

Please sign in to comment.