From e6330d28bb7b680fd9e9e43f72b65d682f21e415 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Mon, 7 Oct 2024 21:58:50 -0700 Subject: [PATCH 01/20] Remove all the `get_mut` functions from `Assets`. Removing these functions makes it simpler to redesign them for when assets are Arc'd. --- crates/bevy_asset/src/assets.rs | 43 --------------------------------- 1 file changed, 43 deletions(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 6e3bf1b5cf51f..7e93fb6d69ddc 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -216,20 +216,6 @@ impl DenseAssetStorage { } } - pub(crate) fn get_mut(&mut self, index: AssetIndex) -> Option<&mut A> { - let entry = self.storage.get_mut(index.index as usize)?; - match entry { - Entry::None => None, - Entry::Some { value, generation } => { - if *generation == index.generation { - value.as_mut() - } else { - None - } - } - } - } - pub(crate) fn flush(&mut self) { // NOTE: this assumes the allocator index is monotonically increasing. let new_len = self @@ -332,20 +318,6 @@ impl Assets { } } - /// Retrieves an [`Asset`] stored for the given `id` if it exists. If it does not exist, it will be inserted using `insert_fn`. - // PERF: Optimize this or remove it - pub fn get_or_insert_with( - &mut self, - id: impl Into>, - insert_fn: impl FnOnce() -> A, - ) -> &mut A { - let id: AssetId = id.into(); - if self.get(id).is_none() { - self.insert(id, insert_fn()); - } - self.get_mut(id).unwrap() - } - /// Returns `true` if the `id` exists in this collection. Otherwise it returns `false`. pub fn contains(&self, id: impl Into>) -> bool { match id.into() { @@ -421,21 +393,6 @@ impl Assets { } } - /// Retrieves a mutable reference to the [`Asset`] with the given `id`, if it exists. - /// Note that this supports anything that implements `Into>`, which includes [`Handle`] and [`AssetId`]. - #[inline] - pub fn get_mut(&mut self, id: impl Into>) -> Option<&mut A> { - let id: AssetId = id.into(); - let result = match id { - AssetId::Index { index, .. } => self.dense_storage.get_mut(index), - AssetId::Uuid { uuid } => self.hash_map.get_mut(&uuid), - }; - if result.is_some() { - self.queued_events.push(AssetEvent::Modified { id }); - } - result - } - /// Removes (and returns) the [`Asset`] with the given `id`, if it exists. /// Note that this supports anything that implements `Into>`, which includes [`Handle`] and [`AssetId`]. pub fn remove(&mut self, id: impl Into>) -> Option { From 6fe8cc1cddf16295c69f9d14afef7605cafa3bcb Mon Sep 17 00:00:00 2001 From: andriyDev Date: Mon, 7 Oct 2024 22:01:50 -0700 Subject: [PATCH 02/20] Make `Assets` store assets as `Arc` instead of just `A`. --- crates/bevy_asset/src/assets.rs | 57 +++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 7e93fb6d69ddc..8c1cf3e6a4092 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -108,7 +108,10 @@ enum Entry { #[default] None, /// Some is an indicator that there is a live handle active for the entry at this [`AssetIndex`] - Some { value: Option, generation: u32 }, + Some { + value: Option>, + generation: u32, + }, } /// Stores [`Asset`] values in a Vec-like storage identified by [`AssetIndex`]. @@ -143,7 +146,7 @@ impl DenseAssetStorage { pub(crate) fn insert( &mut self, index: AssetIndex, - asset: A, + asset: Arc, ) -> Result { self.flush(); let entry = &mut self.storage[index.index as usize]; @@ -168,7 +171,7 @@ impl DenseAssetStorage { /// Removes the asset stored at the given `index` and returns it as [`Some`] (if the asset exists). /// This will recycle the id and allow new entries to be inserted. - pub(crate) fn remove_dropped(&mut self, index: AssetIndex) -> Option { + pub(crate) fn remove_dropped(&mut self, index: AssetIndex) -> Option> { self.remove_internal(index, |dense_storage| { dense_storage.storage[index.index as usize] = Entry::None; dense_storage.allocator.recycle(index); @@ -178,7 +181,7 @@ impl DenseAssetStorage { /// Removes the asset stored at the given `index` and returns it as [`Some`] (if the asset exists). /// This will _not_ recycle the id. New values with the current ID can still be inserted. The ID will /// not be reused until [`DenseAssetStorage::remove_dropped`] is called. - pub(crate) fn remove_still_alive(&mut self, index: AssetIndex) -> Option { + pub(crate) fn remove_still_alive(&mut self, index: AssetIndex) -> Option> { self.remove_internal(index, |_| {}) } @@ -186,7 +189,7 @@ impl DenseAssetStorage { &mut self, index: AssetIndex, removed_action: impl FnOnce(&mut Self), - ) -> Option { + ) -> Option> { self.flush(); let value = match &mut self.storage[index.index as usize] { Entry::None => return None, @@ -202,7 +205,8 @@ impl DenseAssetStorage { value } - pub(crate) fn get(&self, index: AssetIndex) -> Option<&A> { + /// Gets a borrow to the [`Arc`]d asset. + pub(crate) fn get_arc(&self, index: AssetIndex) -> Option<&Arc> { let entry = self.storage.get(index.index as usize)?; match entry { Entry::None => None, @@ -271,7 +275,7 @@ impl DenseAssetStorage { #[derive(Resource)] pub struct Assets { dense_storage: DenseAssetStorage, - hash_map: HashMap, + hash_map: HashMap>, handle_provider: AssetHandleProvider, queued_events: Vec>, /// Assets managed by the `Assets` struct with live strong `Handle`s @@ -307,7 +311,8 @@ impl Assets { } /// Inserts the given `asset`, identified by the given `id`. If an asset already exists for `id`, it will be replaced. - pub fn insert(&mut self, id: impl Into>, asset: A) { + pub fn insert(&mut self, id: impl Into>, asset: impl Into>) { + let asset = asset.into(); match id.into() { AssetId::Index { index, .. } => { self.insert_with_index(index, asset).unwrap(); @@ -321,12 +326,12 @@ impl Assets { /// Returns `true` if the `id` exists in this collection. Otherwise it returns `false`. pub fn contains(&self, id: impl Into>) -> bool { match id.into() { - AssetId::Index { index, .. } => self.dense_storage.get(index).is_some(), + AssetId::Index { index, .. } => self.dense_storage.get_arc(index).is_some(), AssetId::Uuid { uuid } => self.hash_map.contains_key(&uuid), } } - pub(crate) fn insert_with_uuid(&mut self, uuid: Uuid, asset: A) -> Option { + pub(crate) fn insert_with_uuid(&mut self, uuid: Uuid, asset: Arc) -> Option> { let result = self.hash_map.insert(uuid, asset); if result.is_some() { self.queued_events @@ -340,7 +345,7 @@ impl Assets { pub(crate) fn insert_with_index( &mut self, index: AssetIndex, - asset: A, + asset: Arc, ) -> Result { let replaced = self.dense_storage.insert(index, asset)?; if replaced { @@ -355,7 +360,7 @@ impl Assets { /// Adds the given `asset` and allocates a new strong [`Handle`] for it. #[inline] - pub fn add(&mut self, asset: impl Into) -> Handle { + pub fn add(&mut self, asset: impl Into>) -> Handle { let index = self.dense_storage.allocator.reserve(); self.insert_with_index(index, asset.into()).unwrap(); Handle::Strong( @@ -388,14 +393,24 @@ impl Assets { #[inline] pub fn get(&self, id: impl Into>) -> Option<&A> { match id.into() { - AssetId::Index { index, .. } => self.dense_storage.get(index), - AssetId::Uuid { uuid } => self.hash_map.get(&uuid), + AssetId::Index { index, .. } => self.dense_storage.get_arc(index).map(|a| &**a), + AssetId::Uuid { uuid } => self.hash_map.get(&uuid).map(|a| &**a), + } + } + + /// Retrieves the [`Arc`] of an [`Asset`] with the given `id`, if it exists. + /// Note that this supports anything that implements `Into>`, which includes [`Handle`] and [`AssetId`]. + #[inline] + pub fn get_arc(&self, id: impl Into>) -> Option> { + match id.into() { + AssetId::Index { index, .. } => self.dense_storage.get_arc(index).cloned(), + AssetId::Uuid { uuid } => self.hash_map.get(&uuid).cloned(), } } /// Removes (and returns) the [`Asset`] with the given `id`, if it exists. /// Note that this supports anything that implements `Into>`, which includes [`Handle`] and [`AssetId`]. - pub fn remove(&mut self, id: impl Into>) -> Option { + pub fn remove(&mut self, id: impl Into>) -> Option> { let id: AssetId = id.into(); let result = self.remove_untracked(id); if result.is_some() { @@ -406,7 +421,7 @@ impl Assets { /// Removes (and returns) the [`Asset`] with the given `id`, if it exists. This skips emitting [`AssetEvent::Removed`]. /// Note that this supports anything that implements `Into>`, which includes [`Handle`] and [`AssetId`]. - pub fn remove_untracked(&mut self, id: impl Into>) -> Option { + pub fn remove_untracked(&mut self, id: impl Into>) -> Option> { let id: AssetId = id.into(); self.duplicate_handles.remove(&id); match id { @@ -452,7 +467,7 @@ impl Assets { /// Returns an iterator over the [`AssetId`] and [`Asset`] ref of every asset in this collection. // PERF: this could be accelerated if we implement a skip list. Consider the cost/benefits - pub fn iter(&self) -> impl Iterator, &A)> { + pub fn iter(&self) -> impl Iterator, Arc)> + '_ { self.dense_storage .storage .iter() @@ -467,13 +482,13 @@ impl Assets { }, marker: PhantomData, }; - (id, v) + (id, v.clone()) }), }) .chain( self.hash_map .iter() - .map(|(i, v)| (AssetId::Uuid { uuid: *i }, v)), + .map(|(i, v)| (AssetId::Uuid { uuid: *i }, v.clone())), ) } @@ -551,11 +566,11 @@ impl Assets { pub struct AssetsMutIterator<'a, A: Asset> { queued_events: &'a mut Vec>, dense_storage: Enumerate>>, - hash_map: bevy_utils::hashbrown::hash_map::IterMut<'a, Uuid, A>, + hash_map: bevy_utils::hashbrown::hash_map::IterMut<'a, Uuid, Arc>, } impl<'a, A: Asset> Iterator for AssetsMutIterator<'a, A> { - type Item = (AssetId, &'a mut A); + type Item = (AssetId, &'a mut Arc); fn next(&mut self) -> Option { for (i, entry) in &mut self.dense_storage { From b24e0506fcae089070759afd0c5bd8dfc265aae7 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Tue, 8 Oct 2024 23:13:43 -0700 Subject: [PATCH 03/20] Separate out inserting/adding an owned asset from the arc version. Many callers expect to be able to call `meshes.add(Rectangle::new(50.0, 100.0))`. Now we can keep all these callsites the same (while still supporting inserting/adding `Arc`s directly). --- crates/bevy_asset/src/assets.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 8c1cf3e6a4092..260f3e88039de 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -310,8 +310,15 @@ impl Assets { self.handle_provider.reserve_handle().typed::() } - /// Inserts the given `asset`, identified by the given `id`. If an asset already exists for `id`, it will be replaced. - pub fn insert(&mut self, id: impl Into>, asset: impl Into>) { + /// Inserts the given `asset`, identified by the given `id`. If an asset already exists for + /// `id`, it will be replaced. + pub fn insert(&mut self, id: impl Into>, asset: A) { + self.insert_arc(id, asset); + } + + /// Inserts the given [`Arc`]-ed `asset`, identified by the given `id`. If an asset already + /// exists for `id`, it will be replaced. + pub fn insert_arc(&mut self, id: impl Into>, asset: impl Into>) { let asset = asset.into(); match id.into() { AssetId::Index { index, .. } => { @@ -360,7 +367,13 @@ impl Assets { /// Adds the given `asset` and allocates a new strong [`Handle`] for it. #[inline] - pub fn add(&mut self, asset: impl Into>) -> Handle { + pub fn add(&mut self, asset: impl Into) -> Handle { + self.add_arc(asset.into()) + } + + /// Adds the given [`Arc`]-ed `asset` and allocates a new strong [`Handle`] for it. + #[inline] + pub fn add_arc(&mut self, asset: impl Into>) -> Handle { let index = self.dense_storage.allocator.reserve(); self.insert_with_index(index, asset.into()).unwrap(); Handle::Strong( From ea5703a7fe91c1ee502b1b5127ef59335a148703 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Tue, 8 Oct 2024 00:29:32 -0700 Subject: [PATCH 04/20] Implement `get_cloned_mut`. Now for assets that impl `Clone`, we can get a mutable borrow to the asset always by cloning. --- crates/bevy_asset/src/assets.rs | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 260f3e88039de..6b098cf2326e3 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -220,6 +220,21 @@ impl DenseAssetStorage { } } + /// Gets a mutable borrow to the [`Arc`]d asset. + /// + /// Returns [`None`] if the asset is missing, or has a different generation. + pub(crate) fn get_arc_mut(&mut self, index: AssetIndex) -> Option<&mut Arc> { + let entry = self.storage.get_mut(index.index as usize)?; + let Entry::Some { value, generation } = entry else { + return None; + }; + + if *generation != index.generation { + return None; + } + value.as_mut() + } + pub(crate) fn flush(&mut self) { // NOTE: this assumes the allocator index is monotonically increasing. let new_len = self @@ -575,6 +590,26 @@ impl Assets { } } +impl Assets { + /// Retrieves a mutable reference to the [`Asset`] with the given `id` if it exists. + /// + /// If the asset is currently aliased (another [`Arc`] or [`Weak`] to this asset exists), the + /// asset is cloned. + /// + /// [`Weak`]: std::sync::Weak + pub fn get_cloned_mut(&mut self, id: impl Into>) -> Option<&mut A> { + let id = id.into(); + let arc = match id { + AssetId::Index { index, .. } => self.dense_storage.get_arc_mut(index)?, + AssetId::Uuid { uuid } => self.hash_map.get_mut(&uuid)?, + }; + + self.queued_events.push(AssetEvent::Modified { id }); + // This clones the asset if the asset is aliased. + Some(Arc::make_mut(arc)) + } +} + /// A mutable iterator over [`Assets`]. pub struct AssetsMutIterator<'a, A: Asset> { queued_events: &'a mut Vec>, From 8858784df4d7110270a63cefb6e2f0a7d554df6b Mon Sep 17 00:00:00 2001 From: andriyDev Date: Mon, 7 Oct 2024 23:18:29 -0700 Subject: [PATCH 05/20] Implement `get_inplace_mut`. Now we can mutate assets in place as long as there are no more Arcs or Weaks for that asset. --- crates/bevy_asset/src/assets.rs | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 6b098cf2326e3..a5e2bbf3d7eff 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -436,6 +436,33 @@ impl Assets { } } + /// Retrieves a mutable reference to the [`Asset`] with the given `id` if it exists. + /// + /// If the asset is currently aliased (another [`Arc`] or [`Weak`] to this asset exists), + /// returns an error. + /// + /// [`Weak`]: std::sync::Weak + pub fn get_inplace_mut( + &mut self, + id: impl Into>, + ) -> Result<&mut A, MutableAssetError> { + let id = id.into(); + let arc = match id { + AssetId::Index { index, .. } => self.dense_storage.get_arc_mut(index), + AssetId::Uuid { uuid } => self.hash_map.get_mut(&uuid), + }; + + let Some(arc) = arc else { + return Err(MutableAssetError::Missing); + }; + let Some(asset_mut) = Arc::get_mut(arc) else { + return Err(MutableAssetError::Aliased); + }; + + self.queued_events.push(AssetEvent::Modified { id }); + Ok(asset_mut) + } + /// Removes (and returns) the [`Asset`] with the given `id`, if it exists. /// Note that this supports anything that implements `Into>`, which includes [`Handle`] and [`AssetId`]. pub fn remove(&mut self, id: impl Into>) -> Option> { @@ -658,6 +685,14 @@ pub struct InvalidGenerationError { current_generation: u32, } +#[derive(Error, Display, Debug)] +pub enum MutableAssetError { + #[display("The asset is not present or has an invalid generation.")] + Missing, + #[display("The asset Arc is aliased (there is another Arc or Weak to this asset), so it is not safe to mutate.")] + Aliased, +} + #[cfg(test)] mod test { use crate::AssetIndex; From adab62e81d245c25136590acbf6f959912c6ce07 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Tue, 8 Oct 2024 17:08:03 -0700 Subject: [PATCH 06/20] Implement `get_reflect_cloned_mut`. This is basically the same as `get_cloned_mut` but using reflection. This is not really intended to be used publicly, but the reflect.rs needs a clone that only relies on reflection, so making it public seems fine. --- crates/bevy_asset/src/assets.rs | 36 ++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index a5e2bbf3d7eff..53af5be5acbef 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -8,7 +8,7 @@ use bevy_ecs::{ prelude::EventWriter, system::{Res, ResMut, Resource, SystemChangeTick}, }; -use bevy_reflect::{Reflect, TypePath}; +use bevy_reflect::{FromReflect, Reflect, TypePath}; use bevy_utils::HashMap; use core::{any::TypeId, iter::Enumerate, marker::PhantomData, sync::atomic::AtomicU32}; use crossbeam_channel::{Receiver, Sender}; @@ -637,6 +637,40 @@ impl Assets { } } +impl Assets { + /// Retrieves a mutable reference to the [`Asset`] with the given `id` if it exists. + /// + /// If the asset is currently aliased (another [`Arc`] or [`Weak`] to this asset exists), the + /// asset is "cloned" using reflection. + /// + /// [`Weak`]: std::sync::Weak + pub fn get_reflect_cloned_mut(&mut self, id: impl Into>) -> Option<&mut A> { + let id = id.into(); + let arc = match id { + AssetId::Index { index, .. } => self.dense_storage.get_arc_mut(index)?, + AssetId::Uuid { uuid } => self.hash_map.get_mut(&uuid)?, + }; + + self.queued_events.push(AssetEvent::Modified { id }); + if Arc::get_mut(arc).is_some() { + // This is a workaround to the lack of polonius (the problem described at + // https://rust-lang.github.io/rfcs/2094-nll.html#problem-case-3-conditional-control-flow-across-functions) + // Since we can get mutable access to the `Arc` and the value inside the `Arc`, it is + // impossible for us to "lose" access in between these calls. + return Some( + Arc::get_mut(arc) + .expect("the Arc is aliased somehow even though we just got it mutably in `Assets::get_reflect_cloned_mut`."), + ); + } + + let cloned_asset = FromReflect::from_reflect(arc.as_ref()).expect( + "could not call `FromReflect::from_reflect` in `Assets::get_reflect_cloned_mut`", + ); + *arc = Arc::new(cloned_asset); + Some(Arc::get_mut(arc).expect("the Arc is aliased somehow even though we just cloned it in `Assets::get_reflect_cloned_mut`.")) + } +} + /// A mutable iterator over [`Assets`]. pub struct AssetsMutIterator<'a, A: Asset> { queued_events: &'a mut Vec>, From 8a65303eb147db8bac551b0ac2a8bd90193c0d91 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Tue, 8 Oct 2024 17:12:07 -0700 Subject: [PATCH 07/20] Update most calls in bevy_asset to either use a `.into()` or one of the `get_*_mut` variants. --- crates/bevy_asset/src/direct_access_ext.rs | 13 +++++++++++ crates/bevy_asset/src/lib.rs | 2 +- crates/bevy_asset/src/loader.rs | 4 +++- crates/bevy_asset/src/reflect.rs | 25 +++++++++++++++++----- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/crates/bevy_asset/src/direct_access_ext.rs b/crates/bevy_asset/src/direct_access_ext.rs index bfa7fa17b29c0..e164987ace894 100644 --- a/crates/bevy_asset/src/direct_access_ext.rs +++ b/crates/bevy_asset/src/direct_access_ext.rs @@ -1,6 +1,8 @@ //! Add methods on `World` to simplify loading assets when all //! you have is a `World`. +use alloc::sync::Arc; + use bevy_ecs::world::World; use crate::{meta::Settings, Asset, AssetPath, AssetServer, Assets, Handle}; @@ -9,6 +11,9 @@ pub trait DirectAssetAccessExt { /// Insert an asset similarly to [`Assets::add`]. fn add_asset(&mut self, asset: impl Into) -> Handle; + /// Insert an [`Arc`]ed asset similarly to [`Assets::add_arc`]. + fn add_arc_asset(&mut self, asset: impl Into>) -> Handle; + /// Load an asset similarly to [`AssetServer::load`]. fn load_asset<'a, A: Asset>(&self, path: impl Into>) -> Handle; @@ -28,6 +33,14 @@ impl DirectAssetAccessExt for World { self.resource_mut::>().add(asset) } + /// Insert an [`Arc`]ed asset similarly to [`Assets::add_arc`]. + /// + /// # Panics + /// If `self` doesn't have an [`AssetServer`] resource initialized yet. + fn add_arc_asset(&mut self, asset: impl Into>) -> Handle { + self.resource_mut::>().add_arc(asset) + } + /// Load an asset similarly to [`AssetServer::load`]. /// /// # Panics diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 2568badb7e4a5..d90a8fbb7dd1e 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -1083,7 +1083,7 @@ mod tests { { let mut texts = app.world_mut().resource_mut::>(); - let a = texts.get_mut(a_id).unwrap(); + let a = texts.get_inplace_mut(a_id).unwrap(); a.text = "Changed".to_string(); } diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 96515460e494a..8c7aaaeef2239 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -277,7 +277,9 @@ impl_downcast!(AssetContainer); impl AssetContainer for A { fn insert(self: Box, id: UntypedAssetId, world: &mut World) { - world.resource_mut::>().insert(id.typed(), *self); + world + .resource_mut::>() + .insert_arc(id.typed(), self); } fn asset_type_name(&self) -> &'static str { diff --git a/crates/bevy_asset/src/reflect.rs b/crates/bevy_asset/src/reflect.rs index 3aaa1580bb110..d5238a35759e6 100644 --- a/crates/bevy_asset/src/reflect.rs +++ b/crates/bevy_asset/src/reflect.rs @@ -1,3 +1,4 @@ +use alloc::sync::Arc; use core::any::{Any, TypeId}; use bevy_ecs::world::{unsafe_world_cell::UnsafeWorldCell, World}; @@ -18,6 +19,7 @@ pub struct ReflectAsset { assets_resource_type_id: TypeId, get: fn(&World, UntypedHandle) -> Option<&dyn Reflect>, + get_arc: fn(&World, UntypedHandle) -> Option>, // SAFETY: // - may only be called with an [`UnsafeWorldCell`] which can be used to access the corresponding `Assets` resource mutably // - may only be used to access **at most one** access at once @@ -45,7 +47,12 @@ impl ReflectAsset { (self.get)(world, handle) } - /// Equivalent of [`Assets::get_mut`] + /// Equivalent of [`Assets::get_arc`] + pub fn get_arc(&self, world: &World, handle: UntypedHandle) -> Option> { + (self.get_arc)(world, handle) + } + + /// Equivalent of [`Assets::get_reflect_cloned_mut`] pub fn get_mut<'w>( &self, world: &'w mut World, @@ -61,7 +68,7 @@ impl ReflectAsset { } } - /// Equivalent of [`Assets::get_mut`], but works with an [`UnsafeWorldCell`]. + /// Equivalent of [`Assets::get_reflect_cloned_mut`], but works with an [`UnsafeWorldCell`]. /// /// Only use this method when you have ensured that you are the *only* one with access to the [`Assets`] resource of the asset type. /// Furthermore, this does *not* allow you to have look up two distinct handles, @@ -139,15 +146,23 @@ impl FromType for ReflectAsset { get: |world, handle| { let assets = world.resource::>(); let asset = assets.get(&handle.typed_debug_checked()); - asset.map(|asset| asset as &dyn Reflect) + asset.map(|asset| asset as _) + }, + get_arc: |world, handle| { + let assets = world.resource::>(); + let asset = assets.get_arc(&handle.typed_debug_checked()); + asset.map(|asset| asset as _) }, get_unchecked_mut: |world, handle| { // SAFETY: `get_unchecked_mut` must be called with `UnsafeWorldCell` having access to `Assets`, // and must ensure to only have at most one reference to it live at all times. #[expect(unsafe_code, reason = "Uses `UnsafeWorldCell::get_resource_mut()`.")] let assets = unsafe { world.get_resource_mut::>().unwrap().into_inner() }; - let asset = assets.get_mut(&handle.typed_debug_checked()); - asset.map(|asset| asset as &mut dyn Reflect) + + let handle = handle.typed_debug_checked(); + assets + .get_reflect_cloned_mut(&handle) + .map(|asset| asset as _) }, add: |world, value| { let mut assets = world.resource_mut::>(); From 6e6de653e9c024d56782d1fc02875a8ce23d580e Mon Sep 17 00:00:00 2001 From: andriyDev Date: Tue, 8 Oct 2024 21:50:20 -0700 Subject: [PATCH 08/20] Make RenderAssets take an `Arc` to the source asset and extract source assets as Arcs. --- crates/bevy_render/src/mesh/mod.rs | 4 +++- crates/bevy_render/src/render_asset.rs | 16 +++++++++------- crates/bevy_render/src/storage.rs | 8 +++++--- crates/bevy_render/src/texture/gpu_image.rs | 17 ++++++++--------- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/crates/bevy_render/src/mesh/mod.rs b/crates/bevy_render/src/mesh/mod.rs index 7a7829e0f4ef1..2b1cae2b2b696 100644 --- a/crates/bevy_render/src/mesh/mod.rs +++ b/crates/bevy_render/src/mesh/mod.rs @@ -1,3 +1,5 @@ +use alloc::sync::Arc; + use bevy_hierarchy::Children; use bevy_math::Vec3; pub use bevy_mesh::*; @@ -168,7 +170,7 @@ impl RenderAsset for RenderMesh { /// Converts the extracted mesh into a [`RenderMesh`]. fn prepare_asset( - mesh: Self::SourceAsset, + mesh: Arc, _: AssetId, (images, ref mut mesh_vertex_buffer_layouts): &mut SystemParamItem, ) -> Result> { diff --git a/crates/bevy_render/src/render_asset.rs b/crates/bevy_render/src/render_asset.rs index 2757dceb9fa81..fbbcf6e59bed0 100644 --- a/crates/bevy_render/src/render_asset.rs +++ b/crates/bevy_render/src/render_asset.rs @@ -1,6 +1,7 @@ use crate::{ render_resource::AsBindGroupError, ExtractSchedule, MainWorld, Render, RenderApp, RenderSet, }; +use alloc::sync::Arc; use bevy_app::{App, Plugin, SubApp}; pub use bevy_asset::RenderAssetUsages; use bevy_asset::{Asset, AssetEvent, AssetId, Assets}; @@ -21,7 +22,7 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum PrepareAssetError { #[error("Failed to prepare asset")] - RetryNextUpdate(E), + RetryNextUpdate(Arc), #[error("Failed to build bind group: {0}")] AsBindGroupError(AsBindGroupError), } @@ -62,9 +63,10 @@ pub trait RenderAsset: Send + Sync + 'static + Sized { /// Prepares the [`RenderAsset::SourceAsset`] for the GPU by transforming it into a [`RenderAsset`]. /// - /// ECS data may be accessed via `param`. + /// ECS data may be accessed via `param`. If you want to move the `source_asset`s fields, + /// consider using [`Arc::try_unwrap`] (and falling back to cloning the asset otherwise). fn prepare_asset( - source_asset: Self::SourceAsset, + source_asset: Arc, asset_id: AssetId, param: &mut SystemParamItem, ) -> Result>; @@ -149,7 +151,7 @@ impl RenderAssetDependency for A { #[derive(Resource)] pub struct ExtractedAssets { /// The assets extracted this frame. - pub extracted: Vec<(AssetId, A::SourceAsset)>, + pub extracted: Vec<(AssetId, Arc)>, /// IDs of the assets removed this frame. /// @@ -256,8 +258,8 @@ pub(crate) fn extract_render_asset( let mut extracted_assets = Vec::new(); let mut added = >::default(); for id in changed_assets.drain() { - if let Some(asset) = assets.get(id) { - let asset_usage = A::asset_usage(asset); + if let Some(asset) = assets.get_arc(id) { + let asset_usage = A::asset_usage(&asset); if asset_usage.contains(RenderAssetUsages::RENDER_WORLD) { if asset_usage == RenderAssetUsages::RENDER_WORLD { if let Some(asset) = assets.remove(id) { @@ -286,7 +288,7 @@ pub(crate) fn extract_render_asset( /// All assets that should be prepared next frame. #[derive(Resource)] pub struct PrepareNextFrameAssets { - assets: Vec<(AssetId, A::SourceAsset)>, + assets: Vec<(AssetId, Arc)>, } impl Default for PrepareNextFrameAssets { diff --git a/crates/bevy_render/src/storage.rs b/crates/bevy_render/src/storage.rs index 7434f3999f3dc..309dc2987b735 100644 --- a/crates/bevy_render/src/storage.rs +++ b/crates/bevy_render/src/storage.rs @@ -1,3 +1,5 @@ +use alloc::sync::Arc; + use crate::{ render_asset::{PrepareAssetError, RenderAsset, RenderAssetPlugin, RenderAssetUsages}, render_resource::{Buffer, BufferUsages}, @@ -113,15 +115,15 @@ impl RenderAsset for GpuShaderStorageBuffer { } fn prepare_asset( - source_asset: Self::SourceAsset, + source_asset: Arc, _: AssetId, render_device: &mut SystemParamItem, ) -> Result> { - match source_asset.data { + match source_asset.data.as_ref() { Some(data) => { let buffer = render_device.create_buffer_with_data(&BufferInitDescriptor { label: source_asset.buffer_description.label, - contents: &data, + contents: data, usage: source_asset.buffer_description.usage, }); Ok(GpuShaderStorageBuffer { buffer }) diff --git a/crates/bevy_render/src/texture/gpu_image.rs b/crates/bevy_render/src/texture/gpu_image.rs index f1ee1ade7e8f5..f74ff79a4e92e 100644 --- a/crates/bevy_render/src/texture/gpu_image.rs +++ b/crates/bevy_render/src/texture/gpu_image.rs @@ -1,3 +1,5 @@ +use alloc::sync::Arc; + use crate::{ render_asset::{PrepareAssetError, RenderAsset, RenderAssetUsages}, render_resource::{DefaultImageSampler, Sampler, Texture, TextureView}, @@ -41,7 +43,7 @@ impl RenderAsset for GpuImage { /// Converts the extracted image into a [`GpuImage`]. fn prepare_asset( - image: Self::SourceAsset, + image: Arc, _: AssetId, (render_device, render_queue, default_sampler): &mut SystemParamItem, ) -> Result> { @@ -53,14 +55,11 @@ impl RenderAsset for GpuImage { &image.data, ); - let texture_view = texture.create_view( - image - .texture_view_descriptor - .or_else(|| Some(TextureViewDescriptor::default())) - .as_ref() - .unwrap(), - ); - let sampler = match image.sampler { + let texture_view = match image.texture_view_descriptor.as_ref() { + None => texture.create_view(&TextureViewDescriptor::default()), + Some(descriptor) => texture.create_view(descriptor), + }; + let sampler = match &image.sampler { ImageSampler::Default => (***default_sampler).clone(), ImageSampler::Descriptor(descriptor) => { render_device.create_sampler(&descriptor.as_wgpu()) From 8c016a143dad0ef037f01008ee73afd493f965d8 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Wed, 9 Oct 2024 00:43:19 -0700 Subject: [PATCH 09/20] Make all the crates use the new `get_*_mut` variants, or use `Arc` in `RenderAsset`. --- .../src/auto_exposure/compensation_curve.rs | 4 +++- crates/bevy_core_pipeline/src/lib.rs | 2 ++ crates/bevy_gizmos/src/lib.rs | 7 +++++-- crates/bevy_pbr/src/material.rs | 3 ++- crates/bevy_pbr/src/wireframe.rs | 2 +- crates/bevy_sprite/src/mesh2d/material.rs | 3 ++- crates/bevy_sprite/src/mesh2d/wireframe2d.rs | 2 +- crates/bevy_text/src/font_atlas.rs | 5 +++-- crates/bevy_ui/src/lib.rs | 2 ++ crates/bevy_ui/src/render/ui_material_pipeline.rs | 3 ++- 10 files changed, 23 insertions(+), 10 deletions(-) diff --git a/crates/bevy_core_pipeline/src/auto_exposure/compensation_curve.rs b/crates/bevy_core_pipeline/src/auto_exposure/compensation_curve.rs index 25ec27cee4df2..4a9b9b39358bd 100644 --- a/crates/bevy_core_pipeline/src/auto_exposure/compensation_curve.rs +++ b/crates/bevy_core_pipeline/src/auto_exposure/compensation_curve.rs @@ -1,3 +1,5 @@ +use alloc::sync::Arc; + use bevy_asset::prelude::*; use bevy_ecs::system::{lifetimeless::SRes, SystemParamItem}; use bevy_math::{cubic_splines::CubicGenerator, FloatExt, Vec2}; @@ -190,7 +192,7 @@ impl RenderAsset for GpuAutoExposureCompensationCurve { } fn prepare_asset( - source: Self::SourceAsset, + source: Arc, _: AssetId, (render_device, render_queue): &mut SystemParamItem, ) -> Result> { diff --git a/crates/bevy_core_pipeline/src/lib.rs b/crates/bevy_core_pipeline/src/lib.rs index e94daa90f4bdc..7f31b08eea9c2 100644 --- a/crates/bevy_core_pipeline/src/lib.rs +++ b/crates/bevy_core_pipeline/src/lib.rs @@ -6,6 +6,8 @@ html_favicon_url = "https://bevyengine.org/assets/icon.png" )] +extern crate alloc; + pub mod auto_exposure; pub mod blit; pub mod bloom; diff --git a/crates/bevy_gizmos/src/lib.rs b/crates/bevy_gizmos/src/lib.rs index 242f19bc6bd5d..c9e1fc92e618b 100644 --- a/crates/bevy_gizmos/src/lib.rs +++ b/crates/bevy_gizmos/src/lib.rs @@ -121,6 +121,9 @@ use { bytemuck::cast_slice, }; +extern crate alloc; + +use alloc::sync::Arc; #[cfg(all( feature = "bevy_render", any(feature = "bevy_pbr", feature = "bevy_sprite"), @@ -389,7 +392,7 @@ fn update_gizmo_meshes( handles.handles.insert(TypeId::of::(), None); } else if let Some(handle) = handles.handles.get_mut(&TypeId::of::()) { if let Some(handle) = handle { - let gizmo = gizmo_assets.get_mut(handle.id()).unwrap(); + let gizmo = gizmo_assets.get_cloned_mut(handle.id()).unwrap(); gizmo.buffer.list_positions = mem::take(&mut storage.list_positions); gizmo.buffer.list_colors = mem::take(&mut storage.list_colors); @@ -548,7 +551,7 @@ impl RenderAsset for GpuLineGizmo { type Param = SRes; fn prepare_asset( - gizmo: Self::SourceAsset, + gizmo: Arc, _: AssetId, render_device: &mut SystemParamItem, ) -> Result> { diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index f894774af140d..2b837d9714d5b 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -6,6 +6,7 @@ use crate::meshlet::{ InstanceManager, }; use crate::*; +use alloc::sync::Arc; use bevy_asset::{Asset, AssetId, AssetServer}; use bevy_core_pipeline::{ core_3d::{ @@ -1026,7 +1027,7 @@ impl RenderAsset for PreparedMaterial { ); fn prepare_asset( - material: Self::SourceAsset, + material: Arc, material_id: AssetId, ( render_device, diff --git a/crates/bevy_pbr/src/wireframe.rs b/crates/bevy_pbr/src/wireframe.rs index 413933135c85a..35a567d5c69e5 100644 --- a/crates/bevy_pbr/src/wireframe.rs +++ b/crates/bevy_pbr/src/wireframe.rs @@ -121,7 +121,7 @@ fn global_color_changed( mut materials: ResMut>, global_material: Res, ) { - if let Some(global_material) = materials.get_mut(&global_material.handle) { + if let Some(global_material) = materials.get_cloned_mut(&global_material.handle) { global_material.color = config.default_color.into(); } } diff --git a/crates/bevy_sprite/src/mesh2d/material.rs b/crates/bevy_sprite/src/mesh2d/material.rs index fcc830301c554..d50998348f73c 100644 --- a/crates/bevy_sprite/src/mesh2d/material.rs +++ b/crates/bevy_sprite/src/mesh2d/material.rs @@ -4,6 +4,7 @@ use crate::{ DrawMesh2d, Mesh2d, Mesh2dPipeline, Mesh2dPipelineKey, RenderMesh2dInstances, SetMesh2dBindGroup, SetMesh2dViewBindGroup, }; +use alloc::sync::Arc; use bevy_app::{App, Plugin}; use bevy_asset::{Asset, AssetApp, AssetId, AssetServer, Handle}; use bevy_core_pipeline::{ @@ -648,7 +649,7 @@ impl RenderAsset for PreparedMaterial2d { type Param = (SRes, SRes>, M::Param); fn prepare_asset( - material: Self::SourceAsset, + material: Arc, _: AssetId, (render_device, pipeline, material_param): &mut SystemParamItem, ) -> Result> { diff --git a/crates/bevy_sprite/src/mesh2d/wireframe2d.rs b/crates/bevy_sprite/src/mesh2d/wireframe2d.rs index 6f2659fbaaf91..6871d7df1c513 100644 --- a/crates/bevy_sprite/src/mesh2d/wireframe2d.rs +++ b/crates/bevy_sprite/src/mesh2d/wireframe2d.rs @@ -118,7 +118,7 @@ fn global_color_changed( mut materials: ResMut>, global_material: Res, ) { - if let Some(global_material) = materials.get_mut(&global_material.handle) { + if let Some(global_material) = materials.get_cloned_mut(&global_material.handle) { global_material.color = config.default_color.into(); } } diff --git a/crates/bevy_text/src/font_atlas.rs b/crates/bevy_text/src/font_atlas.rs index 4ce4ea62072db..2da8870ea65ec 100644 --- a/crates/bevy_text/src/font_atlas.rs +++ b/crates/bevy_text/src/font_atlas.rs @@ -95,8 +95,9 @@ impl FontAtlas { texture: &Image, offset: IVec2, ) -> Result<(), TextError> { - let atlas_layout = atlas_layouts.get_mut(&self.texture_atlas).unwrap(); - let atlas_texture = textures.get_mut(&self.texture).unwrap(); + // Accessing these assets mutably (especially the texture) could incur a large cloning cost. + let atlas_layout = atlas_layouts.get_cloned_mut(&self.texture_atlas).unwrap(); + let atlas_texture = textures.get_cloned_mut(&self.texture).unwrap(); if let Some(glyph_index) = self.dynamic_texture_atlas_builder diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs index 508f133b81cd9..1988d623c53c9 100644 --- a/crates/bevy_ui/src/lib.rs +++ b/crates/bevy_ui/src/lib.rs @@ -10,6 +10,8 @@ //! Spawn UI elements with [`widget::Button`], [`ImageNode`], [`Text`](prelude::Text) and [`Node`] //! This UI is laid out with the Flexbox and CSS Grid layout models (see ) +extern crate alloc; + pub mod measurement; pub mod node_bundles; pub mod ui_material; diff --git a/crates/bevy_ui/src/render/ui_material_pipeline.rs b/crates/bevy_ui/src/render/ui_material_pipeline.rs index 481dce07d37f1..5e17744488ef1 100644 --- a/crates/bevy_ui/src/render/ui_material_pipeline.rs +++ b/crates/bevy_ui/src/render/ui_material_pipeline.rs @@ -1,3 +1,4 @@ +use alloc::sync::Arc; use core::{hash::Hash, marker::PhantomData, ops::Range}; use crate::*; @@ -587,7 +588,7 @@ impl RenderAsset for PreparedUiMaterial { type Param = (SRes, SRes>, M::Param); fn prepare_asset( - material: Self::SourceAsset, + material: Arc, _: AssetId, (render_device, pipeline, ref mut material_param): &mut SystemParamItem, ) -> Result> { From 3c66b59e88dbcf8d20a48a5a4a69dd5276f1c11e Mon Sep 17 00:00:00 2001 From: andriyDev Date: Wed, 9 Oct 2024 00:52:28 -0700 Subject: [PATCH 10/20] Update the examples to use the `get_*_mut` variants or otherwise dealing with `Arc`s. --- examples/2d/cpu_draw.rs | 4 +++- examples/2d/texture_atlas.rs | 2 +- examples/3d/animated_material.rs | 2 +- examples/3d/blend_modes.rs | 2 +- examples/3d/depth_of_field.rs | 5 ++++- examples/3d/generate_custom_mesh.rs | 2 +- examples/3d/lightmaps.rs | 15 ++++++++++++--- examples/3d/parallax_mapping.rs | 8 ++++---- examples/3d/query_gltf_primitives.rs | 4 ++-- examples/3d/skybox.rs | 2 +- examples/3d/tonemapping.rs | 2 +- examples/3d/transmission.rs | 2 +- examples/3d/transparency_3d.rs | 4 +++- examples/animation/animated_fox.rs | 2 +- examples/animation/animation_masks.rs | 3 ++- examples/app/headless_renderer.rs | 2 +- examples/asset/alter_mesh.rs | 2 +- examples/asset/alter_sprite.rs | 2 +- examples/asset/asset_decompression.rs | 7 +++++-- examples/shader/array_texture.rs | 2 +- examples/shader/shader_prepass.rs | 2 +- examples/shader/storage_buffer.rs | 4 ++-- .../tools/scene_viewer/scene_viewer_plugin.rs | 4 +++- examples/ui/ui_material.rs | 2 +- 24 files changed, 54 insertions(+), 32 deletions(-) diff --git a/examples/2d/cpu_draw.rs b/examples/2d/cpu_draw.rs index 0faea0a4ef8f9..247eea410d199 100644 --- a/examples/2d/cpu_draw.rs +++ b/examples/2d/cpu_draw.rs @@ -106,7 +106,9 @@ fn draw( } // Get the image from Bevy's asset storage. - let image = images.get_mut(&my_handle.0).expect("Image not found"); + let image = images + .get_cloned_mut(&my_handle.0) + .expect("Image not found"); // Compute the position of the pixel to draw. diff --git a/examples/2d/texture_atlas.rs b/examples/2d/texture_atlas.rs index 43822ad4fe449..468bb3e3c9350 100644 --- a/examples/2d/texture_atlas.rs +++ b/examples/2d/texture_atlas.rs @@ -242,7 +242,7 @@ fn create_texture_atlas( let texture = textures.add(texture); // Update the sampling settings of the texture atlas - let image = textures.get_mut(&texture).unwrap(); + let image = textures.get_cloned_mut(&texture).unwrap(); image.sampler = sampling.unwrap_or_default(); (texture_atlas_layout, texture_atlas_sources, texture) diff --git a/examples/3d/animated_material.rs b/examples/3d/animated_material.rs index a4d6046ff08f5..f7a907d63a239 100644 --- a/examples/3d/animated_material.rs +++ b/examples/3d/animated_material.rs @@ -50,7 +50,7 @@ fn animate_materials( mut materials: ResMut>, ) { for material_handle in material_handles.iter() { - if let Some(material) = materials.get_mut(material_handle) { + if let Some(material) = materials.get_cloned_mut(material_handle) { if let Color::Hsla(ref mut hsla) = material.base_color { *hsla = hsla.rotate_hue(time.delta_secs() * 100.0); } diff --git a/examples/3d/blend_modes.rs b/examples/3d/blend_modes.rs index b9f4343102898..1fbe418905b61 100644 --- a/examples/3d/blend_modes.rs +++ b/examples/3d/blend_modes.rs @@ -271,7 +271,7 @@ fn example_control_system( let randomize_colors = input.just_pressed(KeyCode::KeyC); for (material_handle, controls) in &controllable { - let material = materials.get_mut(material_handle).unwrap(); + let material = materials.get_cloned_mut(material_handle).unwrap(); if controls.color && randomize_colors { material.base_color = Srgba { diff --git a/examples/3d/depth_of_field.rs b/examples/3d/depth_of_field.rs index d6ca77bbde0ca..68d7fa56df17b 100644 --- a/examples/3d/depth_of_field.rs +++ b/examples/3d/depth_of_field.rs @@ -198,7 +198,10 @@ fn tweak_scene( // Add a nice lightmap to the circuit board. for (entity, name, material) in named_entities.iter_mut() { if &**name == "CircuitBoard" { - materials.get_mut(material).unwrap().lightmap_exposure = 10000.0; + materials + .get_cloned_mut(material) + .unwrap() + .lightmap_exposure = 10000.0; commands.entity(entity).insert(Lightmap { image: asset_server.load("models/DepthOfFieldExample/CircuitBoardLightmap.hdr"), ..default() diff --git a/examples/3d/generate_custom_mesh.rs b/examples/3d/generate_custom_mesh.rs index 2222486ba1da7..5b1201e2a1845 100644 --- a/examples/3d/generate_custom_mesh.rs +++ b/examples/3d/generate_custom_mesh.rs @@ -79,7 +79,7 @@ fn input_handler( ) { if keyboard_input.just_pressed(KeyCode::Space) { let mesh_handle = mesh_query.get_single().expect("Query not successful"); - let mesh = meshes.get_mut(mesh_handle).unwrap(); + let mesh = meshes.get_cloned_mut(mesh_handle).unwrap(); toggle_texture(mesh); } if keyboard_input.pressed(KeyCode::KeyX) { diff --git a/examples/3d/lightmaps.rs b/examples/3d/lightmaps.rs index 0a2a7bcad2840..e95548171e61e 100644 --- a/examples/3d/lightmaps.rs +++ b/examples/3d/lightmaps.rs @@ -67,7 +67,10 @@ fn add_lightmaps_to_meshes( let exposure = 250.0; for (entity, name, material) in meshes.iter() { if &**name == "large_box" { - materials.get_mut(material).unwrap().lightmap_exposure = exposure; + materials + .get_cloned_mut(material) + .unwrap() + .lightmap_exposure = exposure; commands.entity(entity).insert(Lightmap { image: asset_server.load("lightmaps/CornellBox-Large.zstd.ktx2"), ..default() @@ -76,7 +79,10 @@ fn add_lightmaps_to_meshes( } if &**name == "small_box" { - materials.get_mut(material).unwrap().lightmap_exposure = exposure; + materials + .get_cloned_mut(material) + .unwrap() + .lightmap_exposure = exposure; commands.entity(entity).insert(Lightmap { image: asset_server.load("lightmaps/CornellBox-Small.zstd.ktx2"), ..default() @@ -85,7 +91,10 @@ fn add_lightmaps_to_meshes( } if name.starts_with("cornell_box") { - materials.get_mut(material).unwrap().lightmap_exposure = exposure; + materials + .get_cloned_mut(material) + .unwrap() + .lightmap_exposure = exposure; commands.entity(entity).insert(Lightmap { image: asset_server.load("lightmaps/CornellBox-Box.zstd.ktx2"), ..default() diff --git a/examples/3d/parallax_mapping.rs b/examples/3d/parallax_mapping.rs index 47d83a4e76960..61f9ca77adb8a 100644 --- a/examples/3d/parallax_mapping.rs +++ b/examples/3d/parallax_mapping.rs @@ -1,7 +1,7 @@ //! A simple 3D scene with a spinning cube with a normal map and depth map to demonstrate parallax mapping. //! Press left mouse button to cycle through different views. -use std::fmt; +use std::{fmt, sync::Arc}; use bevy::{image::ImageLoaderSettings, math::ops, prelude::*}; @@ -97,7 +97,7 @@ fn update_parallax_depth_scale( for (_, mat) in materials.iter_mut() { let current_depth = mat.parallax_depth_scale; let new_depth = current_depth.lerp(target_depth.0, DEPTH_CHANGE_RATE); - mat.parallax_depth_scale = new_depth; + Arc::make_mut(mat).parallax_depth_scale = new_depth; *writer.text(*text, 1) = format!("Parallax depth scale: {new_depth:.5}\n"); if (new_depth - current_depth).abs() <= 0.000000001 { *depth_update = false; @@ -122,7 +122,7 @@ fn switch_method( *writer.text(text_entity, 3) = format!("Method: {}\n", *current); for (_, mat) in materials.iter_mut() { - mat.parallax_mapping_method = current.0; + Arc::make_mut(mat).parallax_mapping_method = current.0; } } @@ -146,7 +146,7 @@ fn update_parallax_layers( *writer.text(text_entity, 2) = format!("Layers: {layer_count:.0}\n"); for (_, mat) in materials.iter_mut() { - mat.max_parallax_layer_count = layer_count; + Arc::make_mut(mat).max_parallax_layer_count = layer_count; } } diff --git a/examples/3d/query_gltf_primitives.rs b/examples/3d/query_gltf_primitives.rs index 780abeb977aae..d68a493ca1ae9 100644 --- a/examples/3d/query_gltf_primitives.rs +++ b/examples/3d/query_gltf_primitives.rs @@ -26,7 +26,7 @@ fn find_top_material_and_mesh( for (mat_handle, mesh_handle, name) in mat_query.iter() { // locate a material by material name if name.0 == "Top" { - if let Some(material) = materials.get_mut(mat_handle) { + if let Some(material) = materials.get_cloned_mut(mat_handle) { if let Color::Hsla(ref mut hsla) = material.base_color { *hsla = hsla.rotate_hue(time.delta_secs() * 100.0); } else { @@ -34,7 +34,7 @@ fn find_top_material_and_mesh( } } - if let Some(mesh) = meshes.get_mut(mesh_handle) { + if let Some(mesh) = meshes.get_cloned_mut(mesh_handle) { if let Some(VertexAttributeValues::Float32x3(positions)) = mesh.attribute_mut(Mesh::ATTRIBUTE_POSITION) { diff --git a/examples/3d/skybox.rs b/examples/3d/skybox.rs index dd797473bf57d..543a577dd2a94 100644 --- a/examples/3d/skybox.rs +++ b/examples/3d/skybox.rs @@ -148,7 +148,7 @@ fn asset_loaded( ) { if !cubemap.is_loaded && asset_server.load_state(&cubemap.image_handle).is_loaded() { info!("Swapping to {}...", CUBEMAPS[cubemap.index].0); - let image = images.get_mut(&cubemap.image_handle).unwrap(); + let image = images.get_cloned_mut(&cubemap.image_handle).unwrap(); // NOTE: PNGs do not have any metadata that could indicate they contain a cubemap texture, // so they appear as one texture. The following code reconfigures the texture as necessary. if image.texture_descriptor.array_layer_count() == 1 { diff --git a/examples/3d/tonemapping.rs b/examples/3d/tonemapping.rs index 875198e7fed3f..76fe0764c0014 100644 --- a/examples/3d/tonemapping.rs +++ b/examples/3d/tonemapping.rs @@ -208,7 +208,7 @@ fn drag_drop_image( }; for mat_h in &image_mat { - if let Some(mat) = materials.get_mut(mat_h) { + if let Some(mat) = materials.get_cloned_mut(mat_h) { mat.base_color_texture = Some(new_image.clone()); // Despawn the image viewer instructions diff --git a/examples/3d/transmission.rs b/examples/3d/transmission.rs index b6a6425baf495..d64a41323ef5e 100644 --- a/examples/3d/transmission.rs +++ b/examples/3d/transmission.rs @@ -440,7 +440,7 @@ fn example_control_system( let randomize_colors = input.just_pressed(KeyCode::KeyC); for (material_handle, controls) in &controllable { - let material = materials.get_mut(material_handle).unwrap(); + let material = materials.get_cloned_mut(material_handle).unwrap(); if controls.specular_transmission { material.specular_transmission = state.specular_transmission; material.thickness = state.thickness; diff --git a/examples/3d/transparency_3d.rs b/examples/3d/transparency_3d.rs index 35cb7c1d14f25..8f39b8cab1970 100644 --- a/examples/3d/transparency_3d.rs +++ b/examples/3d/transparency_3d.rs @@ -2,6 +2,8 @@ //! Shows the effects of different blend modes. //! The `fade_transparency` system smoothly changes the transparency over time. +use std::sync::Arc; + use bevy::{math::ops, prelude::*}; fn main() { @@ -110,6 +112,6 @@ fn setup( pub fn fade_transparency(time: Res)>, ) { for (entity, Compressed { compressed, .. }) in query.iter() { - let Some(GzAsset { uncompressed }) = compressed_assets.remove(compressed) else { + let Some(compressed) = compressed_assets.remove(compressed) else { continue; }; + let GzAsset { uncompressed } = + Arc::into_inner(compressed).expect("The asset Arc is not aliased."); + let uncompressed = uncompressed.take::().unwrap(); commands diff --git a/examples/shader/array_texture.rs b/examples/shader/array_texture.rs index 1d156c66e417e..6e061acab6b00 100644 --- a/examples/shader/array_texture.rs +++ b/examples/shader/array_texture.rs @@ -63,7 +63,7 @@ fn create_array_texture( return; } loading_texture.is_loaded = true; - let image = images.get_mut(&loading_texture.handle).unwrap(); + let image = images.get_cloned_mut(&loading_texture.handle).unwrap(); // Create a new array texture asset from the loaded texture. let array_layers = 4; diff --git a/examples/shader/shader_prepass.rs b/examples/shader/shader_prepass.rs index b53f9fe941729..027938248ed9a 100644 --- a/examples/shader/shader_prepass.rs +++ b/examples/shader/shader_prepass.rs @@ -233,7 +233,7 @@ fn toggle_prepass_view( color.0 = Color::WHITE; }); - let mat = materials.get_mut(*material_handle).unwrap(); + let mat = materials.get_cloned_mut(*material_handle).unwrap(); mat.settings.show_depth = (*prepass_view == 1) as u32; mat.settings.show_normals = (*prepass_view == 2) as u32; mat.settings.show_motion_vectors = (*prepass_view == 3) as u32; diff --git a/examples/shader/storage_buffer.rs b/examples/shader/storage_buffer.rs index 4cb3275d7e0b4..c70466a9dff65 100644 --- a/examples/shader/storage_buffer.rs +++ b/examples/shader/storage_buffer.rs @@ -67,8 +67,8 @@ fn update( mut materials: ResMut>, mut buffers: ResMut>, ) { - let material = materials.get_mut(&material_handle.0).unwrap(); - let buffer = buffers.get_mut(&material.colors).unwrap(); + let material = materials.get_cloned_mut(&material_handle.0).unwrap(); + let buffer = buffers.get_cloned_mut(&material.colors).unwrap(); buffer.set_data( (0..5) .map(|i| { diff --git a/examples/tools/scene_viewer/scene_viewer_plugin.rs b/examples/tools/scene_viewer/scene_viewer_plugin.rs index 49f4805d06eb7..4a024b64aeeaa 100644 --- a/examples/tools/scene_viewer/scene_viewer_plugin.rs +++ b/examples/tools/scene_viewer/scene_viewer_plugin.rs @@ -113,7 +113,9 @@ fn scene_load_check( scene_handle.scene_index ) }); - let scene = scenes.get_mut(gltf_scene_handle).unwrap(); + let scene = scenes + .get_inplace_mut(gltf_scene_handle) + .expect("The asset is missing or is aliased."); let mut query = scene .world diff --git a/examples/ui/ui_material.rs b/examples/ui/ui_material.rs index 51752f255796b..f9ba845fa50f5 100644 --- a/examples/ui/ui_material.rs +++ b/examples/ui/ui_material.rs @@ -83,7 +83,7 @@ fn animate( ) { let duration = 2.0; for handle in &q { - if let Some(material) = materials.get_mut(handle) { + if let Some(material) = materials.get_cloned_mut(handle) { // rainbow color effect let new_color = Color::hsl((time.elapsed_secs() * 60.0) % 360.0, 1., 0.5); let border_color = Color::hsl((time.elapsed_secs() * 60.0) % 360.0, 0.75, 0.75); From 2314f796bc35a3092bdacca0d9cad8066862332c Mon Sep 17 00:00:00 2001 From: andriyDev Date: Thu, 26 Sep 2024 23:51:35 -0700 Subject: [PATCH 11/20] Create an example for the use of asset arcing. --- Cargo.toml | 11 ++ examples/README.md | 1 + examples/asset/arc_asset.rs | 251 ++++++++++++++++++++++++++++ examples/asset/files/heights.li.ron | 6 + 4 files changed, 269 insertions(+) create mode 100644 examples/asset/arc_asset.rs create mode 100644 examples/asset/files/heights.li.ron diff --git a/Cargo.toml b/Cargo.toml index 922a2176513a0..06869f45db957 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1581,6 +1581,17 @@ description = "Demonstrates various methods to load assets" category = "Assets" wasm = false +[[example]] +name = "arc_asset" +path = "examples/asset/arc_asset.rs" +doc-scrape-examples = true + +[package.metadata.example.arc_asset] +name = "Arced Assets" +description = "Demonstrates how to acquire Arc'd assets and use them in an async context" +category = "Assets" +wasm = false + [[example]] name = "asset_settings" path = "examples/asset/asset_settings.rs" diff --git a/examples/README.md b/examples/README.md index 5f51e2c38e660..bd29ce41a4fe7 100644 --- a/examples/README.md +++ b/examples/README.md @@ -233,6 +233,7 @@ Example | Description --- | --- [Alter Mesh](../examples/asset/alter_mesh.rs) | Shows how to modify the underlying asset of a Mesh after spawning. [Alter Sprite](../examples/asset/alter_sprite.rs) | Shows how to modify texture assets after spawning. +[Arced Assets](../examples/asset/arc_asset.rs) | Demonstrates how to acquire Arc'd assets and use them in an async context [Asset Decompression](../examples/asset/asset_decompression.rs) | Demonstrates loading a compressed asset [Asset Loading](../examples/asset/asset_loading.rs) | Demonstrates various methods to load assets [Asset Processing](../examples/asset/processing/asset_processing.rs) | Demonstrates how to process and load custom assets diff --git a/examples/asset/arc_asset.rs b/examples/asset/arc_asset.rs new file mode 100644 index 0000000000000..d19feb3aee3b6 --- /dev/null +++ b/examples/asset/arc_asset.rs @@ -0,0 +1,251 @@ +//! This example illustrates how to use assets in an async context (through locking). + +use std::sync::Arc; + +use bevy::{ + asset::AssetLoader, + color::palettes::tailwind, + math::FloatOrd, + prelude::*, + render::{mesh::Indices, render_asset::RenderAssetUsages}, + tasks::AsyncComputeTaskPool, +}; +use rand::{Rng, SeedableRng}; +use serde::{Deserialize, Serialize}; +use thiserror::Error; + +fn main() { + App::new() + .add_plugins( + // This just tells the asset server to look in the right examples folder + DefaultPlugins.set(AssetPlugin { + file_path: "examples/asset/files".to_string(), + ..Default::default() + }), + ) + .init_asset::() + .register_asset_loader(LinearInterpolationLoader) + .add_systems(Startup, setup) + .add_systems(Update, (start_mesh_generation, finish_mesh_generation)) + .run(); +} + +fn setup( + asset_server: Res, + mut materials: ResMut>, + meshes: Res>, + mut commands: Commands, +) { + // Spawn a camera. + commands.spawn(( + Transform::from_translation(Vec3::new(15.0, 15.0, 15.0)).looking_at(Vec3::ZERO, Vec3::Y), + Camera3d::default(), + )); + + // Spawn a light. + commands.spawn(( + Transform::default().looking_to(Dir3::from_xyz(1.0, -1.0, 0.0).unwrap(), Dir3::Y), + DirectionalLight::default(), + )); + + // Spawn the mesh. Reserve the handle so we can generate it later. + let mesh = meshes.reserve_handle(); + commands.spawn(( + Mesh3d(mesh.clone()), + MeshMaterial3d(materials.add(StandardMaterial { + base_color: tailwind::SLATE_100.into(), + ..Default::default() + })), + )); + + // Create the parameters for mesh generation. + commands.insert_resource(MeshGeneration { + height_interpolation: asset_server.load("heights.li.ron"), + mesh, + size: UVec2::new(30, 30), + }); + + // Create the channel we will communicate across. + let (sender, receiver) = crossbeam_channel::bounded(1); + commands.insert_resource(MeshGenerationChannel { sender, receiver }); +} + +#[derive(Resource)] +struct MeshGeneration { + height_interpolation: Handle, + mesh: Handle, + size: UVec2, +} + +#[derive(Resource)] +struct MeshGenerationChannel { + sender: crossbeam_channel::Sender, + receiver: crossbeam_channel::Receiver, +} + +/// Starts a mesh generation task whenever the height interpolation asset is updated. +fn start_mesh_generation( + mut asset_events: EventReader>, + linear_interpolations: Res>, + mesh_generation: Res, + channel: Res, +) { + // Only recompute if the height interpolation asset has changed. + let regenerate_id = mesh_generation.height_interpolation.id(); + let mut recompute = false; + for asset_event in asset_events.read() { + match asset_event { + AssetEvent::Added { id } | AssetEvent::Modified { id } if *id == regenerate_id => { + recompute = true; + } + _ => {} + } + } + + if !recompute { + return; + } + + let task_pool = AsyncComputeTaskPool::get(); + let size = mesh_generation.size; + // Get an `Arc` of the height interpolation asset to pass to the spawned task. + let height_interpolation = linear_interpolations + .get_arc(&mesh_generation.height_interpolation) + .expect("The asset is loaded"); + let channel = channel.sender.clone(); + // Spawn a task to generate the mesh, then send the resulting mesh across the channel. + task_pool + .spawn(async move { + let mesh = generate_mesh(size, height_interpolation); + channel.send(mesh).expect("The channel never closes"); + }) + .detach(); +} + +/// Reads from the mesh generation channel and inserts the mesh asset. +fn finish_mesh_generation( + mesh_generation: Res, + channel: Res, + mut meshes: ResMut>, +) { + let Ok(mesh) = channel.receiver.try_recv() else { + return; + }; + meshes.insert(&mesh_generation.mesh, mesh); +} + +/// A basic linear interpolation curve implementation. +#[derive(Asset, TypePath, Serialize, Deserialize)] +struct LinearInterpolation(Vec<(f32, f32)>); + +impl LinearInterpolation { + /// Samples the linear interpolation at `value`. + fn sample(&self, value: f32) -> f32 { + match self.0.iter().position(|(x, _)| value < *x) { + None => self.0.last().expect("The interpolation is non-empty").1, + Some(0) => self.0.first().expect("The interpolation is non-empty").1, + Some(next) => { + let previous = next - 1; + + let (next_x, next_y) = self.0[next]; + let (previous_x, previous_y) = self.0[previous]; + + let alpha = (value - previous_x) / (next_x - previous_x); + + alpha * (next_y - previous_y) + previous_y + } + } + } +} + +#[derive(Default)] +struct LinearInterpolationLoader; + +#[derive(Debug, Error)] +enum LinearInterpolationLoaderError { + #[error(transparent)] + Io(#[from] std::io::Error), + #[error(transparent)] + RonSpannedError(#[from] ron::error::SpannedError), + #[error("The loaded interpolation is empty.")] + Empty, + #[error("The loaded interpolation contains duplicate X values")] + DuplicateXValues, +} + +impl AssetLoader for LinearInterpolationLoader { + type Asset = LinearInterpolation; + type Settings = (); + type Error = LinearInterpolationLoaderError; + + async fn load( + &self, + reader: &mut dyn bevy::asset::io::Reader, + _settings: &Self::Settings, + _load_context: &mut bevy::asset::LoadContext<'_>, + ) -> Result { + let mut bytes = Vec::new(); + reader.read_to_end(&mut bytes).await?; + let mut interpolation: LinearInterpolation = ron::de::from_bytes(&bytes)?; + if interpolation.0.is_empty() { + return Err(Self::Error::Empty); + } + interpolation.0.sort_by_key(|(key, _)| FloatOrd(*key)); + if interpolation + .0 + .windows(2) + .any(|window| window[0].0 == window[1].0) + { + return Err(Self::Error::DuplicateXValues); + } + Ok(interpolation) + } + + fn extensions(&self) -> &[&str] { + &["li.ron"] + } +} + +/// Generates the mesh given the interpolation curve and the size of the mesh. +fn generate_mesh(size: UVec2, interpolation: Arc) -> Mesh { + let mut rng = rand_chacha::ChaChaRng::seed_from_u64(12345); + + let center = Vec3::new((size.x as f32) / 2.0, 0.0, (size.y as f32) / -2.0); + + let mut vertices = Vec::with_capacity(((size.x + 1) * (size.y + 1)) as usize); + let mut uvs = Vec::with_capacity(((size.x + 1) * (size.y + 1)) as usize); + for y in 0..size.y + 1 { + for x in 0..size.x + 1 { + let height = interpolation.sample(rng.gen()); + vertices.push(Vec3::new(x as f32, height, -(y as f32)) - center); + uvs.push(Vec2::new(x as f32, -(y as f32))); + } + } + + let y_stride = size.x + 1; + let mut indices = Vec::with_capacity((size.x * size.y * 6) as usize); + for y in 0..size.y { + for x in 0..size.x { + indices.push(x + y * y_stride); + indices.push(x + 1 + y * y_stride); + indices.push(x + 1 + (y + 1) * y_stride); + indices.push(x + y * y_stride); + indices.push(x + 1 + (y + 1) * y_stride); + indices.push(x + (y + 1) * y_stride); + } + } + + let mut mesh = Mesh::new( + bevy_render::mesh::PrimitiveTopology::TriangleList, + RenderAssetUsages::RENDER_WORLD, + ) + .with_inserted_attribute(Mesh::ATTRIBUTE_POSITION, vertices) + .with_inserted_attribute(Mesh::ATTRIBUTE_UV_0, uvs) + .with_inserted_indices(Indices::U32(indices)); + + mesh.compute_normals(); + mesh.generate_tangents() + .expect("The tangents are well formed"); + + mesh +} diff --git a/examples/asset/files/heights.li.ron b/examples/asset/files/heights.li.ron new file mode 100644 index 0000000000000..8d4e03f71a21e --- /dev/null +++ b/examples/asset/files/heights.li.ron @@ -0,0 +1,6 @@ +([ + (0.0, 0.0), + (0.5, 0.1), + (0.7, 1.0), + (1.0, 2.0), +]) \ No newline at end of file From 1ee7217327aafe690c258a85e095825856278fa4 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Wed, 9 Oct 2024 23:40:23 -0700 Subject: [PATCH 12/20] Add a nice big warning on `get_arc` about holding the `Arc` for long periods of time. --- crates/bevy_asset/src/assets.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 53af5be5acbef..1a8d64dfadaba 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -427,7 +427,14 @@ impl Assets { } /// Retrieves the [`Arc`] of an [`Asset`] with the given `id`, if it exists. - /// Note that this supports anything that implements `Into>`, which includes [`Handle`] and [`AssetId`]. + /// + /// Note that this supports anything that implements `Into>`, which includes + /// [`Handle`] and [`AssetId`]. Be careful with holding the Arc indefinitely: holding the + /// [`Arc`] (or a [`Weak`]) prevents the asset from being mutated in place. This can incur + /// clones when using `get_cloned_mut`, or can just entirely block mutation when using + /// `get_inplace_mut`. + /// + /// [`Weak`]: std::sync::Weak #[inline] pub fn get_arc(&self, id: impl Into>) -> Option> { match id.into() { From 961fc147bc5af22b2e7bc0f8a386724fee269afc Mon Sep 17 00:00:00 2001 From: andriyDev Date: Fri, 1 Nov 2024 12:18:31 -0700 Subject: [PATCH 13/20] Rewrite any "Arc-ed" to "Arc-d" to make it consistent with other uses. --- Cargo.toml | 2 +- crates/bevy_asset/src/assets.rs | 6 +++--- crates/bevy_asset/src/direct_access_ext.rs | 4 ++-- examples/README.md | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 06869f45db957..57629f70da439 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1587,7 +1587,7 @@ path = "examples/asset/arc_asset.rs" doc-scrape-examples = true [package.metadata.example.arc_asset] -name = "Arced Assets" +name = "Arc'd Assets" description = "Demonstrates how to acquire Arc'd assets and use them in an async context" category = "Assets" wasm = false diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 1a8d64dfadaba..dbdc8874a068e 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -331,8 +331,8 @@ impl Assets { self.insert_arc(id, asset); } - /// Inserts the given [`Arc`]-ed `asset`, identified by the given `id`. If an asset already - /// exists for `id`, it will be replaced. + /// Inserts the given [`Arc`]d `asset`, identified by the given `id`. If an asset already exists + /// for `id`, it will be replaced. pub fn insert_arc(&mut self, id: impl Into>, asset: impl Into>) { let asset = asset.into(); match id.into() { @@ -386,7 +386,7 @@ impl Assets { self.add_arc(asset.into()) } - /// Adds the given [`Arc`]-ed `asset` and allocates a new strong [`Handle`] for it. + /// Adds the given [`Arc`]d `asset` and allocates a new strong [`Handle`] for it. #[inline] pub fn add_arc(&mut self, asset: impl Into>) -> Handle { let index = self.dense_storage.allocator.reserve(); diff --git a/crates/bevy_asset/src/direct_access_ext.rs b/crates/bevy_asset/src/direct_access_ext.rs index e164987ace894..6ce9e995c7b21 100644 --- a/crates/bevy_asset/src/direct_access_ext.rs +++ b/crates/bevy_asset/src/direct_access_ext.rs @@ -11,7 +11,7 @@ pub trait DirectAssetAccessExt { /// Insert an asset similarly to [`Assets::add`]. fn add_asset(&mut self, asset: impl Into) -> Handle; - /// Insert an [`Arc`]ed asset similarly to [`Assets::add_arc`]. + /// Insert an [`Arc`]d asset similarly to [`Assets::add_arc`]. fn add_arc_asset(&mut self, asset: impl Into>) -> Handle; /// Load an asset similarly to [`AssetServer::load`]. @@ -33,7 +33,7 @@ impl DirectAssetAccessExt for World { self.resource_mut::>().add(asset) } - /// Insert an [`Arc`]ed asset similarly to [`Assets::add_arc`]. + /// Insert an [`Arc`]d asset similarly to [`Assets::add_arc`]. /// /// # Panics /// If `self` doesn't have an [`AssetServer`] resource initialized yet. diff --git a/examples/README.md b/examples/README.md index bd29ce41a4fe7..8121f7467dcb8 100644 --- a/examples/README.md +++ b/examples/README.md @@ -233,7 +233,7 @@ Example | Description --- | --- [Alter Mesh](../examples/asset/alter_mesh.rs) | Shows how to modify the underlying asset of a Mesh after spawning. [Alter Sprite](../examples/asset/alter_sprite.rs) | Shows how to modify texture assets after spawning. -[Arced Assets](../examples/asset/arc_asset.rs) | Demonstrates how to acquire Arc'd assets and use them in an async context +[Arc'd Assets](../examples/asset/arc_asset.rs) | Demonstrates how to acquire Arc'd assets and use them in an async context [Asset Decompression](../examples/asset/asset_decompression.rs) | Demonstrates loading a compressed asset [Asset Loading](../examples/asset/asset_loading.rs) | Demonstrates various methods to load assets [Asset Processing](../examples/asset/processing/asset_processing.rs) | Demonstrates how to process and load custom assets From a26d49ff5aae83e9aba162e567f1112fa96af301 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Fri, 1 Nov 2024 12:24:36 -0700 Subject: [PATCH 14/20] Improve the MutableAssetError messages to match Rust guidelines. https://rust-lang.github.io/api-guidelines/interoperability.html?highlight=error#error-types-are-meaningful-and-well-behaved-c-good-err --- crates/bevy_asset/src/assets.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index dbdc8874a068e..f3dfe278bc5a3 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -728,9 +728,9 @@ pub struct InvalidGenerationError { #[derive(Error, Display, Debug)] pub enum MutableAssetError { - #[display("The asset is not present or has an invalid generation.")] + #[display("asset is not present or has an invalid generation")] Missing, - #[display("The asset Arc is aliased (there is another Arc or Weak to this asset), so it is not safe to mutate.")] + #[display("asset `Arc` is aliased (there is another `Arc` or `Weak` to this asset), so it is not safe to mutate")] Aliased, } From 1f9d313cb8f7ffed141a3bb1af34deb502642b4d Mon Sep 17 00:00:00 2001 From: andriyDev Date: Fri, 1 Nov 2024 12:27:51 -0700 Subject: [PATCH 15/20] Rename `get_inplace_mut` to `get_in_place_mut`. --- crates/bevy_asset/src/assets.rs | 4 ++-- crates/bevy_asset/src/lib.rs | 2 +- examples/tools/scene_viewer/scene_viewer_plugin.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index f3dfe278bc5a3..43deb7d8956e5 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -432,7 +432,7 @@ impl Assets { /// [`Handle`] and [`AssetId`]. Be careful with holding the Arc indefinitely: holding the /// [`Arc`] (or a [`Weak`]) prevents the asset from being mutated in place. This can incur /// clones when using `get_cloned_mut`, or can just entirely block mutation when using - /// `get_inplace_mut`. + /// `get_in_place_mut`. /// /// [`Weak`]: std::sync::Weak #[inline] @@ -449,7 +449,7 @@ impl Assets { /// returns an error. /// /// [`Weak`]: std::sync::Weak - pub fn get_inplace_mut( + pub fn get_in_place_mut( &mut self, id: impl Into>, ) -> Result<&mut A, MutableAssetError> { diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index d90a8fbb7dd1e..f74901609b45f 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -1083,7 +1083,7 @@ mod tests { { let mut texts = app.world_mut().resource_mut::>(); - let a = texts.get_inplace_mut(a_id).unwrap(); + let a = texts.get_in_place_mut(a_id).unwrap(); a.text = "Changed".to_string(); } diff --git a/examples/tools/scene_viewer/scene_viewer_plugin.rs b/examples/tools/scene_viewer/scene_viewer_plugin.rs index 4a024b64aeeaa..695d7586495d5 100644 --- a/examples/tools/scene_viewer/scene_viewer_plugin.rs +++ b/examples/tools/scene_viewer/scene_viewer_plugin.rs @@ -114,7 +114,7 @@ fn scene_load_check( ) }); let scene = scenes - .get_inplace_mut(gltf_scene_handle) + .get_in_place_mut(gltf_scene_handle) .expect("The asset is missing or is aliased."); let mut query = scene From 5611e11f38c1fa1b89ff799137ce19cca1249440 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Fri, 1 Nov 2024 12:29:36 -0700 Subject: [PATCH 16/20] Fix the example using bad wording. --- examples/asset/arc_asset.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/asset/arc_asset.rs b/examples/asset/arc_asset.rs index d19feb3aee3b6..1ce360c664c73 100644 --- a/examples/asset/arc_asset.rs +++ b/examples/asset/arc_asset.rs @@ -1,4 +1,5 @@ -//! This example illustrates how to use assets in an async context (through locking). +//! This example illustrates how to use assets in an async context (through cloning the underlying +//! `Arc`). use std::sync::Arc; From d51a0b23526eb343cd4576b0343afe68e549b2ee Mon Sep 17 00:00:00 2001 From: andriyDev Date: Tue, 10 Dec 2024 22:32:48 -0800 Subject: [PATCH 17/20] Switch MutableAssetError to use thiserror instead of derive_more. --- crates/bevy_asset/src/assets.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 43deb7d8956e5..10a3119bce503 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -726,11 +726,11 @@ pub struct InvalidGenerationError { current_generation: u32, } -#[derive(Error, Display, Debug)] +#[derive(Error, Debug)] pub enum MutableAssetError { - #[display("asset is not present or has an invalid generation")] + #[error("asset is not present or has an invalid generation")] Missing, - #[display("asset `Arc` is aliased (there is another `Arc` or `Weak` to this asset), so it is not safe to mutate")] + #[error("asset `Arc` is aliased (there is another `Arc` or `Weak` to this asset), so it is not safe to mutate")] Aliased, } From e40384e5fbda3e15f0d2fb1ca03109a4b5287ce6 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Sun, 29 Dec 2024 13:16:12 -0800 Subject: [PATCH 18/20] Fix the `asset_changed` test by using `get_inplace_mut`. --- crates/bevy_asset/src/asset_changed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/asset_changed.rs b/crates/bevy_asset/src/asset_changed.rs index 9aafc157f7f68..fc8e1cf39f61c 100644 --- a/crates/bevy_asset/src/asset_changed.rs +++ b/crates/bevy_asset/src/asset_changed.rs @@ -362,7 +362,7 @@ mod tests { .iter() .find_map(|(h, a)| (a.0 == i).then_some(h)) .unwrap(); - let asset = assets.get_mut(id).unwrap(); + let asset = assets.get_in_place_mut(id).unwrap(); println!("setting new value for {}", asset.0); asset.1 = "new_value"; }; From 2ed2094d534685b8f2305341166c3d70f7785d06 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Sun, 29 Dec 2024 13:20:04 -0800 Subject: [PATCH 19/20] Make the docs for `AssetChanged` reference `get_cloned_mut` instead of the old `get_mut`. --- crates/bevy_asset/src/asset_changed.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_asset/src/asset_changed.rs b/crates/bevy_asset/src/asset_changed.rs index fc8e1cf39f61c..6db8dee5626ed 100644 --- a/crates/bevy_asset/src/asset_changed.rs +++ b/crates/bevy_asset/src/asset_changed.rs @@ -92,9 +92,9 @@ impl<'w, A: AsAssetId> AssetChangeCheck<'w, A> { /// /// Unlike `Changed`, this is true whenever the asset for the `A` /// in `ResMut>` changed. For example, when a mesh changed through the -/// [`Assets::get_mut`] method, `AssetChanged` will iterate over all -/// entities with the `Handle` for that mesh. Meanwhile, `Changed>` -/// will iterate over no entities. +/// [`Assets::get_cloned_mut`] method, `AssetChanged` will iterate +/// over all entities with the `Handle` for that mesh. Meanwhile, +/// `Changed>` will iterate over no entities. /// /// Swapping the actual `A` component is a common pattern. So you /// should check for _both_ `AssetChanged` and `Changed` with @@ -121,7 +121,7 @@ impl<'w, A: AsAssetId> AssetChangeCheck<'w, A> { /// If no `A` asset updated since the last time the system ran, then no lookups occur. /// /// [`AssetEvents`]: crate::AssetEvents -/// [`Assets::get_mut`]: crate::Assets::get_mut +/// [`Assets::get_cloned_mut`]: crate::Assets::get_cloned_mut pub struct AssetChanged(PhantomData); /// [`WorldQuery`] fetch for [`AssetChanged`]. From 9289a0790639c744a2c231a66e18d01ec1435c29 Mon Sep 17 00:00:00 2001 From: andriyDev Date: Sun, 29 Dec 2024 13:28:31 -0800 Subject: [PATCH 20/20] Fix the `mixed_lighting` example to use `get_cloned_mut`. --- examples/3d/mixed_lighting.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/3d/mixed_lighting.rs b/examples/3d/mixed_lighting.rs index 88b2459f8e61c..39bea16a8e994 100644 --- a/examples/3d/mixed_lighting.rs +++ b/examples/3d/mixed_lighting.rs @@ -257,7 +257,7 @@ fn update_lightmaps( } // Lightmap exposure defaults to zero, so we need to set it. - if let Some(ref mut material) = materials.get_mut(material) { + if let Some(ref mut material) = materials.get_cloned_mut(material) { material.lightmap_exposure = LIGHTMAP_EXPOSURE; } @@ -279,7 +279,7 @@ fn update_lightmaps( // Add lightmaps to or remove lightmaps from the sphere. if &**name == "Sphere" { // Lightmap exposure defaults to zero, so we need to set it. - if let Some(ref mut material) = materials.get_mut(material) { + if let Some(ref mut material) = materials.get_cloned_mut(material) { material.lightmap_exposure = LIGHTMAP_EXPOSURE; }