From 4e66032556229c6025e56245fa15650dbfb1a5d9 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Thu, 8 May 2025 17:12:17 +0100 Subject: [PATCH 01/32] Changed `MetaTransform` to be a `Box` that will optionally override the settings in `AssetMeta`. --- crates/bevy_asset/src/loader.rs | 2 +- crates/bevy_asset/src/loader_builders.rs | 15 +++--- crates/bevy_asset/src/meta.rs | 58 +++++++++------------ crates/bevy_asset/src/server/mod.rs | 46 +++++----------- crates/bevy_gltf/src/loader/mod.rs | 9 ++-- crates/bevy_image/src/hdr_texture_loader.rs | 2 +- examples/3d/ssr.rs | 9 ++-- 7 files changed, 59 insertions(+), 82 deletions(-) diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 8f4863b885c68..841c51daa8a05 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -32,7 +32,7 @@ pub trait AssetLoader: Send + Sync + 'static { /// The top level [`Asset`] loaded by this [`AssetLoader`]. type Asset: Asset; /// The settings type used by this [`AssetLoader`]. - type Settings: Settings + Default + Serialize + for<'a> Deserialize<'a>; + type Settings: Settings + Default + Serialize + for<'a> Deserialize<'a> + Clone; /// The type of [error](`std::error::Error`) which could be encountered by this loader. type Error: Into>; /// Asynchronously loads [`AssetLoader::Asset`] (and any other labeled assets) from the bytes provided by [`Reader`]. diff --git a/crates/bevy_asset/src/loader_builders.rs b/crates/bevy_asset/src/loader_builders.rs index 13bea2b71dc2a..de9e4d525ed83 100644 --- a/crates/bevy_asset/src/loader_builders.rs +++ b/crates/bevy_asset/src/loader_builders.rs @@ -3,7 +3,7 @@ use crate::{ io::Reader, - meta::{meta_transform_settings, AssetMetaDyn, MetaTransform, Settings}, + meta::{MetaTransform, Settings}, Asset, AssetLoadError, AssetPath, ErasedAssetLoader, ErasedLoadedAsset, Handle, LoadContext, LoadDirectError, LoadedAsset, LoadedUntypedAsset, UntypedHandle, }; @@ -176,6 +176,7 @@ impl<'ctx, 'builder> NestedLoader<'ctx, 'builder, StaticTyped, Deferred> { } impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'builder, T, M> { + /* fn with_transform( mut self, transform: impl Fn(&mut dyn AssetMetaDyn) + Send + Sync + 'static, @@ -190,17 +191,16 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui } self } + */ /// Configure the settings used to load the asset. /// /// If the settings type `S` does not match the settings expected by `A`'s asset loader, an error will be printed to the log /// and the asset load will fail. #[must_use] - pub fn with_settings( - self, - settings: impl Fn(&mut S) + Send + Sync + 'static, - ) -> Self { - self.with_transform(move |meta| meta_transform_settings(meta, &settings)) + pub fn with_settings(mut self, settings: S) -> Self { + self.meta_transform = Some(Box::new(settings)); + self } // convert between `T`s @@ -429,7 +429,8 @@ impl<'builder, 'reader, T> NestedLoader<'_, '_, T, Immediate<'builder, 'reader>> }; if let Some(meta_transform) = self.meta_transform { - meta_transform(&mut *meta); + // XXX TODO: Does this need the same deref hack as `load_internal`? + meta.apply_settings(&meta_transform); } let asset = self diff --git a/crates/bevy_asset/src/meta.rs b/crates/bevy_asset/src/meta.rs index 0e972261198cc..74089679169ef 100644 --- a/crates/bevy_asset/src/meta.rs +++ b/crates/bevy_asset/src/meta.rs @@ -11,10 +11,9 @@ use crate::{ use downcast_rs::{impl_downcast, Downcast}; use ron::ser::PrettyConfig; use serde::{Deserialize, Serialize}; -use tracing::error; pub const META_FORMAT_VERSION: &str = "1.0"; -pub type MetaTransform = Box; +pub type MetaTransform = Box; /// Asset metadata that informs how an [`Asset`] should be handled by the asset system. /// @@ -125,8 +124,8 @@ pub struct ProcessedInfoMinimal { pub trait AssetMetaDyn: Downcast + Send + Sync { /// Returns a reference to the [`AssetLoader`] settings, if they exist. fn loader_settings(&self) -> Option<&dyn Settings>; - /// Returns a mutable reference to the [`AssetLoader`] settings, if they exist. - fn loader_settings_mut(&mut self) -> Option<&mut dyn Settings>; + /// XXX TODO. + fn apply_settings(&mut self, settings: &dyn Settings); /// Serializes the internal [`AssetMeta`]. fn serialize(&self) -> Vec; /// Returns a reference to the [`ProcessedInfo`] if it exists. @@ -143,11 +142,18 @@ impl AssetMetaDyn for AssetMeta { None } } - fn loader_settings_mut(&mut self) -> Option<&mut dyn Settings> { + fn apply_settings(&mut self, new_settings: &dyn Settings) { if let AssetAction::Load { settings, .. } = &mut self.asset { - Some(settings) - } else { - None + if let Some(new_settings) = new_settings.downcast_ref::<::Settings>() + { + *settings = new_settings.clone(); + } else { + tracing::error!( + "Configured settings type {} does not match AssetLoader settings type {}", + new_settings.debug_type_name(), + core::any::type_name::<::Settings>(), + ); + } } } fn serialize(&self) -> Vec { @@ -168,9 +174,19 @@ impl_downcast!(AssetMetaDyn); /// Settings used by the asset system, such as by [`AssetLoader`], [`Process`], and [`AssetSaver`] /// /// [`AssetSaver`]: crate::saver::AssetSaver -pub trait Settings: Downcast + Send + Sync + 'static {} +pub trait Settings: Downcast + Send + Sync + 'static { + // XXX TODO: Better solution. Currently used by apply_settings to print error. + fn debug_type_name(&self) -> &'static str; +} -impl Settings for T where T: Send + Sync {} +impl Settings for T +where + T: Send + Sync, +{ + fn debug_type_name(&self) -> &'static str { + core::any::type_name::() + } +} impl_downcast!(Settings); @@ -216,28 +232,6 @@ impl AssetLoader for () { } } -pub(crate) fn meta_transform_settings( - meta: &mut dyn AssetMetaDyn, - settings: &(impl Fn(&mut S) + Send + Sync + 'static), -) { - if let Some(loader_settings) = meta.loader_settings_mut() { - if let Some(loader_settings) = loader_settings.downcast_mut::() { - settings(loader_settings); - } else { - error!( - "Configured settings type {} does not match AssetLoader settings type", - core::any::type_name::(), - ); - } - } -} - -pub(crate) fn loader_settings_meta_transform( - settings: impl Fn(&mut S) + Send + Sync + 'static, -) -> MetaTransform { - Box::new(move |meta| meta_transform_settings(meta, &settings)) -} - pub type AssetHash = [u8; 32]; /// NOTE: changing the hashing logic here is a _breaking change_ that requires a [`META_FORMAT_VERSION`] bump. diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index ff5800474d8b7..2920ad50861ce 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -9,10 +9,7 @@ use crate::{ MissingProcessedAssetReaderError, Reader, }, loader::{AssetLoader, ErasedAssetLoader, LoadContext, LoadedAsset}, - meta::{ - loader_settings_meta_transform, AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal, - MetaTransform, Settings, - }, + meta::{AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal, MetaTransform, Settings}, path::AssetPath, Asset, AssetEvent, AssetHandleProvider, AssetId, AssetLoadFailedEvent, AssetMetaCheck, Assets, DeserializeMetaError, ErasedLoadedAsset, Handle, LoadedUntypedAsset, UnapprovedPathMode, @@ -378,14 +375,9 @@ impl AssetServer { pub fn load_with_settings<'a, A: Asset, S: Settings>( &self, path: impl Into>, - settings: impl Fn(&mut S) + Send + Sync + 'static, + settings: S, ) -> Handle { - self.load_with_meta_transform( - path, - Some(loader_settings_meta_transform(settings)), - (), - false, - ) + self.load_with_meta_transform(path, Some(Box::new(settings)), (), false) } /// Same as [`load`](AssetServer::load_with_settings), but you can load assets from unaproved paths @@ -396,14 +388,9 @@ impl AssetServer { pub fn load_with_settings_override<'a, A: Asset, S: Settings>( &self, path: impl Into>, - settings: impl Fn(&mut S) + Send + Sync + 'static, + settings: S, ) -> Handle { - self.load_with_meta_transform( - path, - Some(loader_settings_meta_transform(settings)), - (), - true, - ) + self.load_with_meta_transform(path, Some(Box::new(settings)), (), true) } /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item. @@ -419,15 +406,10 @@ impl AssetServer { pub fn load_acquire_with_settings<'a, A: Asset, S: Settings, G: Send + Sync + 'static>( &self, path: impl Into>, - settings: impl Fn(&mut S) + Send + Sync + 'static, + settings: S, guard: G, ) -> Handle { - self.load_with_meta_transform( - path, - Some(loader_settings_meta_transform(settings)), - guard, - false, - ) + self.load_with_meta_transform(path, Some(Box::new(settings)), guard, false) } /// Same as [`load`](AssetServer::load_acquire_with_settings), but you can load assets from unaproved paths @@ -443,15 +425,10 @@ impl AssetServer { >( &self, path: impl Into>, - settings: impl Fn(&mut S) + Send + Sync + 'static, + settings: S, guard: G, ) -> Handle { - self.load_with_meta_transform( - path, - Some(loader_settings_meta_transform(settings)), - guard, - true, - ) + self.load_with_meta_transform(path, Some(Box::new(settings)), guard, true) } pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>( @@ -672,7 +649,10 @@ impl AssetServer { })?; if let Some(meta_transform) = input_handle.as_ref().and_then(|h| h.meta_transform()) { - (*meta_transform)(&mut *meta); + // Deref to avoid `apply_settings` downcast failing because the type + // is `Box` when it expected `T`. XXX TODO: Better solution. + let meta_transform = core::ops::Deref::deref(meta_transform); + meta.apply_settings(meta_transform); } // downgrade the input handle so we don't keep the asset alive just because we're loading it // note we can't just pass a weak handle in, as only strong handles contain the asset meta transform diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index f85a739b2e01f..8e0c763ece464 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -169,7 +169,7 @@ pub struct GltfLoader { /// } /// ); /// ``` -#[derive(Serialize, Deserialize)] +#[derive(Serialize, Deserialize, Clone)] pub struct GltfLoaderSettings { /// If empty, the gltf mesh nodes will be skipped. /// @@ -1700,9 +1700,10 @@ impl ImageOrPath { sampler_descriptor, } => load_context .loader() - .with_settings(move |settings: &mut ImageLoaderSettings| { - settings.is_srgb = is_srgb; - settings.sampler = ImageSampler::Descriptor(sampler_descriptor.clone()); + .with_settings(ImageLoaderSettings { + is_srgb, + sampler: ImageSampler::Descriptor(sampler_descriptor.clone()), + ..Default::default() }) .load(path), }; diff --git a/crates/bevy_image/src/hdr_texture_loader.rs b/crates/bevy_image/src/hdr_texture_loader.rs index 3177e94865809..d21917fdb048a 100644 --- a/crates/bevy_image/src/hdr_texture_loader.rs +++ b/crates/bevy_image/src/hdr_texture_loader.rs @@ -10,7 +10,7 @@ use wgpu_types::{Extent3d, TextureDimension, TextureFormat}; #[derive(Clone, Default)] pub struct HdrTextureLoader; -#[derive(Serialize, Deserialize, Default, Debug)] +#[derive(Serialize, Deserialize, Default, Debug, Clone)] pub struct HdrTextureLoaderSettings { pub asset_usage: RenderAssetUsages, } diff --git a/examples/3d/ssr.rs b/examples/3d/ssr.rs index a4d2c1b742c0f..61148b2c5d126 100644 --- a/examples/3d/ssr.rs +++ b/examples/3d/ssr.rs @@ -190,15 +190,16 @@ fn spawn_water( extension: Water { normals: asset_server.load_with_settings::( "textures/water_normals.png", - |settings| { - settings.is_srgb = false; - settings.sampler = ImageSampler::Descriptor(ImageSamplerDescriptor { + ImageLoaderSettings { + is_srgb: false, + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: ImageAddressMode::Repeat, address_mode_v: ImageAddressMode::Repeat, mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, ..default() - }); + }), + ..Default::default() }, ), // These water settings are just random values to create some From 14a2a36330a66d472cc154a2a795b44396c006e1 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Thu, 8 May 2025 21:56:23 +0100 Subject: [PATCH 02/32] Trying a different tack - add the settings to `AssetPath`. --- crates/bevy_asset/src/direct_access_ext.rs | 8 +-- crates/bevy_asset/src/loader_builders.rs | 4 +- crates/bevy_asset/src/path.rs | 76 +++++++++++++++++++++- crates/bevy_asset/src/server/mod.rs | 27 ++++---- 4 files changed, 96 insertions(+), 19 deletions(-) diff --git a/crates/bevy_asset/src/direct_access_ext.rs b/crates/bevy_asset/src/direct_access_ext.rs index 792d523a30063..7d77538063e99 100644 --- a/crates/bevy_asset/src/direct_access_ext.rs +++ b/crates/bevy_asset/src/direct_access_ext.rs @@ -14,10 +14,10 @@ pub trait DirectAssetAccessExt { fn load_asset<'a, A: Asset>(&self, path: impl Into>) -> Handle; /// Load an asset with settings, similarly to [`AssetServer::load_with_settings`]. - fn load_asset_with_settings<'a, A: Asset, S: Settings>( + fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( &self, path: impl Into>, - settings: impl Fn(&mut S) + Send + Sync + 'static, + settings: S, ) -> Handle; } impl DirectAssetAccessExt for World { @@ -40,10 +40,10 @@ impl DirectAssetAccessExt for World { /// /// # Panics /// If `self` doesn't have an [`AssetServer`] resource initialized yet. - fn load_asset_with_settings<'a, A: Asset, S: Settings>( + fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( &self, path: impl Into>, - settings: impl Fn(&mut S) + Send + Sync + 'static, + settings: S, ) -> Handle { self.resource::() .load_with_settings(path, settings) diff --git a/crates/bevy_asset/src/loader_builders.rs b/crates/bevy_asset/src/loader_builders.rs index de9e4d525ed83..dd22b971760e7 100644 --- a/crates/bevy_asset/src/loader_builders.rs +++ b/crates/bevy_asset/src/loader_builders.rs @@ -428,9 +428,9 @@ impl<'builder, 'reader, T> NestedLoader<'_, '_, T, Immediate<'builder, 'reader>> (meta, loader, ReaderRef::Boxed(reader)) }; - if let Some(meta_transform) = self.meta_transform { + if let Some(settings) = path.settings() { // XXX TODO: Does this need the same deref hack as `load_internal`? - meta.apply_settings(&meta_transform); + meta.apply_settings(settings); } let asset = self diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index ad127812dcfcf..da17b202a98f9 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -1,19 +1,60 @@ -use crate::io::AssetSourceId; +use crate::{io::AssetSourceId, meta::Settings}; use alloc::{ borrow::ToOwned, + boxed::Box, string::{String, ToString}, + sync::Arc, }; use atomicow::CowArc; +use bevy_platform::hash::FixedHasher; use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize}; use core::{ fmt::{Debug, Display}, - hash::Hash, + hash::{BuildHasher, Hash, Hasher}, ops::Deref, }; use serde::{de::Visitor, Deserialize, Serialize}; use std::path::{Path, PathBuf}; use thiserror::Error; +#[derive(Eq, PartialEq, Hash, Copy, Clone)] +pub struct AssetPathSettingsId(u64); + +pub struct AssetPathSettings { + pub value: Box, + pub id: AssetPathSettingsId, +} + +impl AssetPathSettings { + pub fn new(settings: S) -> AssetPathSettings { + // XXX TODO: Serializing to string is probably not the right solution. + let string = ron::ser::to_string(&settings).unwrap(); // XXX TODO: Avoid unwrap. + + // XXX TODO: What's the appropriate hasher? + // XXX TODO: Should we hash the type id as well? + let id = AssetPathSettingsId(FixedHasher.hash_one(&string)); + + AssetPathSettings { + value: Box::new(settings), + id, + } + } +} + +impl Hash for AssetPathSettings { + fn hash(&self, state: &mut H) { + self.id.hash(state); + } +} + +impl PartialEq for AssetPathSettings { + fn eq(&self, other: &Self) -> bool { + self.id == other.id + } +} + +impl Eq for AssetPathSettings {} + /// Represents a path to an asset in a "virtual filesystem". /// /// Asset paths consist of three main parts: @@ -58,6 +99,7 @@ pub struct AssetPath<'a> { source: AssetSourceId<'a>, path: CowArc<'a, Path>, label: Option>, + settings: Option>, } impl<'a> Debug for AssetPath<'a> { @@ -131,6 +173,7 @@ impl<'a> AssetPath<'a> { }, path: CowArc::Borrowed(path), label: label.map(CowArc::Borrowed), + settings: None, // XXX TODO? }) } @@ -230,6 +273,7 @@ impl<'a> AssetPath<'a> { path: CowArc::Borrowed(path), source: AssetSourceId::Default, label: None, + settings: None, // XXX TODO? } } @@ -258,6 +302,12 @@ impl<'a> AssetPath<'a> { self.path.deref() } + /// XXX TODO: Docs. + #[inline] + pub fn settings(&self) -> Option<&AssetPathSettings> { + self.settings.as_deref() + } + /// Gets the path to the asset in the "virtual filesystem" without a label (if a label is currently set). #[inline] pub fn without_label(&self) -> AssetPath<'_> { @@ -265,6 +315,7 @@ impl<'a> AssetPath<'a> { source: self.source.clone(), path: self.path.clone(), label: None, + settings: self.settings.clone(), } } @@ -288,6 +339,7 @@ impl<'a> AssetPath<'a> { source: self.source, path: self.path, label: Some(label.into()), + settings: self.settings, } } @@ -299,6 +351,17 @@ impl<'a> AssetPath<'a> { source: source.into(), path: self.path, label: self.label, + settings: self.settings, + } + } + + #[inline] + pub fn with_settings(self, settings: S) -> AssetPath<'a> { + AssetPath { + source: self.source, + path: self.path, + label: self.label, + settings: Some(Arc::new(AssetPathSettings::new(settings))), } } @@ -313,6 +376,7 @@ impl<'a> AssetPath<'a> { source: self.source.clone(), label: None, path, + settings: self.settings.clone(), // XXX TODO: Bit weird? }) } @@ -326,6 +390,7 @@ impl<'a> AssetPath<'a> { source: self.source.into_owned(), path: self.path.into_owned(), label: self.label.map(CowArc::into_owned), + settings: self.settings.clone(), // XXX TODO? } } @@ -447,6 +512,7 @@ impl<'a> AssetPath<'a> { }, path: CowArc::Owned(result_path.into()), label: rlabel.map(|l| CowArc::Owned(l.into())), + settings: self.settings.clone(), // XXX TODO? }) } } @@ -533,16 +599,19 @@ impl AssetPath<'static> { source, path, label, + settings, } = self; let source = source.as_static(); let path = path.as_static(); let label = label.map(CowArc::as_static); + let settings = settings.clone(); Self { source, path, label, + settings, } } @@ -562,6 +631,7 @@ impl<'a> From<&'a str> for AssetPath<'a> { source: source.into(), path: CowArc::Borrowed(path), label: label.map(CowArc::Borrowed), + settings: None, } } } @@ -587,6 +657,7 @@ impl<'a> From<&'a Path> for AssetPath<'a> { source: AssetSourceId::Default, path: CowArc::Borrowed(path), label: None, + settings: None, } } } @@ -598,6 +669,7 @@ impl From for AssetPath<'static> { source: AssetSourceId::Default, path: path.into(), label: None, + settings: None, } } } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 2920ad50861ce..e8acf8b78208c 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -372,12 +372,12 @@ impl AssetServer { /// [`AssetLoader`] settings. The type `S` _must_ match the configured [`AssetLoader::Settings`] or `settings` changes /// will be ignored and an error will be printed to the log. #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] - pub fn load_with_settings<'a, A: Asset, S: Settings>( + pub fn load_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( &self, path: impl Into>, settings: S, ) -> Handle { - self.load_with_meta_transform(path, Some(Box::new(settings)), (), false) + self.load_with_meta_transform(path.into().with_settings(settings), None, (), false) } /// Same as [`load`](AssetServer::load_with_settings), but you can load assets from unaproved paths @@ -385,12 +385,12 @@ impl AssetServer { /// is [`Deny`](UnapprovedPathMode::Deny). /// /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`] - pub fn load_with_settings_override<'a, A: Asset, S: Settings>( + pub fn load_with_settings_override<'a, A: Asset, S: Settings + serde::Serialize>( &self, path: impl Into>, settings: S, ) -> Handle { - self.load_with_meta_transform(path, Some(Box::new(settings)), (), true) + self.load_with_meta_transform(path.into().with_settings(settings), None, (), true) } /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item. @@ -403,13 +403,18 @@ impl AssetServer { /// [`AssetLoader`] settings. The type `S` _must_ match the configured [`AssetLoader::Settings`] or `settings` changes /// will be ignored and an error will be printed to the log. #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] - pub fn load_acquire_with_settings<'a, A: Asset, S: Settings, G: Send + Sync + 'static>( + pub fn load_acquire_with_settings< + 'a, + A: Asset, + S: Settings + serde::Serialize, + G: Send + Sync + 'static, + >( &self, path: impl Into>, settings: S, guard: G, ) -> Handle { - self.load_with_meta_transform(path, Some(Box::new(settings)), guard, false) + self.load_with_meta_transform(path.into().with_settings(settings), None, guard, false) } /// Same as [`load`](AssetServer::load_acquire_with_settings), but you can load assets from unaproved paths @@ -420,7 +425,7 @@ impl AssetServer { pub fn load_acquire_with_settings_override< 'a, A: Asset, - S: Settings, + S: Settings + serde::Serialize, G: Send + Sync + 'static, >( &self, @@ -428,7 +433,7 @@ impl AssetServer { settings: S, guard: G, ) -> Handle { - self.load_with_meta_transform(path, Some(Box::new(settings)), guard, true) + self.load_with_meta_transform(path.into().with_settings(settings), None, guard, true) } pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>( @@ -648,11 +653,11 @@ impl AssetServer { } })?; - if let Some(meta_transform) = input_handle.as_ref().and_then(|h| h.meta_transform()) { + if let Some(settings) = path.settings() { // Deref to avoid `apply_settings` downcast failing because the type // is `Box` when it expected `T`. XXX TODO: Better solution. - let meta_transform = core::ops::Deref::deref(meta_transform); - meta.apply_settings(meta_transform); + let settings = core::ops::Deref::deref(&settings.value); + meta.apply_settings(settings); } // downgrade the input handle so we don't keep the asset alive just because we're loading it // note we can't just pass a weak handle in, as only strong handles contain the asset meta transform From 5779ba27d0dc84e339ce6974be123a6afc8b0631 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Thu, 8 May 2025 21:56:47 +0100 Subject: [PATCH 03/32] Updated examples. --- examples/2d/mesh2d_repeated_texture.rs | 16 +++++++--------- examples/3d/deferred_rendering.rs | 5 ++++- examples/3d/parallax_mapping.rs | 5 ++++- examples/3d/scrolling_fog.rs | 9 +++++---- examples/asset/alter_mesh.rs | 5 ++++- examples/asset/alter_sprite.rs | 5 ++++- examples/asset/asset_decompression.rs | 4 ++++ examples/asset/asset_settings.rs | 5 +++-- examples/asset/repeated_texture.rs | 16 +++++++--------- examples/ui/ui_texture_slice_flip_and_tile.rs | 5 +++-- 10 files changed, 45 insertions(+), 30 deletions(-) diff --git a/examples/2d/mesh2d_repeated_texture.rs b/examples/2d/mesh2d_repeated_texture.rs index 096a1a5f99d3e..8436b221f8093 100644 --- a/examples/2d/mesh2d_repeated_texture.rs +++ b/examples/2d/mesh2d_repeated_texture.rs @@ -34,16 +34,14 @@ fn setup( asset_server.load("textures/fantasy_ui_borders/panel-border-010.png"); let image_with_repeated_sampler = asset_server.load_with_settings( "textures/fantasy_ui_borders/panel-border-010-repeated.png", - |s: &mut _| { - *s = ImageLoaderSettings { - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - // rewriting mode to repeat image, - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, - ..default() - }), + ImageLoaderSettings { + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + // rewriting mode to repeat image, + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, ..default() - } + }), + ..default() }, ); diff --git a/examples/3d/deferred_rendering.rs b/examples/3d/deferred_rendering.rs index d6e58fe5058a8..c709c170ad090 100644 --- a/examples/3d/deferred_rendering.rs +++ b/examples/3d/deferred_rendering.rs @@ -226,7 +226,10 @@ fn setup_parallax( "textures/parallax_example/cube_normal.png", // The normal map texture is in linear color space. Lighting won't look correct // if `is_srgb` is `true`, which is the default. - |settings: &mut ImageLoaderSettings| settings.is_srgb = false, + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, ); let mut cube = Mesh::from(Cuboid::new(0.15, 0.15, 0.15)); diff --git a/examples/3d/parallax_mapping.rs b/examples/3d/parallax_mapping.rs index 47d83a4e76960..f3a24a434c637 100644 --- a/examples/3d/parallax_mapping.rs +++ b/examples/3d/parallax_mapping.rs @@ -208,7 +208,10 @@ fn setup( "textures/parallax_example/cube_normal.png", // The normal map texture is in linear color space. Lighting won't look correct // if `is_srgb` is `true`, which is the default. - |settings: &mut ImageLoaderSettings| settings.is_srgb = false, + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, ); // Camera diff --git a/examples/3d/scrolling_fog.rs b/examples/3d/scrolling_fog.rs index f65b4131701cd..e42e3d3773fa1 100644 --- a/examples/3d/scrolling_fog.rs +++ b/examples/3d/scrolling_fog.rs @@ -94,8 +94,9 @@ fn setup( // Load a repeating 3d noise texture. Make sure to set ImageAddressMode to Repeat // so that the texture wraps around as the density texture offset is moved along. // Also set ImageFilterMode to Linear so that the fog isn't pixelated. - let noise_texture = assets.load_with_settings("volumes/fog_noise.ktx2", |settings: &mut _| { - *settings = ImageLoaderSettings { + let noise_texture = assets.load_with_settings( + "volumes/fog_noise.ktx2", + ImageLoaderSettings { sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: ImageAddressMode::Repeat, address_mode_v: ImageAddressMode::Repeat, @@ -106,8 +107,8 @@ fn setup( ..default() }), ..default() - } - }); + }, + ); // Spawn a FogVolume and use the repeating noise texture as its density texture. commands.spawn(( diff --git a/examples/asset/alter_mesh.rs b/examples/asset/alter_mesh.rs index 7462c47ae1f85..fe1ea5108d722 100644 --- a/examples/asset/alter_mesh.rs +++ b/examples/asset/alter_mesh.rs @@ -83,7 +83,10 @@ fn setup( // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the mesh in // RAM. For this example however, this would not work, as we need to inspect and modify the // mesh at runtime. - |settings: &mut GltfLoaderSettings| settings.load_meshes = RenderAssetUsages::all(), + GltfLoaderSettings { + load_meshes: RenderAssetUsages::all(), + ..Default::default() + }, ); // Here, we rely on the default loader settings to achieve a similar result to the above. diff --git a/examples/asset/alter_sprite.rs b/examples/asset/alter_sprite.rs index d47c303921521..c1d6736ed97d2 100644 --- a/examples/asset/alter_sprite.rs +++ b/examples/asset/alter_sprite.rs @@ -66,7 +66,10 @@ fn setup(mut commands: Commands, asset_server: Res) { // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the image in // RAM. For this example however, this would not work, as we need to inspect and modify the // image at runtime. - |settings: &mut ImageLoaderSettings| settings.asset_usage = RenderAssetUsages::all(), + ImageLoaderSettings { + asset_usage: RenderAssetUsages::all(), + ..Default::default() + }, ); commands.spawn(( diff --git a/examples/asset/asset_decompression.rs b/examples/asset/asset_decompression.rs index e514924ca9d0a..68270951142a8 100644 --- a/examples/asset/asset_decompression.rs +++ b/examples/asset/asset_decompression.rs @@ -23,6 +23,10 @@ struct GzAssetLoader; /// Possible errors that can be produced by [`GzAssetLoader`] #[non_exhaustive] #[derive(Debug, Error)] +#[expect( + clippy::large_enum_variant, + reason = "XXX TODO: Why did this start happening?" +)] enum GzAssetLoaderError { /// An [IO](std::io) Error #[error("Could not load asset: {0}")] diff --git a/examples/asset/asset_settings.rs b/examples/asset/asset_settings.rs index cfc76d774a960..c8cf01909bec5 100644 --- a/examples/asset/asset_settings.rs +++ b/examples/asset/asset_settings.rs @@ -64,8 +64,9 @@ fn setup(mut commands: Commands, asset_server: Res) { Sprite { image: asset_server.load_with_settings( "bevy_pixel_dark_with_settings.png", - |settings: &mut ImageLoaderSettings| { - settings.sampler = ImageSampler::nearest(); + ImageLoaderSettings { + sampler: ImageSampler::nearest(), + ..Default::default() }, ), custom_size: Some(Vec2 { x: 160.0, y: 120.0 }), diff --git a/examples/asset/repeated_texture.rs b/examples/asset/repeated_texture.rs index 34664a9e42b5c..a39f8755c4033 100644 --- a/examples/asset/repeated_texture.rs +++ b/examples/asset/repeated_texture.rs @@ -39,16 +39,14 @@ fn setup( MeshMaterial3d(materials.add(StandardMaterial { base_color_texture: Some(asset_server.load_with_settings( "textures/fantasy_ui_borders/panel-border-010-repeated.png", - |s: &mut _| { - *s = ImageLoaderSettings { - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - // rewriting mode to repeat image, - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, - ..default() - }), + ImageLoaderSettings { + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + // rewriting mode to repeat image, + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, ..default() - } + }), + ..default() }, )), diff --git a/examples/ui/ui_texture_slice_flip_and_tile.rs b/examples/ui/ui_texture_slice_flip_and_tile.rs index 3eaf4277f5b70..7977f52f1a417 100644 --- a/examples/ui/ui_texture_slice_flip_and_tile.rs +++ b/examples/ui/ui_texture_slice_flip_and_tile.rs @@ -20,9 +20,10 @@ fn main() { fn setup(mut commands: Commands, asset_server: Res) { let image = asset_server.load_with_settings( "textures/fantasy_ui_borders/numbered_slices.png", - |settings: &mut ImageLoaderSettings| { + ImageLoaderSettings { // Need to use nearest filtering to avoid bleeding between the slices with tiling - settings.sampler = ImageSampler::nearest(); + sampler: ImageSampler::nearest(), + ..Default::default() }, ); From eab30f4c6401f888b2d6ab35224acc670cdc4762 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Fri, 9 May 2025 00:08:58 +0100 Subject: [PATCH 04/32] Removed meta transform since it's no longer used. --- crates/bevy_asset/src/assets.rs | 7 +-- crates/bevy_asset/src/handle.rs | 31 +--------- crates/bevy_asset/src/loader.rs | 6 +- crates/bevy_asset/src/loader_builders.rs | 76 +++++++----------------- crates/bevy_asset/src/meta.rs | 2 - crates/bevy_asset/src/path.rs | 11 ++++ crates/bevy_asset/src/server/info.rs | 33 +++------- crates/bevy_asset/src/server/mod.rs | 62 ++++++------------- 8 files changed, 67 insertions(+), 161 deletions(-) diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 9fa8eb4381485..0a962a373c575 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -387,10 +387,7 @@ impl Assets { 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( - self.handle_provider - .get_handle(index.into(), false, None, None), - ) + Handle::Strong(self.handle_provider.get_handle(index.into(), false, None)) } /// Upgrade an `AssetId` into a strong `Handle` that will prevent asset drop. @@ -408,7 +405,7 @@ impl Assets { AssetId::Uuid { uuid } => uuid.into(), }; Some(Handle::Strong( - self.handle_provider.get_handle(index, false, None, None), + self.handle_provider.get_handle(index, false, None), )) } diff --git a/crates/bevy_asset/src/handle.rs b/crates/bevy_asset/src/handle.rs index a78c43ffdde30..a8f2158e49e3e 100644 --- a/crates/bevy_asset/src/handle.rs +++ b/crates/bevy_asset/src/handle.rs @@ -1,7 +1,4 @@ -use crate::{ - meta::MetaTransform, Asset, AssetId, AssetIndexAllocator, AssetPath, InternalAssetId, - UntypedAssetId, -}; +use crate::{Asset, AssetId, AssetIndexAllocator, AssetPath, InternalAssetId, UntypedAssetId}; use alloc::sync::Arc; use bevy_reflect::{std_traits::ReflectDefault, Reflect, TypePath}; use core::{ @@ -44,7 +41,7 @@ impl AssetHandleProvider { /// [`UntypedHandle`] will match the [`Asset`] [`TypeId`] assigned to this [`AssetHandleProvider`]. pub fn reserve_handle(&self) -> UntypedHandle { let index = self.allocator.reserve(); - UntypedHandle::Strong(self.get_handle(InternalAssetId::Index(index), false, None, None)) + UntypedHandle::Strong(self.get_handle(InternalAssetId::Index(index), false, None)) } pub(crate) fn get_handle( @@ -52,12 +49,10 @@ impl AssetHandleProvider { id: InternalAssetId, asset_server_managed: bool, path: Option>, - meta_transform: Option, ) -> Arc { Arc::new(StrongHandle { id: id.untyped(self.type_id), drop_sender: self.drop_sender.clone(), - meta_transform, path, asset_server_managed, }) @@ -67,15 +62,9 @@ impl AssetHandleProvider { &self, asset_server_managed: bool, path: Option>, - meta_transform: Option, ) -> Arc { let index = self.allocator.reserve(); - self.get_handle( - InternalAssetId::Index(index), - asset_server_managed, - path, - meta_transform, - ) + self.get_handle(InternalAssetId::Index(index), asset_server_managed, path) } } @@ -86,10 +75,6 @@ pub struct StrongHandle { pub(crate) id: UntypedAssetId, pub(crate) asset_server_managed: bool, pub(crate) path: Option>, - /// Modifies asset meta. This is stored on the handle because it is: - /// 1. configuration tied to the lifetime of a specific asset load - /// 2. configuration that must be repeatable when the asset is hot-reloaded - pub(crate) meta_transform: Option, pub(crate) drop_sender: Sender, } @@ -381,16 +366,6 @@ impl UntypedHandle { pub fn try_typed(self) -> Result, UntypedAssetConversionError> { Handle::try_from(self) } - - /// The "meta transform" for the strong handle. This will only be [`Some`] if the handle is strong and there is a meta transform - /// associated with it. - #[inline] - pub fn meta_transform(&self) -> Option<&MetaTransform> { - match self { - UntypedHandle::Strong(handle) => handle.meta_transform.as_ref(), - UntypedHandle::Weak(_) => None, - } - } } impl PartialEq for UntypedHandle { diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 841c51daa8a05..0a1cfccabe40a 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -432,9 +432,7 @@ impl<'a> LoadContext<'a> { let label = label.into(); let loaded_asset: ErasedLoadedAsset = loaded_asset.into(); let labeled_path = self.asset_path.clone().with_label(label.clone()); - let handle = self - .asset_server - .get_or_create_path_handle(labeled_path, None); + let handle = self.asset_server.get_or_create_path_handle(labeled_path); self.labeled_assets.insert( label, LabeledAsset { @@ -518,7 +516,7 @@ impl<'a> LoadContext<'a> { label: impl Into>, ) -> Handle { let path = self.asset_path.clone().with_label(label); - let handle = self.asset_server.get_or_create_path_handle::(path, None); + let handle = self.asset_server.get_or_create_path_handle::(path); self.dependencies.insert(handle.id().untyped()); handle } diff --git a/crates/bevy_asset/src/loader_builders.rs b/crates/bevy_asset/src/loader_builders.rs index dd22b971760e7..1a0b1575b94fc 100644 --- a/crates/bevy_asset/src/loader_builders.rs +++ b/crates/bevy_asset/src/loader_builders.rs @@ -2,10 +2,9 @@ //! [`LoadContext::loader`]. use crate::{ - io::Reader, - meta::{MetaTransform, Settings}, - Asset, AssetLoadError, AssetPath, ErasedAssetLoader, ErasedLoadedAsset, Handle, LoadContext, - LoadDirectError, LoadedAsset, LoadedUntypedAsset, UntypedHandle, + io::Reader, meta::Settings, Asset, AssetLoadError, AssetPath, AssetPathSettings, + ErasedAssetLoader, ErasedLoadedAsset, Handle, LoadContext, LoadDirectError, LoadedAsset, + LoadedUntypedAsset, UntypedHandle, }; use alloc::{borrow::ToOwned, boxed::Box, sync::Arc}; use core::any::TypeId; @@ -117,7 +116,7 @@ impl ReaderRef<'_> { /// [`LoadTransformAndSave`]: crate::processor::LoadTransformAndSave pub struct NestedLoader<'ctx, 'builder, T, M> { load_context: &'builder mut LoadContext<'ctx>, - meta_transform: Option, + settings: Option, typing: T, mode: M, } @@ -168,7 +167,7 @@ impl<'ctx, 'builder> NestedLoader<'ctx, 'builder, StaticTyped, Deferred> { pub(crate) fn new(load_context: &'builder mut LoadContext<'ctx>) -> Self { NestedLoader { load_context, - meta_transform: None, + settings: None, typing: StaticTyped(()), mode: Deferred(()), } @@ -176,30 +175,13 @@ impl<'ctx, 'builder> NestedLoader<'ctx, 'builder, StaticTyped, Deferred> { } impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'builder, T, M> { - /* - fn with_transform( - mut self, - transform: impl Fn(&mut dyn AssetMetaDyn) + Send + Sync + 'static, - ) -> Self { - if let Some(prev_transform) = self.meta_transform { - self.meta_transform = Some(Box::new(move |meta| { - prev_transform(meta); - transform(meta); - })); - } else { - self.meta_transform = Some(Box::new(transform)); - } - self - } - */ - /// Configure the settings used to load the asset. /// /// If the settings type `S` does not match the settings expected by `A`'s asset loader, an error will be printed to the log /// and the asset load will fail. #[must_use] - pub fn with_settings(mut self, settings: S) -> Self { - self.meta_transform = Some(Box::new(settings)); + pub fn with_settings(mut self, settings: S) -> Self { + self.settings = Some(AssetPathSettings::new(settings)); self } @@ -218,7 +200,7 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui pub fn with_static_type(self) -> NestedLoader<'ctx, 'builder, StaticTyped, M> { NestedLoader { load_context: self.load_context, - meta_transform: self.meta_transform, + settings: self.settings, typing: StaticTyped(()), mode: self.mode, } @@ -235,7 +217,7 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui ) -> NestedLoader<'ctx, 'builder, DynamicTyped, M> { NestedLoader { load_context: self.load_context, - meta_transform: self.meta_transform, + settings: self.settings, typing: DynamicTyped { asset_type_id }, mode: self.mode, } @@ -249,7 +231,7 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui pub fn with_unknown_type(self) -> NestedLoader<'ctx, 'builder, UnknownTyped, M> { NestedLoader { load_context: self.load_context, - meta_transform: self.meta_transform, + settings: self.settings, typing: UnknownTyped(()), mode: self.mode, } @@ -264,7 +246,7 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui pub fn deferred(self) -> NestedLoader<'ctx, 'builder, T, Deferred> { NestedLoader { load_context: self.load_context, - meta_transform: self.meta_transform, + settings: self.settings, typing: self.typing, mode: Deferred(()), } @@ -281,7 +263,7 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui pub fn immediate<'c>(self) -> NestedLoader<'ctx, 'builder, T, Immediate<'builder, 'c>> { NestedLoader { load_context: self.load_context, - meta_transform: self.meta_transform, + settings: self.settings, typing: self.typing, mode: Immediate { reader: None }, } @@ -303,18 +285,15 @@ impl NestedLoader<'_, '_, StaticTyped, Deferred> { /// [`with_dynamic_type`]: Self::with_dynamic_type /// [`with_unknown_type`]: Self::with_unknown_type pub fn load<'c, A: Asset>(self, path: impl Into>) -> Handle { - let path = path.into().to_owned(); + let path = path.into().with_settings_2(self.settings).to_owned(); let handle = if self.load_context.should_load_dependencies { - self.load_context.asset_server.load_with_meta_transform( - path, - self.meta_transform, - (), - true, - ) + self.load_context + .asset_server + .load_with_meta_transform(path, (), true) } else { self.load_context .asset_server - .get_or_create_path_handle(path, None) + .get_or_create_path_handle(path) }; self.load_context.dependencies.insert(handle.id().untyped()); handle @@ -330,24 +309,15 @@ impl NestedLoader<'_, '_, DynamicTyped, Deferred> { /// /// [`with_dynamic_type`]: Self::with_dynamic_type pub fn load<'p>(self, path: impl Into>) -> UntypedHandle { - let path = path.into().to_owned(); + let path = path.into().with_settings_2(self.settings).to_owned(); let handle = if self.load_context.should_load_dependencies { self.load_context .asset_server - .load_erased_with_meta_transform( - path, - self.typing.asset_type_id, - self.meta_transform, - (), - ) + .load_erased_with_meta_transform(path, self.typing.asset_type_id, ()) } else { self.load_context .asset_server - .get_or_create_path_handle_erased( - path, - self.typing.asset_type_id, - self.meta_transform, - ) + .get_or_create_path_handle_erased(path, self.typing.asset_type_id) }; self.load_context.dependencies.insert(handle.id()); handle @@ -360,15 +330,15 @@ impl NestedLoader<'_, '_, UnknownTyped, Deferred> { /// /// This will infer the asset type from metadata. pub fn load<'p>(self, path: impl Into>) -> Handle { - let path = path.into().to_owned(); + let path = path.into().with_settings_2(self.settings).to_owned(); let handle = if self.load_context.should_load_dependencies { self.load_context .asset_server - .load_unknown_type_with_meta_transform(path, self.meta_transform) + .load_unknown_type_with_meta_transform(path) } else { self.load_context .asset_server - .get_or_create_path_handle(path, self.meta_transform) + .get_or_create_path_handle(path) }; self.load_context.dependencies.insert(handle.id().untyped()); handle diff --git a/crates/bevy_asset/src/meta.rs b/crates/bevy_asset/src/meta.rs index 74089679169ef..01d8e51879a46 100644 --- a/crates/bevy_asset/src/meta.rs +++ b/crates/bevy_asset/src/meta.rs @@ -1,5 +1,4 @@ use alloc::{ - boxed::Box, string::{String, ToString}, vec::Vec, }; @@ -13,7 +12,6 @@ use ron::ser::PrettyConfig; use serde::{Deserialize, Serialize}; pub const META_FORMAT_VERSION: &str = "1.0"; -pub type MetaTransform = Box; /// Asset metadata that informs how an [`Asset`] should be handled by the asset system. /// diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index da17b202a98f9..afcb264ab8b52 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -365,6 +365,17 @@ impl<'a> AssetPath<'a> { } } + // XXX TODO: Ugly duplication. + #[inline] + pub fn with_settings_2(self, settings: Option) -> AssetPath<'a> { + AssetPath { + source: self.source, + path: self.path, + label: self.label, + settings: settings.map(Arc::new), + } + } + /// Returns an [`AssetPath`] for the parent folder of this path, if there is a parent folder in the path. pub fn parent(&self) -> Option> { let path = match &self.path { diff --git a/crates/bevy_asset/src/server/info.rs b/crates/bevy_asset/src/server/info.rs index 1b3bb3cb65d68..967fb5048f497 100644 --- a/crates/bevy_asset/src/server/info.rs +++ b/crates/bevy_asset/src/server/info.rs @@ -1,8 +1,7 @@ use crate::{ - meta::{AssetHash, MetaTransform}, - Asset, AssetHandleProvider, AssetLoadError, AssetPath, DependencyLoadState, ErasedLoadedAsset, - Handle, InternalAssetEvent, LoadState, RecursiveDependencyLoadState, StrongHandle, - UntypedAssetId, UntypedHandle, + meta::AssetHash, Asset, AssetHandleProvider, AssetLoadError, AssetPath, DependencyLoadState, + ErasedLoadedAsset, Handle, InternalAssetEvent, LoadState, RecursiveDependencyLoadState, + StrongHandle, UntypedAssetId, UntypedHandle, }; use alloc::{ borrow::ToOwned, @@ -111,7 +110,6 @@ impl AssetInfos { self.watching_for_changes, type_id, None, - None, true, ), Either::Left(type_name), @@ -126,7 +124,6 @@ impl AssetInfos { watching_for_changes: bool, type_id: TypeId, path: Option>, - meta_transform: Option, loading: bool, ) -> Result { let provider = handle_providers @@ -143,7 +140,7 @@ impl AssetInfos { } } - let handle = provider.reserve_handle_internal(true, path.clone(), meta_transform); + let handle = provider.reserve_handle_internal(true, path.clone()); let mut info = AssetInfo::new(Arc::downgrade(&handle), path); if loading { info.load_state = LoadState::Loading; @@ -159,14 +156,9 @@ impl AssetInfos { &mut self, path: AssetPath<'static>, loading_mode: HandleLoadingMode, - meta_transform: Option, ) -> (Handle, bool) { - let result = self.get_or_create_path_handle_internal( - path, - Some(TypeId::of::()), - loading_mode, - meta_transform, - ); + let result = + self.get_or_create_path_handle_internal(path, Some(TypeId::of::()), loading_mode); // it is ok to unwrap because TypeId was specified above let (handle, should_load) = unwrap_with_context(result, Either::Left(core::any::type_name::())).unwrap(); @@ -179,14 +171,8 @@ impl AssetInfos { type_id: TypeId, type_name: Option<&str>, loading_mode: HandleLoadingMode, - meta_transform: Option, ) -> (UntypedHandle, bool) { - let result = self.get_or_create_path_handle_internal( - path, - Some(type_id), - loading_mode, - meta_transform, - ); + let result = self.get_or_create_path_handle_internal(path, Some(type_id), loading_mode); let type_info = match type_name { Some(type_name) => Either::Left(type_name), None => Either::Right(type_id), @@ -202,7 +188,6 @@ impl AssetInfos { path: AssetPath<'static>, type_id: Option, loading_mode: HandleLoadingMode, - meta_transform: Option, ) -> Result<(UntypedHandle, bool), GetOrCreateHandleInternalError> { let handles = self.path_to_id.entry(path.clone()).or_default(); @@ -251,8 +236,7 @@ impl AssetInfos { .handle_providers .get(&type_id) .ok_or(MissingHandleProviderError(type_id))?; - let handle = - provider.get_handle(id.internal(), true, Some(path), meta_transform); + let handle = provider.get_handle(id.internal(), true, Some(path)); info.weak_handle = Arc::downgrade(&handle); Ok((UntypedHandle::Strong(handle), should_load)) } @@ -270,7 +254,6 @@ impl AssetInfos { self.watching_for_changes, type_id, Some(path), - meta_transform, should_load, )?; entry.insert(handle.id()); diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index e8acf8b78208c..abfc67e5def84 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -9,7 +9,7 @@ use crate::{ MissingProcessedAssetReaderError, Reader, }, loader::{AssetLoader, ErasedAssetLoader, LoadContext, LoadedAsset}, - meta::{AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal, MetaTransform, Settings}, + meta::{AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal, Settings}, path::AssetPath, Asset, AssetEvent, AssetHandleProvider, AssetId, AssetLoadFailedEvent, AssetMetaCheck, Assets, DeserializeMetaError, ErasedLoadedAsset, Handle, LoadedUntypedAsset, UnapprovedPathMode, @@ -319,7 +319,7 @@ impl AssetServer { /// The asset load will fail and an error will be printed to the logs if the asset stored at `path` is not of type `A`. #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] pub fn load<'a, A: Asset>(&self, path: impl Into>) -> Handle { - self.load_with_meta_transform(path, None, (), false) + self.load_with_meta_transform(path, (), false) } /// Same as [`load`](AssetServer::load), but you can load assets from unaproved paths @@ -328,7 +328,7 @@ impl AssetServer { /// /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`] pub fn load_override<'a, A: Asset>(&self, path: impl Into>) -> Handle { - self.load_with_meta_transform(path, None, (), true) + self.load_with_meta_transform(path, (), true) } /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item. @@ -352,7 +352,7 @@ impl AssetServer { path: impl Into>, guard: G, ) -> Handle { - self.load_with_meta_transform(path, None, guard, false) + self.load_with_meta_transform(path, guard, false) } /// Same as [`load`](AssetServer::load_acquire), but you can load assets from unaproved paths @@ -365,7 +365,7 @@ impl AssetServer { path: impl Into>, guard: G, ) -> Handle { - self.load_with_meta_transform(path, None, guard, true) + self.load_with_meta_transform(path, guard, true) } /// Begins loading an [`Asset`] of type `A` stored at `path`. The given `settings` function will override the asset's @@ -377,7 +377,7 @@ impl AssetServer { path: impl Into>, settings: S, ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), None, (), false) + self.load_with_meta_transform(path.into().with_settings(settings), (), false) } /// Same as [`load`](AssetServer::load_with_settings), but you can load assets from unaproved paths @@ -390,7 +390,7 @@ impl AssetServer { path: impl Into>, settings: S, ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), None, (), true) + self.load_with_meta_transform(path.into().with_settings(settings), (), true) } /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item. @@ -414,7 +414,7 @@ impl AssetServer { settings: S, guard: G, ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), None, guard, false) + self.load_with_meta_transform(path.into().with_settings(settings), guard, false) } /// Same as [`load`](AssetServer::load_acquire_with_settings), but you can load assets from unaproved paths @@ -433,13 +433,12 @@ impl AssetServer { settings: S, guard: G, ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), None, guard, true) + self.load_with_meta_transform(path.into().with_settings(settings), guard, true) } pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>( &self, path: impl Into>, - meta_transform: Option, guard: G, override_unapproved: bool, ) -> Handle { @@ -456,11 +455,8 @@ impl AssetServer { } let mut infos = self.data.infos.write(); - let (handle, should_load) = infos.get_or_create_path_handle::( - path.clone(), - HandleLoadingMode::Request, - meta_transform, - ); + let (handle, should_load) = + infos.get_or_create_path_handle::(path.clone(), HandleLoadingMode::Request); if should_load { self.spawn_load_task(handle.clone().untyped(), path, infos, guard); @@ -473,7 +469,6 @@ impl AssetServer { &self, path: impl Into>, type_id: TypeId, - meta_transform: Option, guard: G, ) -> UntypedHandle { let path = path.into().into_owned(); @@ -483,7 +478,6 @@ impl AssetServer { type_id, None, HandleLoadingMode::Request, - meta_transform, ); if should_load { @@ -507,10 +501,7 @@ impl AssetServer { let owned_handle = handle.clone(); let server = self.clone(); let task = IoTaskPool::get().spawn(async move { - if let Err(err) = server - .load_internal(Some(owned_handle), path, false, None) - .await - { + if let Err(err) = server.load_internal(Some(owned_handle), path, false).await { error!("{}", err); } drop(guard); @@ -535,13 +526,12 @@ impl AssetServer { path: impl Into>, ) -> Result { let path: AssetPath = path.into(); - self.load_internal(None, path, false, None).await + self.load_internal(None, path, false).await } pub(crate) fn load_unknown_type_with_meta_transform<'a>( &self, path: impl Into>, - meta_transform: Option, ) -> Handle { let path = path.into().into_owned(); let untyped_source = AssetSourceId::Name(match path.source() { @@ -554,7 +544,6 @@ impl AssetServer { let (handle, should_load) = infos.get_or_create_path_handle::( path.clone().with_source(untyped_source), HandleLoadingMode::Request, - meta_transform, ); // drop the lock on `AssetInfos` before spawning a task that may block on it in single-threaded @@ -620,7 +609,7 @@ impl AssetServer { /// required to figure out the asset type before a handle can be created. #[must_use = "not using the returned strong handle may result in the unexpected release of the assets"] pub fn load_untyped<'a>(&self, path: impl Into>) -> Handle { - self.load_unknown_type_with_meta_transform(path, None) + self.load_unknown_type_with_meta_transform(path) } /// Performs an async asset load. @@ -632,7 +621,6 @@ impl AssetServer { mut input_handle: Option, path: AssetPath<'a>, force: bool, - meta_transform: Option, ) -> Result { let asset_type_id = input_handle.as_ref().map(UntypedHandle::type_id); @@ -687,7 +675,6 @@ impl AssetServer { path.clone(), path.label().is_none().then(|| loader.asset_type_id()), HandleLoadingMode::Request, - meta_transform, ); unwrap_with_context(result, Either::Left(loader.asset_type_name())) } @@ -724,7 +711,6 @@ impl AssetServer { loader.asset_type_id(), Some(loader.asset_type_name()), HandleLoadingMode::Force, - None, ); (base_handle, base_path) } else { @@ -802,7 +788,7 @@ impl AssetServer { .infos .read() .get_path_handles(&path) - .map(|handle| server.load_internal(Some(handle), path.clone(), true, None)) + .map(|handle| server.load_internal(Some(handle), path.clone(), true)) .collect::>(); for result in requests { @@ -813,7 +799,7 @@ impl AssetServer { } if !reloaded && server.data.infos.read().should_reload(&path) { - if let Err(err) = server.load_internal(None, path, true, None).await { + if let Err(err) = server.load_internal(None, path, true).await { error!("{}", err); } } @@ -850,7 +836,6 @@ impl AssetServer { loaded_asset.asset_type_id(), Some(loaded_asset.asset_type_name()), HandleLoadingMode::NotLoading, - None, ); handle } else { @@ -935,11 +920,7 @@ impl AssetServer { .data .infos .write() - .get_or_create_path_handle::( - path.clone(), - HandleLoadingMode::Request, - None, - ); + .get_or_create_path_handle::(path.clone(), HandleLoadingMode::Request); if !should_load { return handle; } @@ -1261,15 +1242,10 @@ impl AssetServer { pub(crate) fn get_or_create_path_handle<'a, A: Asset>( &self, path: impl Into>, - meta_transform: Option, ) -> Handle { let mut infos = self.data.infos.write(); infos - .get_or_create_path_handle::( - path.into().into_owned(), - HandleLoadingMode::NotLoading, - meta_transform, - ) + .get_or_create_path_handle::(path.into().into_owned(), HandleLoadingMode::NotLoading) .0 } @@ -1281,7 +1257,6 @@ impl AssetServer { &self, path: impl Into>, type_id: TypeId, - meta_transform: Option, ) -> UntypedHandle { let mut infos = self.data.infos.write(); infos @@ -1290,7 +1265,6 @@ impl AssetServer { type_id, None, HandleLoadingMode::NotLoading, - meta_transform, ) .0 } From a47b90f2fbf6e95e1764e9fb58690f0321622dd5 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Fri, 9 May 2025 09:20:48 +0100 Subject: [PATCH 05/32] Added type id to AssetPathSettingsId. Implemented Display for AssetPathSettings. --- crates/bevy_asset/src/path.rs | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index afcb264ab8b52..3bc4dd2b35f2b 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -9,6 +9,7 @@ use atomicow::CowArc; use bevy_platform::hash::FixedHasher; use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize}; use core::{ + any::TypeId, fmt::{Debug, Display}, hash::{BuildHasher, Hash, Hasher}, ops::Deref, @@ -18,7 +19,18 @@ use std::path::{Path, PathBuf}; use thiserror::Error; #[derive(Eq, PartialEq, Hash, Copy, Clone)] -pub struct AssetPathSettingsId(u64); +pub struct AssetPathSettingsId { + // XXX TODO: Unclear if we want type id? Does make debugging a bit easier. + type_id: TypeId, + hash: u64, +} + +impl Display for AssetPathSettingsId { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + // XXX TODO: Reconsider formatting. Also we're using Debug for type_id... + write!(f, "{:?}/{}", self.type_id, self.hash) + } +} pub struct AssetPathSettings { pub value: Box, @@ -32,7 +44,10 @@ impl AssetPathSettings { // XXX TODO: What's the appropriate hasher? // XXX TODO: Should we hash the type id as well? - let id = AssetPathSettingsId(FixedHasher.hash_one(&string)); + let id = AssetPathSettingsId { + type_id: TypeId::of::(), + hash: FixedHasher.hash_one(&string), + }; AssetPathSettings { value: Box::new(settings), @@ -55,6 +70,12 @@ impl PartialEq for AssetPathSettings { impl Eq for AssetPathSettings {} +impl Display for AssetPathSettings { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + self.id.fmt(f) + } +} + /// Represents a path to an asset in a "virtual filesystem". /// /// Asset paths consist of three main parts: @@ -99,6 +120,8 @@ pub struct AssetPath<'a> { source: AssetSourceId<'a>, path: CowArc<'a, Path>, label: Option>, + // XXX TODO: This is an Arc for now to simplify the implementation. Should + // consider changing to CowArc. settings: Option>, } @@ -117,6 +140,11 @@ impl<'a> Display for AssetPath<'a> { if let Some(label) = &self.label { write!(f, "#{label}")?; } + // XXX TODO: This might need a rethink as I'm not sure if the output + // needs to be parseable. Also see comments on AssetPathSettingsId::fmt. + if let Some(settings) = &self.settings { + write!(f, " (settings: {settings})")?; + } Ok(()) } } From f0f645ce0db1bb9c2b5b58e57263ae41f5b83c65 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Fri, 9 May 2025 11:55:42 +0100 Subject: [PATCH 06/32] Removed `load_with_settings` and variants. This is now done by `AssetPath::with_settings`. --- crates/bevy_asset/src/direct_access_ext.rs | 21 +----- crates/bevy_asset/src/server/mod.rs | 70 +------------------ examples/2d/mesh2d_repeated_texture.rs | 22 +++--- examples/3d/deferred_rendering.rs | 18 ++--- examples/3d/parallax_mapping.rs | 18 ++--- examples/3d/scrolling_fog.rs | 6 +- examples/3d/ssr.rs | 28 ++++---- examples/asset/alter_mesh.rs | 8 +-- examples/asset/alter_sprite.rs | 41 +++++------ examples/asset/asset_settings.rs | 14 ++-- examples/asset/repeated_texture.rs | 43 +++++++----- examples/ui/ui_texture_slice_flip_and_tile.rs | 16 +++-- 12 files changed, 119 insertions(+), 186 deletions(-) diff --git a/crates/bevy_asset/src/direct_access_ext.rs b/crates/bevy_asset/src/direct_access_ext.rs index 7d77538063e99..d65b307a5d779 100644 --- a/crates/bevy_asset/src/direct_access_ext.rs +++ b/crates/bevy_asset/src/direct_access_ext.rs @@ -3,7 +3,7 @@ use bevy_ecs::world::World; -use crate::{meta::Settings, Asset, AssetPath, AssetServer, Assets, Handle}; +use crate::{Asset, AssetPath, AssetServer, Assets, Handle}; /// An extension trait for methods for working with assets directly from a [`World`]. pub trait DirectAssetAccessExt { @@ -12,13 +12,6 @@ pub trait DirectAssetAccessExt { /// Load an asset similarly to [`AssetServer::load`]. fn load_asset<'a, A: Asset>(&self, path: impl Into>) -> Handle; - - /// Load an asset with settings, similarly to [`AssetServer::load_with_settings`]. - fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( - &self, - path: impl Into>, - settings: S, - ) -> Handle; } impl DirectAssetAccessExt for World { /// Insert an asset similarly to [`Assets::add`]. @@ -36,16 +29,4 @@ impl DirectAssetAccessExt for World { fn load_asset<'a, A: Asset>(&self, path: impl Into>) -> Handle { self.resource::().load(path) } - /// Load an asset with settings, similarly to [`AssetServer::load_with_settings`]. - /// - /// # Panics - /// If `self` doesn't have an [`AssetServer`] resource initialized yet. - fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( - &self, - path: impl Into>, - settings: S, - ) -> Handle { - self.resource::() - .load_with_settings(path, settings) - } } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index abfc67e5def84..fbe7c6ffddf3b 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -9,7 +9,7 @@ use crate::{ MissingProcessedAssetReaderError, Reader, }, loader::{AssetLoader, ErasedAssetLoader, LoadContext, LoadedAsset}, - meta::{AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal, Settings}, + meta::{AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal}, path::AssetPath, Asset, AssetEvent, AssetHandleProvider, AssetId, AssetLoadFailedEvent, AssetMetaCheck, Assets, DeserializeMetaError, ErasedLoadedAsset, Handle, LoadedUntypedAsset, UnapprovedPathMode, @@ -368,74 +368,6 @@ impl AssetServer { self.load_with_meta_transform(path, guard, true) } - /// Begins loading an [`Asset`] of type `A` stored at `path`. The given `settings` function will override the asset's - /// [`AssetLoader`] settings. The type `S` _must_ match the configured [`AssetLoader::Settings`] or `settings` changes - /// will be ignored and an error will be printed to the log. - #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] - pub fn load_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( - &self, - path: impl Into>, - settings: S, - ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), (), false) - } - - /// Same as [`load`](AssetServer::load_with_settings), but you can load assets from unaproved paths - /// if [`AssetPlugin::unapproved_path_mode`](super::AssetPlugin::unapproved_path_mode) - /// is [`Deny`](UnapprovedPathMode::Deny). - /// - /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`] - pub fn load_with_settings_override<'a, A: Asset, S: Settings + serde::Serialize>( - &self, - path: impl Into>, - settings: S, - ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), (), true) - } - - /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item. - /// The guard item is dropped when either the asset is loaded or loading has failed. - /// - /// This function only guarantees the asset referenced by the [`Handle`] is loaded. If your asset is separated into - /// multiple files, sub-assets referenced by the main asset might still be loading, depend on the implementation of the [`AssetLoader`]. - /// - /// The given `settings` function will override the asset's - /// [`AssetLoader`] settings. The type `S` _must_ match the configured [`AssetLoader::Settings`] or `settings` changes - /// will be ignored and an error will be printed to the log. - #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] - pub fn load_acquire_with_settings< - 'a, - A: Asset, - S: Settings + serde::Serialize, - G: Send + Sync + 'static, - >( - &self, - path: impl Into>, - settings: S, - guard: G, - ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), guard, false) - } - - /// Same as [`load`](AssetServer::load_acquire_with_settings), but you can load assets from unaproved paths - /// if [`AssetPlugin::unapproved_path_mode`](super::AssetPlugin::unapproved_path_mode) - /// is [`Deny`](UnapprovedPathMode::Deny). - /// - /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`] - pub fn load_acquire_with_settings_override< - 'a, - A: Asset, - S: Settings + serde::Serialize, - G: Send + Sync + 'static, - >( - &self, - path: impl Into>, - settings: S, - guard: G, - ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), guard, true) - } - pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>( &self, path: impl Into>, diff --git a/examples/2d/mesh2d_repeated_texture.rs b/examples/2d/mesh2d_repeated_texture.rs index 8436b221f8093..6d9b6fba2f400 100644 --- a/examples/2d/mesh2d_repeated_texture.rs +++ b/examples/2d/mesh2d_repeated_texture.rs @@ -7,6 +7,7 @@ use bevy::{ math::Affine2, prelude::*, }; +use bevy_asset::AssetPath; /// How much to move some rectangles away from the center const RECTANGLE_OFFSET: f32 = 250.0; @@ -32,17 +33,18 @@ fn setup( // settings let image_with_default_sampler = asset_server.load("textures/fantasy_ui_borders/panel-border-010.png"); - let image_with_repeated_sampler = asset_server.load_with_settings( - "textures/fantasy_ui_borders/panel-border-010-repeated.png", - ImageLoaderSettings { - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - // rewriting mode to repeat image, - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, + let image_with_repeated_sampler = asset_server.load( + AssetPath::from("textures/fantasy_ui_borders/panel-border-010-repeated.png").with_settings( + ImageLoaderSettings { + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + // rewriting mode to repeat image, + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, + ..default() + }), ..default() - }), - ..default() - }, + }, + ), ); // central rectangle with not repeated texture diff --git a/examples/3d/deferred_rendering.rs b/examples/3d/deferred_rendering.rs index c709c170ad090..82f402c5a0b3f 100644 --- a/examples/3d/deferred_rendering.rs +++ b/examples/3d/deferred_rendering.rs @@ -13,6 +13,7 @@ use bevy::{ }, prelude::*, }; +use bevy_asset::AssetPath; fn main() { App::new() @@ -222,14 +223,15 @@ fn setup_parallax( // The normal map. Note that to generate it in the GIMP image editor, you should // open the depth map, and do Filters → Generic → Normal Map // You should enable the "flip X" checkbox. - let normal_handle = asset_server.load_with_settings( - "textures/parallax_example/cube_normal.png", - // The normal map texture is in linear color space. Lighting won't look correct - // if `is_srgb` is `true`, which is the default. - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, + let normal_handle = asset_server.load( + AssetPath::from("textures/parallax_example/cube_normal.png").with_settings( + // The normal map texture is in linear color space. Lighting won't look correct + // if `is_srgb` is `true`, which is the default. + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, + ), ); let mut cube = Mesh::from(Cuboid::new(0.15, 0.15, 0.15)); diff --git a/examples/3d/parallax_mapping.rs b/examples/3d/parallax_mapping.rs index f3a24a434c637..41639e6fe2bb5 100644 --- a/examples/3d/parallax_mapping.rs +++ b/examples/3d/parallax_mapping.rs @@ -4,6 +4,7 @@ use std::fmt; use bevy::{image::ImageLoaderSettings, math::ops, prelude::*}; +use bevy_asset::AssetPath; fn main() { App::new() @@ -204,14 +205,15 @@ fn setup( // The normal map. Note that to generate it in the GIMP image editor, you should // open the depth map, and do Filters → Generic → Normal Map // You should enable the "flip X" checkbox. - let normal_handle = asset_server.load_with_settings( - "textures/parallax_example/cube_normal.png", - // The normal map texture is in linear color space. Lighting won't look correct - // if `is_srgb` is `true`, which is the default. - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, + let normal_handle = asset_server.load( + AssetPath::from("textures/parallax_example/cube_normal.png").with_settings( + // The normal map texture is in linear color space. Lighting won't look correct + // if `is_srgb` is `true`, which is the default. + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, + ), ); // Camera diff --git a/examples/3d/scrolling_fog.rs b/examples/3d/scrolling_fog.rs index e42e3d3773fa1..0b276397455a8 100644 --- a/examples/3d/scrolling_fog.rs +++ b/examples/3d/scrolling_fog.rs @@ -20,6 +20,7 @@ use bevy::{ pbr::{DirectionalLightShadowMap, FogVolume, VolumetricFog, VolumetricLight}, prelude::*, }; +use bevy_asset::AssetPath; /// Initializes the example. fn main() { @@ -94,8 +95,7 @@ fn setup( // Load a repeating 3d noise texture. Make sure to set ImageAddressMode to Repeat // so that the texture wraps around as the density texture offset is moved along. // Also set ImageFilterMode to Linear so that the fog isn't pixelated. - let noise_texture = assets.load_with_settings( - "volumes/fog_noise.ktx2", + let noise_texture = assets.load(AssetPath::from("volumes/fog_noise.ktx2").with_settings( ImageLoaderSettings { sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: ImageAddressMode::Repeat, @@ -108,7 +108,7 @@ fn setup( }), ..default() }, - ); + )); // Spawn a FogVolume and use the repeating noise texture as its density texture. commands.spawn(( diff --git a/examples/3d/ssr.rs b/examples/3d/ssr.rs index 61148b2c5d126..1f4db0f5c0d60 100644 --- a/examples/3d/ssr.rs +++ b/examples/3d/ssr.rs @@ -4,6 +4,7 @@ use std::ops::Range; use bevy::{ anti_aliasing::fxaa::Fxaa, + asset::AssetPath, color::palettes::css::{BLACK, WHITE}, core_pipeline::Skybox, image::{ @@ -188,19 +189,20 @@ fn spawn_water( ..default() }, extension: Water { - normals: asset_server.load_with_settings::( - "textures/water_normals.png", - ImageLoaderSettings { - is_srgb: false, - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, - mag_filter: ImageFilterMode::Linear, - min_filter: ImageFilterMode::Linear, - ..default() - }), - ..Default::default() - }, + normals: asset_server.load::( + AssetPath::from("textures/water_normals.png").with_settings( + ImageLoaderSettings { + is_srgb: false, + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, + mag_filter: ImageFilterMode::Linear, + min_filter: ImageFilterMode::Linear, + ..default() + }), + ..Default::default() + }, + ), ), // These water settings are just random values to create some // variety. diff --git a/examples/asset/alter_mesh.rs b/examples/asset/alter_mesh.rs index fe1ea5108d722..e2cfcaccafc80 100644 --- a/examples/asset/alter_mesh.rs +++ b/examples/asset/alter_mesh.rs @@ -57,7 +57,7 @@ fn setup( // In normal use, you can call `asset_server.load`, however see below for an explanation of // `RenderAssetUsages`. - let left_shape_model = asset_server.load_with_settings( + let left_shape_model = asset_server.load( GltfAssetLabel::Primitive { mesh: 0, // This field stores an index to this primitive in its parent mesh. In this case, we @@ -68,7 +68,7 @@ fn setup( // which accomplishes the same thing. primitive: 0, } - .from_asset(left_shape.get_model_path()), + .from_asset(left_shape.get_model_path()) // `RenderAssetUsages::all()` is already the default, so the line below could be omitted. // It's helpful to know it exists, however. // @@ -83,10 +83,10 @@ fn setup( // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the mesh in // RAM. For this example however, this would not work, as we need to inspect and modify the // mesh at runtime. - GltfLoaderSettings { + .with_settings(GltfLoaderSettings { load_meshes: RenderAssetUsages::all(), ..Default::default() - }, + }), ); // Here, we rely on the default loader settings to achieve a similar result to the above. diff --git a/examples/asset/alter_sprite.rs b/examples/asset/alter_sprite.rs index c1d6736ed97d2..7fa4b948a1e29 100644 --- a/examples/asset/alter_sprite.rs +++ b/examples/asset/alter_sprite.rs @@ -4,6 +4,7 @@ use bevy::{ image::ImageLoaderSettings, input::common_conditions::input_just_pressed, prelude::*, render::render_asset::RenderAssetUsages, }; +use bevy_asset::AssetPath; fn main() { App::new() @@ -50,26 +51,26 @@ fn setup(mut commands: Commands, asset_server: Res) { let bird_right = Bird::Normal; commands.spawn(Camera2d); - let texture_left = asset_server.load_with_settings( - bird_left.get_texture_path(), - // `RenderAssetUsages::all()` is already the default, so the line below could be omitted. - // It's helpful to know it exists, however. - // - // `RenderAssetUsages` tell Bevy whether to keep the data around: - // - for the GPU (`RenderAssetUsages::RENDER_WORLD`), - // - for the CPU (`RenderAssetUsages::MAIN_WORLD`), - // - or both. - // `RENDER_WORLD` is necessary to render the image, `MAIN_WORLD` is necessary to inspect - // and modify the image (via `ResMut>`). - // - // Since most games will not need to modify textures at runtime, many developers opt to pass - // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the image in - // RAM. For this example however, this would not work, as we need to inspect and modify the - // image at runtime. - ImageLoaderSettings { - asset_usage: RenderAssetUsages::all(), - ..Default::default() - }, + let texture_left = asset_server.load( + AssetPath::from(bird_left.get_texture_path()) + // `RenderAssetUsages::all()` is already the default, so the line below could be omitted. + // It's helpful to know it exists, however. + // + // `RenderAssetUsages` tell Bevy whether to keep the data around: + // - for the GPU (`RenderAssetUsages::RENDER_WORLD`), + // - for the CPU (`RenderAssetUsages::MAIN_WORLD`), + // - or both. + // `RENDER_WORLD` is necessary to render the image, `MAIN_WORLD` is necessary to inspect + // and modify the image (via `ResMut>`). + // + // Since most games will not need to modify textures at runtime, many developers opt to pass + // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the image in + // RAM. For this example however, this would not work, as we need to inspect and modify the + // image at runtime. + .with_settings(ImageLoaderSettings { + asset_usage: RenderAssetUsages::all(), + ..Default::default() + }), ); commands.spawn(( diff --git a/examples/asset/asset_settings.rs b/examples/asset/asset_settings.rs index c8cf01909bec5..7ea9c1ce2feba 100644 --- a/examples/asset/asset_settings.rs +++ b/examples/asset/asset_settings.rs @@ -4,6 +4,7 @@ use bevy::{ image::{ImageLoaderSettings, ImageSampler}, prelude::*, }; +use bevy_asset::AssetPath; fn main() { App::new() @@ -62,12 +63,13 @@ fn setup(mut commands: Commands, asset_server: Res) { // same one as without a .meta file. commands.spawn(( Sprite { - image: asset_server.load_with_settings( - "bevy_pixel_dark_with_settings.png", - ImageLoaderSettings { - sampler: ImageSampler::nearest(), - ..Default::default() - }, + image: asset_server.load( + AssetPath::from("bevy_pixel_dark_with_settings.png").with_settings( + ImageLoaderSettings { + sampler: ImageSampler::nearest(), + ..Default::default() + }, + ), ), custom_size: Some(Vec2 { x: 160.0, y: 120.0 }), ..Default::default() diff --git a/examples/asset/repeated_texture.rs b/examples/asset/repeated_texture.rs index a39f8755c4033..3118af6320743 100644 --- a/examples/asset/repeated_texture.rs +++ b/examples/asset/repeated_texture.rs @@ -6,6 +6,7 @@ use bevy::{ math::Affine2, prelude::*, }; +use bevy_asset::AssetPath; fn main() { App::new() @@ -36,25 +37,31 @@ fn setup( // left cube with repeated texture commands.spawn(( Mesh3d(meshes.add(Cuboid::new(1.0, 1.0, 1.0))), - MeshMaterial3d(materials.add(StandardMaterial { - base_color_texture: Some(asset_server.load_with_settings( - "textures/fantasy_ui_borders/panel-border-010-repeated.png", - ImageLoaderSettings { - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - // rewriting mode to repeat image, - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, - ..default() - }), - ..default() - }, - )), + MeshMaterial3d( + materials.add(StandardMaterial { + base_color_texture: Some( + asset_server.load( + AssetPath::from( + "textures/fantasy_ui_borders/panel-border-010-repeated.png", + ) + .with_settings(ImageLoaderSettings { + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + // rewriting mode to repeat image, + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, + ..default() + }), + ..default() + }), + ), + ), - // uv_transform used here for proportions only, but it is full Affine2 - // that's why you can use rotation and shift also - uv_transform: Affine2::from_scale(Vec2::new(2., 3.)), - ..default() - })), + // uv_transform used here for proportions only, but it is full Affine2 + // that's why you can use rotation and shift also + uv_transform: Affine2::from_scale(Vec2::new(2., 3.)), + ..default() + }), + ), Transform::from_xyz(-1.5, 0.0, 0.0), )); diff --git a/examples/ui/ui_texture_slice_flip_and_tile.rs b/examples/ui/ui_texture_slice_flip_and_tile.rs index 7977f52f1a417..c0b9dc1637668 100644 --- a/examples/ui/ui_texture_slice_flip_and_tile.rs +++ b/examples/ui/ui_texture_slice_flip_and_tile.rs @@ -6,6 +6,7 @@ use bevy::{ ui::widget::NodeImageMode, winit::WinitSettings, }; +use bevy_asset::AssetPath; fn main() { App::new() @@ -18,13 +19,14 @@ fn main() { } fn setup(mut commands: Commands, asset_server: Res) { - let image = asset_server.load_with_settings( - "textures/fantasy_ui_borders/numbered_slices.png", - ImageLoaderSettings { - // Need to use nearest filtering to avoid bleeding between the slices with tiling - sampler: ImageSampler::nearest(), - ..Default::default() - }, + let image = asset_server.load( + AssetPath::from("textures/fantasy_ui_borders/numbered_slices.png").with_settings( + ImageLoaderSettings { + // Need to use nearest filtering to avoid bleeding between the slices with tiling + sampler: ImageSampler::nearest(), + ..Default::default() + }, + ), ); let slicer = TextureSlicer { From 11e6762dfae82b18915f95d8a00791dad1e74483 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Fri, 9 May 2025 13:06:27 +0100 Subject: [PATCH 07/32] Fixed `ExrTextureLoadingSettings` missing clone. --- crates/bevy_image/src/exr_texture_loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_image/src/exr_texture_loader.rs b/crates/bevy_image/src/exr_texture_loader.rs index d9b89dd21e152..931ffee965c82 100644 --- a/crates/bevy_image/src/exr_texture_loader.rs +++ b/crates/bevy_image/src/exr_texture_loader.rs @@ -10,7 +10,7 @@ use wgpu_types::{Extent3d, TextureDimension, TextureFormat}; #[cfg(feature = "exr")] pub struct ExrTextureLoader; -#[derive(Serialize, Deserialize, Default, Debug)] +#[derive(Serialize, Deserialize, Default, Debug, Clone)] #[cfg(feature = "exr")] pub struct ExrTextureLoaderSettings { pub asset_usage: RenderAssetUsages, From feb7aba5e9d41623c4447b536a73222b6ac198b2 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Fri, 9 May 2025 13:06:53 +0100 Subject: [PATCH 08/32] Fixed remaining examples. --- examples/3d/clearcoat.rs | 21 +++++++++++++------ examples/3d/rotate_environment_map.rs | 11 +++++++--- examples/asset/processing/asset_processing.rs | 4 +--- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/examples/3d/clearcoat.rs b/examples/3d/clearcoat.rs index c04b12d5814ed..9c9d37ab3bcf0 100644 --- a/examples/3d/clearcoat.rs +++ b/examples/3d/clearcoat.rs @@ -26,6 +26,7 @@ use bevy::{ math::vec3, prelude::*, }; +use bevy_asset::AssetPath; /// The size of each sphere. const SPHERE_SCALE: f32 = 0.9; @@ -101,9 +102,13 @@ fn spawn_car_paint_sphere( MeshMaterial3d(materials.add(StandardMaterial { clearcoat: 1.0, clearcoat_perceptual_roughness: 0.1, - normal_map_texture: Some(asset_server.load_with_settings( - "textures/BlueNoise-Normal.png", - |settings: &mut ImageLoaderSettings| settings.is_srgb = false, + normal_map_texture: Some(asset_server.load( + AssetPath::from("textures/BlueNoise-Normal.png").with_settings( + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, + ), )), metallic: 0.9, perceptual_roughness: 0.5, @@ -167,9 +172,13 @@ fn spawn_scratched_gold_ball( MeshMaterial3d(materials.add(StandardMaterial { clearcoat: 1.0, clearcoat_perceptual_roughness: 0.3, - clearcoat_normal_texture: Some(asset_server.load_with_settings( - "textures/ScratchedGold-Normal.png", - |settings: &mut ImageLoaderSettings| settings.is_srgb = false, + clearcoat_normal_texture: Some(asset_server.load( + AssetPath::from("textures/ScratchedGold-Normal.png").with_settings( + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, + ), )), metallic: 0.9, perceptual_roughness: 0.1, diff --git a/examples/3d/rotate_environment_map.rs b/examples/3d/rotate_environment_map.rs index 68ecceeccf7de..2dc1d41e69316 100644 --- a/examples/3d/rotate_environment_map.rs +++ b/examples/3d/rotate_environment_map.rs @@ -8,6 +8,7 @@ use bevy::{ image::ImageLoaderSettings, prelude::*, }; +use bevy_asset::AssetPath; /// Entry point. pub fn main() { @@ -68,9 +69,13 @@ fn spawn_sphere( MeshMaterial3d(materials.add(StandardMaterial { clearcoat: 1.0, clearcoat_perceptual_roughness: 0.3, - clearcoat_normal_texture: Some(asset_server.load_with_settings( - "textures/ScratchedGold-Normal.png", - |settings: &mut ImageLoaderSettings| settings.is_srgb = false, + clearcoat_normal_texture: Some(asset_server.load( + AssetPath::from("textures/ScratchedGold-Normal.png").with_settings( + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, + ), )), metallic: 0.9, perceptual_roughness: 0.1, diff --git a/examples/asset/processing/asset_processing.rs b/examples/asset/processing/asset_processing.rs index 47a12fc6bb202..36ac24b02fc45 100644 --- a/examples/asset/processing/asset_processing.rs +++ b/examples/asset/processing/asset_processing.rs @@ -159,9 +159,7 @@ impl AssetLoader for CoolTextLoader { for (path, settings_override) in ron.dependencies_with_settings { let loaded = load_context .loader() - .with_settings(move |settings| { - *settings = settings_override.clone(); - }) + .with_settings(settings_override) .immediate() .load::(&path) .await?; From 9bf90b489d1bc7e63370393688c33b9974b53331 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Fri, 9 May 2025 16:28:17 +0100 Subject: [PATCH 09/32] Changed glTF importer to calculate the texture handles once and reuse them when loading materials. This fixes materials using the wrong handles - it was recalculating the handles without the settings. But it might break some dependency handling - see the comment where `texture_handles` is defined. --- .../extensions/khr_materials_anisotropy.rs | 10 ++- .../extensions/khr_materials_clearcoat.rs | 17 +++-- .../extensions/khr_materials_specular.rs | 15 ++--- .../bevy_gltf/src/loader/gltf_ext/material.rs | 10 ++- .../bevy_gltf/src/loader/gltf_ext/texture.rs | 37 +++-------- crates/bevy_gltf/src/loader/mod.rs | 63 ++++++++++++------- 6 files changed, 72 insertions(+), 80 deletions(-) diff --git a/crates/bevy_gltf/src/loader/extensions/khr_materials_anisotropy.rs b/crates/bevy_gltf/src/loader/extensions/khr_materials_anisotropy.rs index f859cfba84052..e85fc9a0074f1 100644 --- a/crates/bevy_gltf/src/loader/extensions/khr_materials_anisotropy.rs +++ b/crates/bevy_gltf/src/loader/extensions/khr_materials_anisotropy.rs @@ -1,5 +1,5 @@ -use bevy_asset::LoadContext; - +use bevy_asset::Handle; +use bevy_image::Image; use gltf::{Document, Material}; use serde_json::Value; @@ -7,8 +7,6 @@ use serde_json::Value; #[cfg(feature = "pbr_anisotropy_texture")] use { crate::loader::gltf_ext::{material::uv_channel, texture::texture_handle_from_info}, - bevy_asset::Handle, - bevy_image::Image, bevy_pbr::UvChannel, gltf::json::texture::Info, serde_json::value, @@ -38,7 +36,7 @@ impl AnisotropyExtension { reason = "Depending on what features are used to compile this crate, certain parameters may end up unused." )] pub(crate) fn parse( - load_context: &mut LoadContext, + _texture_handles: &[Handle], document: &Document, material: &Material, ) -> Option { @@ -54,7 +52,7 @@ impl AnisotropyExtension { .map(|json_info| { ( uv_channel(material, "anisotropy", json_info.tex_coord), - texture_handle_from_info(&json_info, document, load_context), + texture_handle_from_info(&json_info, document, _texture_handles), ) }) .unzip(); diff --git a/crates/bevy_gltf/src/loader/extensions/khr_materials_clearcoat.rs b/crates/bevy_gltf/src/loader/extensions/khr_materials_clearcoat.rs index 5128487ca4445..fc76a7a0a5334 100644 --- a/crates/bevy_gltf/src/loader/extensions/khr_materials_clearcoat.rs +++ b/crates/bevy_gltf/src/loader/extensions/khr_materials_clearcoat.rs @@ -1,14 +1,11 @@ -use bevy_asset::LoadContext; - +use bevy_asset::Handle; +use bevy_image::Image; use gltf::{Document, Material}; use serde_json::Value; #[cfg(feature = "pbr_multi_layer_material_textures")] -use { - crate::loader::gltf_ext::material::parse_material_extension_texture, bevy_asset::Handle, - bevy_image::Image, bevy_pbr::UvChannel, -}; +use {crate::loader::gltf_ext::material::parse_material_extension_texture, bevy_pbr::UvChannel}; /// Parsed data from the `KHR_materials_clearcoat` extension. /// @@ -42,7 +39,7 @@ impl ClearcoatExtension { reason = "Depending on what features are used to compile this crate, certain parameters may end up unused." )] pub(crate) fn parse( - load_context: &mut LoadContext, + _texture_handles: &[Handle], document: &Document, material: &Material, ) -> Option { @@ -54,7 +51,7 @@ impl ClearcoatExtension { #[cfg(feature = "pbr_multi_layer_material_textures")] let (clearcoat_channel, clearcoat_texture) = parse_material_extension_texture( material, - load_context, + _texture_handles, document, extension, "clearcoatTexture", @@ -65,7 +62,7 @@ impl ClearcoatExtension { let (clearcoat_roughness_channel, clearcoat_roughness_texture) = parse_material_extension_texture( material, - load_context, + _texture_handles, document, extension, "clearcoatRoughnessTexture", @@ -75,7 +72,7 @@ impl ClearcoatExtension { #[cfg(feature = "pbr_multi_layer_material_textures")] let (clearcoat_normal_channel, clearcoat_normal_texture) = parse_material_extension_texture( material, - load_context, + _texture_handles, document, extension, "clearcoatNormalTexture", diff --git a/crates/bevy_gltf/src/loader/extensions/khr_materials_specular.rs b/crates/bevy_gltf/src/loader/extensions/khr_materials_specular.rs index f0adcc4940b10..1287e4366c5c7 100644 --- a/crates/bevy_gltf/src/loader/extensions/khr_materials_specular.rs +++ b/crates/bevy_gltf/src/loader/extensions/khr_materials_specular.rs @@ -1,14 +1,11 @@ -use bevy_asset::LoadContext; - +use bevy_asset::Handle; +use bevy_image::Image; use gltf::{Document, Material}; use serde_json::Value; #[cfg(feature = "pbr_specular_textures")] -use { - crate::loader::gltf_ext::material::parse_material_extension_texture, bevy_asset::Handle, - bevy_image::Image, bevy_pbr::UvChannel, -}; +use {crate::loader::gltf_ext::material::parse_material_extension_texture, bevy_pbr::UvChannel}; /// Parsed data from the `KHR_materials_specular` extension. /// @@ -42,7 +39,7 @@ pub(crate) struct SpecularExtension { impl SpecularExtension { pub(crate) fn parse( - _load_context: &mut LoadContext, + _texture_handles: &[Handle], _document: &Document, material: &Material, ) -> Option { @@ -54,7 +51,7 @@ impl SpecularExtension { #[cfg(feature = "pbr_specular_textures")] let (_specular_channel, _specular_texture) = parse_material_extension_texture( material, - _load_context, + _texture_handles, _document, extension, "specularTexture", @@ -64,7 +61,7 @@ impl SpecularExtension { #[cfg(feature = "pbr_specular_textures")] let (_specular_color_channel, _specular_color_texture) = parse_material_extension_texture( material, - _load_context, + _texture_handles, _document, extension, "specularColorTexture", diff --git a/crates/bevy_gltf/src/loader/gltf_ext/material.rs b/crates/bevy_gltf/src/loader/gltf_ext/material.rs index 9d8b7c5745910..ada870cd154d0 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/material.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/material.rs @@ -16,7 +16,7 @@ use super::texture::texture_transform_to_affine2; ))] use { super::texture::texture_handle_from_info, - bevy_asset::{Handle, LoadContext}, + bevy_asset::Handle, bevy_image::Image, gltf::Document, serde_json::{Map, Value}, @@ -30,7 +30,7 @@ use { ))] pub(crate) fn parse_material_extension_texture( material: &Material, - load_context: &mut LoadContext, + texture_handles: &[Handle], document: &Document, extension: &Map, texture_name: &str, @@ -42,7 +42,11 @@ pub(crate) fn parse_material_extension_texture( { Some(json_info) => ( uv_channel(material, texture_kind, json_info.tex_coord), - Some(texture_handle_from_info(&json_info, document, load_context)), + Some(texture_handle_from_info( + &json_info, + document, + texture_handles, + )), ), None => (UvChannel::default(), None), } diff --git a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs index f666752479bb6..0b0414f1fdc98 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs @@ -1,11 +1,8 @@ -use bevy_asset::{Handle, LoadContext}; +use bevy_asset::Handle; use bevy_image::{Image, ImageAddressMode, ImageFilterMode, ImageSamplerDescriptor}; use bevy_math::Affine2; -use gltf::{ - image::Source, - texture::{MagFilter, MinFilter, Texture, TextureTransform, WrappingMode}, -}; +use gltf::texture::{MagFilter, MinFilter, Texture, TextureTransform, WrappingMode}; #[cfg(any( feature = "pbr_anisotropy_texture", @@ -14,28 +11,12 @@ use gltf::{ ))] use gltf::{json::texture::Info, Document}; -use crate::{loader::DataUri, GltfAssetLabel}; - pub(crate) fn texture_handle( texture: &Texture<'_>, - load_context: &mut LoadContext, + texture_handles: &[Handle], ) -> Handle { - match texture.source().source() { - Source::View { .. } => load_context.get_label_handle(texture_label(texture).to_string()), - Source::Uri { uri, .. } => { - let uri = percent_encoding::percent_decode_str(uri) - .decode_utf8() - .unwrap(); - let uri = uri.as_ref(); - if let Ok(_data_uri) = DataUri::parse(uri) { - load_context.get_label_handle(texture_label(texture).to_string()) - } else { - let parent = load_context.path().parent().unwrap(); - let image_path = parent.join(uri); - load_context.load(image_path) - } - } - } + // XXX TODO: Handle out of range request? + texture_handles[texture.index()].clone() } /// Extracts the texture sampler data from the glTF [`Texture`]. @@ -83,10 +64,6 @@ pub(crate) fn texture_sampler( sampler } -pub(crate) fn texture_label(texture: &Texture<'_>) -> GltfAssetLabel { - GltfAssetLabel::Texture(texture.index()) -} - pub(crate) fn address_mode(wrapping_mode: &WrappingMode) -> ImageAddressMode { match wrapping_mode { WrappingMode::ClampToEdge => ImageAddressMode::ClampToEdge, @@ -116,11 +93,11 @@ pub(crate) fn texture_transform_to_affine2(texture_transform: TextureTransform) pub(crate) fn texture_handle_from_info( info: &Info, document: &Document, - load_context: &mut LoadContext, + texture_handles: &[Handle], ) -> Handle { let texture = document .textures() .nth(info.index.value()) .expect("Texture info references a nonexistent texture"); - texture_handle(&texture, load_context) + texture_handle(&texture, texture_handles) } diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index 8e0c763ece464..f87b6cf4ad1c9 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -159,14 +159,14 @@ pub struct GltfLoader { /// /// To load a gltf but exclude the cameras, replace a call to `asset_server.load("my.gltf")` with /// ```no_run -/// # use bevy_asset::{AssetServer, Handle}; +/// # use bevy_asset::{AssetPath, AssetServer, Handle}; /// # use bevy_gltf::*; /// # let asset_server: AssetServer = panic!(); -/// let gltf_handle: Handle = asset_server.load_with_settings( -/// "my.gltf", -/// |s: &mut GltfLoaderSettings| { -/// s.load_cameras = false; -/// } +/// let gltf_handle: Handle = asset_server.load( +/// AssetPath::from("my.gltf").with_settings(GltfLoaderSettings { +/// load_cameras: false, +/// ..Default::default() +/// }) /// ); /// ``` #[derive(Serialize, Deserialize, Clone)] @@ -528,7 +528,9 @@ async fn load_gltf<'a, 'b, 'c>( // In theory we could store a mapping between texture.index() and handle to use // later in the loader when looking up handles for materials. However this would mean // that the material's load context would no longer track those images as dependencies. - let mut _texture_handles = Vec::new(); + // + // XXX TODO: This has been changed to use texture handles. But the comment above suggests that will break something. + let mut texture_handles = Vec::new(); if gltf.textures().len() == 1 || cfg!(target_arch = "wasm32") { for texture in gltf.textures() { let parent_path = load_context.path().parent().unwrap(); @@ -542,7 +544,7 @@ async fn load_gltf<'a, 'b, 'c>( settings, ) .await?; - image.process_loaded_texture(load_context, &mut _texture_handles); + image.process_loaded_texture(load_context, &mut texture_handles); } } else { #[cfg(not(target_arch = "wasm32"))] @@ -569,7 +571,7 @@ async fn load_gltf<'a, 'b, 'c>( .into_iter() .for_each(|result| match result { Ok(image) => { - image.process_loaded_texture(load_context, &mut _texture_handles); + image.process_loaded_texture(load_context, &mut texture_handles); } Err(err) => { warn!("Error loading glTF texture: {}", err); @@ -583,7 +585,13 @@ async fn load_gltf<'a, 'b, 'c>( if !settings.load_materials.is_empty() { // NOTE: materials must be loaded after textures because image load() calls will happen before load_with_settings, preventing is_srgb from being set properly for material in gltf.materials() { - let handle = load_material(&material, load_context, &gltf.document, false); + let handle = load_material( + &material, + load_context, + &gltf.document, + false, + &texture_handles, + ); if let Some(name) = material.name() { named_materials.insert(name.into(), handle.clone()); } @@ -885,6 +893,7 @@ async fn load_gltf<'a, 'b, 'c>( #[cfg(feature = "bevy_animation")] None, &gltf.document, + &texture_handles, ); if result.is_err() { err = Some(result); @@ -1041,9 +1050,11 @@ fn load_material( load_context: &mut LoadContext, document: &Document, is_scale_inverted: bool, + texture_handles: &[Handle], ) -> Handle { let material_label = material_label(material, is_scale_inverted); - load_context.labeled_asset_scope(material_label.to_string(), |load_context| { + // XXX TODO: Unclear if this is needed assume we follow through on using texture handles? + load_context.labeled_asset_scope(material_label.to_string(), |_| { let pbr = material.pbr_metallic_roughness(); // TODO: handle missing label handle errors here? @@ -1054,7 +1065,7 @@ fn load_material( .unwrap_or_default(); let base_color_texture = pbr .base_color_texture() - .map(|info| texture_handle(&info.texture(), load_context)); + .map(|info| texture_handle(&info.texture(), texture_handles)); let uv_transform = pbr .base_color_texture() @@ -1068,7 +1079,7 @@ fn load_material( let normal_map_texture: Option> = material.normal_texture().map(|normal_texture| { // TODO: handle normal_texture.scale - texture_handle(&normal_texture.texture(), load_context) + texture_handle(&normal_texture.texture(), texture_handles) }); let metallic_roughness_channel = pbr @@ -1082,7 +1093,7 @@ fn load_material( uv_transform, "metallic/roughness", ); - texture_handle(&info.texture(), load_context) + texture_handle(&info.texture(), texture_handles) }); let occlusion_channel = material @@ -1091,7 +1102,7 @@ fn load_material( .unwrap_or_default(); let occlusion_texture = material.occlusion_texture().map(|occlusion_texture| { // TODO: handle occlusion_texture.strength() (a scalar multiplier for occlusion strength) - texture_handle(&occlusion_texture.texture(), load_context) + texture_handle(&occlusion_texture.texture(), texture_handles) }); let emissive = material.emissive_factor(); @@ -1102,7 +1113,7 @@ fn load_material( let emissive_texture = material.emissive_texture().map(|info| { // TODO: handle occlusion_texture.strength() (a scalar multiplier for occlusion strength) warn_on_differing_texture_transforms(material, &info, uv_transform, "emissive"); - texture_handle(&info.texture(), load_context) + texture_handle(&info.texture(), texture_handles) }); #[cfg(feature = "pbr_transmission_textures")] @@ -1117,7 +1128,7 @@ fn load_material( let transmission_texture: Option> = transmission .transmission_texture() .map(|transmission_texture| { - texture_handle(&transmission_texture.texture(), load_context) + texture_handle(&transmission_texture.texture(), texture_handles) }); ( @@ -1148,7 +1159,7 @@ fn load_material( .unwrap_or_default(); let thickness_texture: Option> = volume.thickness_texture().map(|thickness_texture| { - texture_handle(&thickness_texture.texture(), load_context) + texture_handle(&thickness_texture.texture(), texture_handles) }); ( @@ -1177,15 +1188,15 @@ fn load_material( // Parse the `KHR_materials_clearcoat` extension data if necessary. let clearcoat = - ClearcoatExtension::parse(load_context, document, material).unwrap_or_default(); + ClearcoatExtension::parse(texture_handles, document, material).unwrap_or_default(); // Parse the `KHR_materials_anisotropy` extension data if necessary. let anisotropy = - AnisotropyExtension::parse(load_context, document, material).unwrap_or_default(); + AnisotropyExtension::parse(texture_handles, document, material).unwrap_or_default(); // Parse the `KHR_materials_specular` extension data if necessary. let specular = - SpecularExtension::parse(load_context, document, material).unwrap_or_default(); + SpecularExtension::parse(texture_handles, document, material).unwrap_or_default(); // We need to operate in the Linear color space and be willing to exceed 1.0 in our channels let base_emissive = LinearRgba::rgb(emissive[0], emissive[1], emissive[2]); @@ -1296,6 +1307,7 @@ fn load_node( #[cfg(feature = "bevy_animation")] animation_roots: &HashSet, #[cfg(feature = "bevy_animation")] mut animation_context: Option, document: &Document, + texture_handles: &[Handle], ) -> Result<(), GltfError> { let mut gltf_error = None; let transform = node_transform(gltf_node); @@ -1404,7 +1416,13 @@ fn load_node( if !root_load_context.has_labeled_asset(&material_label) && !load_context.has_labeled_asset(&material_label) { - load_material(&material, load_context, document, is_scale_inverted); + load_material( + &material, + load_context, + document, + is_scale_inverted, + texture_handles, + ); } let primitive_label = GltfAssetLabel::Primitive { @@ -1562,6 +1580,7 @@ fn load_node( #[cfg(feature = "bevy_animation")] animation_context.clone(), document, + texture_handles, ) { gltf_error = Some(err); return; From 2287bc33ea055459429c44e1b07fdc4a46ee7e38 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Fri, 9 May 2025 16:35:48 +0100 Subject: [PATCH 10/32] Renamed `AssetPathSettings` to `ErasedSettings`. --- crates/bevy_asset/src/loader_builders.rs | 14 +++++----- crates/bevy_asset/src/path.rs | 35 ++++++++++++------------ 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/crates/bevy_asset/src/loader_builders.rs b/crates/bevy_asset/src/loader_builders.rs index 1a0b1575b94fc..c289551d741c5 100644 --- a/crates/bevy_asset/src/loader_builders.rs +++ b/crates/bevy_asset/src/loader_builders.rs @@ -2,8 +2,8 @@ //! [`LoadContext::loader`]. use crate::{ - io::Reader, meta::Settings, Asset, AssetLoadError, AssetPath, AssetPathSettings, - ErasedAssetLoader, ErasedLoadedAsset, Handle, LoadContext, LoadDirectError, LoadedAsset, + io::Reader, meta::Settings, Asset, AssetLoadError, AssetPath, ErasedAssetLoader, + ErasedLoadedAsset, ErasedSettings, Handle, LoadContext, LoadDirectError, LoadedAsset, LoadedUntypedAsset, UntypedHandle, }; use alloc::{borrow::ToOwned, boxed::Box, sync::Arc}; @@ -116,7 +116,7 @@ impl ReaderRef<'_> { /// [`LoadTransformAndSave`]: crate::processor::LoadTransformAndSave pub struct NestedLoader<'ctx, 'builder, T, M> { load_context: &'builder mut LoadContext<'ctx>, - settings: Option, + settings: Option, typing: T, mode: M, } @@ -181,7 +181,7 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui /// and the asset load will fail. #[must_use] pub fn with_settings(mut self, settings: S) -> Self { - self.settings = Some(AssetPathSettings::new(settings)); + self.settings = Some(ErasedSettings::new(settings)); self } @@ -285,7 +285,7 @@ impl NestedLoader<'_, '_, StaticTyped, Deferred> { /// [`with_dynamic_type`]: Self::with_dynamic_type /// [`with_unknown_type`]: Self::with_unknown_type pub fn load<'c, A: Asset>(self, path: impl Into>) -> Handle { - let path = path.into().with_settings_2(self.settings).to_owned(); + let path = path.into().with_erased_settings(self.settings).to_owned(); let handle = if self.load_context.should_load_dependencies { self.load_context .asset_server @@ -309,7 +309,7 @@ impl NestedLoader<'_, '_, DynamicTyped, Deferred> { /// /// [`with_dynamic_type`]: Self::with_dynamic_type pub fn load<'p>(self, path: impl Into>) -> UntypedHandle { - let path = path.into().with_settings_2(self.settings).to_owned(); + let path = path.into().with_erased_settings(self.settings).to_owned(); let handle = if self.load_context.should_load_dependencies { self.load_context .asset_server @@ -330,7 +330,7 @@ impl NestedLoader<'_, '_, UnknownTyped, Deferred> { /// /// This will infer the asset type from metadata. pub fn load<'p>(self, path: impl Into>) -> Handle { - let path = path.into().with_settings_2(self.settings).to_owned(); + let path = path.into().with_erased_settings(self.settings).to_owned(); let handle = if self.load_context.should_load_dependencies { self.load_context .asset_server diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 3bc4dd2b35f2b..b4146c2325ab4 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -19,58 +19,58 @@ use std::path::{Path, PathBuf}; use thiserror::Error; #[derive(Eq, PartialEq, Hash, Copy, Clone)] -pub struct AssetPathSettingsId { +pub struct ErasedSettingsId { // XXX TODO: Unclear if we want type id? Does make debugging a bit easier. type_id: TypeId, hash: u64, } -impl Display for AssetPathSettingsId { +impl Display for ErasedSettingsId { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { // XXX TODO: Reconsider formatting. Also we're using Debug for type_id... write!(f, "{:?}/{}", self.type_id, self.hash) } } -pub struct AssetPathSettings { +pub struct ErasedSettings { pub value: Box, - pub id: AssetPathSettingsId, + pub id: ErasedSettingsId, } -impl AssetPathSettings { - pub fn new(settings: S) -> AssetPathSettings { +impl ErasedSettings { + pub fn new(settings: S) -> ErasedSettings { // XXX TODO: Serializing to string is probably not the right solution. let string = ron::ser::to_string(&settings).unwrap(); // XXX TODO: Avoid unwrap. // XXX TODO: What's the appropriate hasher? // XXX TODO: Should we hash the type id as well? - let id = AssetPathSettingsId { + let id = ErasedSettingsId { type_id: TypeId::of::(), hash: FixedHasher.hash_one(&string), }; - AssetPathSettings { + ErasedSettings { value: Box::new(settings), id, } } } -impl Hash for AssetPathSettings { +impl Hash for ErasedSettings { fn hash(&self, state: &mut H) { self.id.hash(state); } } -impl PartialEq for AssetPathSettings { +impl PartialEq for ErasedSettings { fn eq(&self, other: &Self) -> bool { self.id == other.id } } -impl Eq for AssetPathSettings {} +impl Eq for ErasedSettings {} -impl Display for AssetPathSettings { +impl Display for ErasedSettings { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { self.id.fmt(f) } @@ -122,7 +122,7 @@ pub struct AssetPath<'a> { label: Option>, // XXX TODO: This is an Arc for now to simplify the implementation. Should // consider changing to CowArc. - settings: Option>, + settings: Option>, } impl<'a> Debug for AssetPath<'a> { @@ -141,7 +141,7 @@ impl<'a> Display for AssetPath<'a> { write!(f, "#{label}")?; } // XXX TODO: This might need a rethink as I'm not sure if the output - // needs to be parseable. Also see comments on AssetPathSettingsId::fmt. + // needs to be parseable. Also see comments on ErasedSettingsId::fmt. if let Some(settings) = &self.settings { write!(f, " (settings: {settings})")?; } @@ -332,7 +332,7 @@ impl<'a> AssetPath<'a> { /// XXX TODO: Docs. #[inline] - pub fn settings(&self) -> Option<&AssetPathSettings> { + pub fn settings(&self) -> Option<&ErasedSettings> { self.settings.as_deref() } @@ -389,13 +389,12 @@ impl<'a> AssetPath<'a> { source: self.source, path: self.path, label: self.label, - settings: Some(Arc::new(AssetPathSettings::new(settings))), + settings: Some(Arc::new(ErasedSettings::new(settings))), } } - // XXX TODO: Ugly duplication. #[inline] - pub fn with_settings_2(self, settings: Option) -> AssetPath<'a> { + pub fn with_erased_settings(self, settings: Option) -> AssetPath<'a> { AssetPath { source: self.source, path: self.path, From 765d9518a868306f0aa85f4dfa0fca92f69c6935 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Fri, 9 May 2025 17:23:17 +0100 Subject: [PATCH 11/32] Clarified comment. --- crates/bevy_asset/src/path.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index b4146c2325ab4..d2193c81a177c 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -121,7 +121,8 @@ pub struct AssetPath<'a> { path: CowArc<'a, Path>, label: Option>, // XXX TODO: This is an Arc for now to simplify the implementation. Should - // consider changing to CowArc. + // consider changing to CowArc, although I'm not sure if that actually has + // any benefits. settings: Option>, } From 529637e0b24335f80f552f235de8829ea92036bc Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 10:52:56 +0100 Subject: [PATCH 12/32] Revert "Fixed remaining examples." This reverts commit feb7aba5e9d41623c4447b536a73222b6ac198b2. --- examples/3d/clearcoat.rs | 21 ++++++------------- examples/3d/rotate_environment_map.rs | 11 +++------- examples/asset/processing/asset_processing.rs | 4 +++- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/examples/3d/clearcoat.rs b/examples/3d/clearcoat.rs index 9c9d37ab3bcf0..c04b12d5814ed 100644 --- a/examples/3d/clearcoat.rs +++ b/examples/3d/clearcoat.rs @@ -26,7 +26,6 @@ use bevy::{ math::vec3, prelude::*, }; -use bevy_asset::AssetPath; /// The size of each sphere. const SPHERE_SCALE: f32 = 0.9; @@ -102,13 +101,9 @@ fn spawn_car_paint_sphere( MeshMaterial3d(materials.add(StandardMaterial { clearcoat: 1.0, clearcoat_perceptual_roughness: 0.1, - normal_map_texture: Some(asset_server.load( - AssetPath::from("textures/BlueNoise-Normal.png").with_settings( - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, - ), + normal_map_texture: Some(asset_server.load_with_settings( + "textures/BlueNoise-Normal.png", + |settings: &mut ImageLoaderSettings| settings.is_srgb = false, )), metallic: 0.9, perceptual_roughness: 0.5, @@ -172,13 +167,9 @@ fn spawn_scratched_gold_ball( MeshMaterial3d(materials.add(StandardMaterial { clearcoat: 1.0, clearcoat_perceptual_roughness: 0.3, - clearcoat_normal_texture: Some(asset_server.load( - AssetPath::from("textures/ScratchedGold-Normal.png").with_settings( - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, - ), + clearcoat_normal_texture: Some(asset_server.load_with_settings( + "textures/ScratchedGold-Normal.png", + |settings: &mut ImageLoaderSettings| settings.is_srgb = false, )), metallic: 0.9, perceptual_roughness: 0.1, diff --git a/examples/3d/rotate_environment_map.rs b/examples/3d/rotate_environment_map.rs index 2dc1d41e69316..68ecceeccf7de 100644 --- a/examples/3d/rotate_environment_map.rs +++ b/examples/3d/rotate_environment_map.rs @@ -8,7 +8,6 @@ use bevy::{ image::ImageLoaderSettings, prelude::*, }; -use bevy_asset::AssetPath; /// Entry point. pub fn main() { @@ -69,13 +68,9 @@ fn spawn_sphere( MeshMaterial3d(materials.add(StandardMaterial { clearcoat: 1.0, clearcoat_perceptual_roughness: 0.3, - clearcoat_normal_texture: Some(asset_server.load( - AssetPath::from("textures/ScratchedGold-Normal.png").with_settings( - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, - ), + clearcoat_normal_texture: Some(asset_server.load_with_settings( + "textures/ScratchedGold-Normal.png", + |settings: &mut ImageLoaderSettings| settings.is_srgb = false, )), metallic: 0.9, perceptual_roughness: 0.1, diff --git a/examples/asset/processing/asset_processing.rs b/examples/asset/processing/asset_processing.rs index 36ac24b02fc45..47a12fc6bb202 100644 --- a/examples/asset/processing/asset_processing.rs +++ b/examples/asset/processing/asset_processing.rs @@ -159,7 +159,9 @@ impl AssetLoader for CoolTextLoader { for (path, settings_override) in ron.dependencies_with_settings { let loaded = load_context .loader() - .with_settings(settings_override) + .with_settings(move |settings| { + *settings = settings_override.clone(); + }) .immediate() .load::(&path) .await?; From 5d735dd2cbd527bbfa147fbb15e9ad3979c2a86d Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 10:53:14 +0100 Subject: [PATCH 13/32] Revert "Removed `load_with_settings` and variants. This is now done by `AssetPath::with_settings`." This reverts commit f0f645ce0db1bb9c2b5b58e57263ae41f5b83c65. --- crates/bevy_asset/src/direct_access_ext.rs | 21 +++++- crates/bevy_asset/src/server/mod.rs | 70 ++++++++++++++++++- examples/2d/mesh2d_repeated_texture.rs | 22 +++--- examples/3d/deferred_rendering.rs | 18 +++-- examples/3d/parallax_mapping.rs | 18 +++-- examples/3d/scrolling_fog.rs | 6 +- examples/3d/ssr.rs | 28 ++++---- examples/asset/alter_mesh.rs | 8 +-- examples/asset/alter_sprite.rs | 41 ++++++----- examples/asset/asset_settings.rs | 14 ++-- examples/asset/repeated_texture.rs | 43 +++++------- examples/ui/ui_texture_slice_flip_and_tile.rs | 16 ++--- 12 files changed, 186 insertions(+), 119 deletions(-) diff --git a/crates/bevy_asset/src/direct_access_ext.rs b/crates/bevy_asset/src/direct_access_ext.rs index d65b307a5d779..7d77538063e99 100644 --- a/crates/bevy_asset/src/direct_access_ext.rs +++ b/crates/bevy_asset/src/direct_access_ext.rs @@ -3,7 +3,7 @@ use bevy_ecs::world::World; -use crate::{Asset, AssetPath, AssetServer, Assets, Handle}; +use crate::{meta::Settings, Asset, AssetPath, AssetServer, Assets, Handle}; /// An extension trait for methods for working with assets directly from a [`World`]. pub trait DirectAssetAccessExt { @@ -12,6 +12,13 @@ pub trait DirectAssetAccessExt { /// Load an asset similarly to [`AssetServer::load`]. fn load_asset<'a, A: Asset>(&self, path: impl Into>) -> Handle; + + /// Load an asset with settings, similarly to [`AssetServer::load_with_settings`]. + fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( + &self, + path: impl Into>, + settings: S, + ) -> Handle; } impl DirectAssetAccessExt for World { /// Insert an asset similarly to [`Assets::add`]. @@ -29,4 +36,16 @@ impl DirectAssetAccessExt for World { fn load_asset<'a, A: Asset>(&self, path: impl Into>) -> Handle { self.resource::().load(path) } + /// Load an asset with settings, similarly to [`AssetServer::load_with_settings`]. + /// + /// # Panics + /// If `self` doesn't have an [`AssetServer`] resource initialized yet. + fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( + &self, + path: impl Into>, + settings: S, + ) -> Handle { + self.resource::() + .load_with_settings(path, settings) + } } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index fbe7c6ffddf3b..abfc67e5def84 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -9,7 +9,7 @@ use crate::{ MissingProcessedAssetReaderError, Reader, }, loader::{AssetLoader, ErasedAssetLoader, LoadContext, LoadedAsset}, - meta::{AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal}, + meta::{AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal, Settings}, path::AssetPath, Asset, AssetEvent, AssetHandleProvider, AssetId, AssetLoadFailedEvent, AssetMetaCheck, Assets, DeserializeMetaError, ErasedLoadedAsset, Handle, LoadedUntypedAsset, UnapprovedPathMode, @@ -368,6 +368,74 @@ impl AssetServer { self.load_with_meta_transform(path, guard, true) } + /// Begins loading an [`Asset`] of type `A` stored at `path`. The given `settings` function will override the asset's + /// [`AssetLoader`] settings. The type `S` _must_ match the configured [`AssetLoader::Settings`] or `settings` changes + /// will be ignored and an error will be printed to the log. + #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] + pub fn load_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( + &self, + path: impl Into>, + settings: S, + ) -> Handle { + self.load_with_meta_transform(path.into().with_settings(settings), (), false) + } + + /// Same as [`load`](AssetServer::load_with_settings), but you can load assets from unaproved paths + /// if [`AssetPlugin::unapproved_path_mode`](super::AssetPlugin::unapproved_path_mode) + /// is [`Deny`](UnapprovedPathMode::Deny). + /// + /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`] + pub fn load_with_settings_override<'a, A: Asset, S: Settings + serde::Serialize>( + &self, + path: impl Into>, + settings: S, + ) -> Handle { + self.load_with_meta_transform(path.into().with_settings(settings), (), true) + } + + /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item. + /// The guard item is dropped when either the asset is loaded or loading has failed. + /// + /// This function only guarantees the asset referenced by the [`Handle`] is loaded. If your asset is separated into + /// multiple files, sub-assets referenced by the main asset might still be loading, depend on the implementation of the [`AssetLoader`]. + /// + /// The given `settings` function will override the asset's + /// [`AssetLoader`] settings. The type `S` _must_ match the configured [`AssetLoader::Settings`] or `settings` changes + /// will be ignored and an error will be printed to the log. + #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] + pub fn load_acquire_with_settings< + 'a, + A: Asset, + S: Settings + serde::Serialize, + G: Send + Sync + 'static, + >( + &self, + path: impl Into>, + settings: S, + guard: G, + ) -> Handle { + self.load_with_meta_transform(path.into().with_settings(settings), guard, false) + } + + /// Same as [`load`](AssetServer::load_acquire_with_settings), but you can load assets from unaproved paths + /// if [`AssetPlugin::unapproved_path_mode`](super::AssetPlugin::unapproved_path_mode) + /// is [`Deny`](UnapprovedPathMode::Deny). + /// + /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`] + pub fn load_acquire_with_settings_override< + 'a, + A: Asset, + S: Settings + serde::Serialize, + G: Send + Sync + 'static, + >( + &self, + path: impl Into>, + settings: S, + guard: G, + ) -> Handle { + self.load_with_meta_transform(path.into().with_settings(settings), guard, true) + } + pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>( &self, path: impl Into>, diff --git a/examples/2d/mesh2d_repeated_texture.rs b/examples/2d/mesh2d_repeated_texture.rs index 6d9b6fba2f400..8436b221f8093 100644 --- a/examples/2d/mesh2d_repeated_texture.rs +++ b/examples/2d/mesh2d_repeated_texture.rs @@ -7,7 +7,6 @@ use bevy::{ math::Affine2, prelude::*, }; -use bevy_asset::AssetPath; /// How much to move some rectangles away from the center const RECTANGLE_OFFSET: f32 = 250.0; @@ -33,18 +32,17 @@ fn setup( // settings let image_with_default_sampler = asset_server.load("textures/fantasy_ui_borders/panel-border-010.png"); - let image_with_repeated_sampler = asset_server.load( - AssetPath::from("textures/fantasy_ui_borders/panel-border-010-repeated.png").with_settings( - ImageLoaderSettings { - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - // rewriting mode to repeat image, - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, - ..default() - }), + let image_with_repeated_sampler = asset_server.load_with_settings( + "textures/fantasy_ui_borders/panel-border-010-repeated.png", + ImageLoaderSettings { + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + // rewriting mode to repeat image, + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, ..default() - }, - ), + }), + ..default() + }, ); // central rectangle with not repeated texture diff --git a/examples/3d/deferred_rendering.rs b/examples/3d/deferred_rendering.rs index 82f402c5a0b3f..c709c170ad090 100644 --- a/examples/3d/deferred_rendering.rs +++ b/examples/3d/deferred_rendering.rs @@ -13,7 +13,6 @@ use bevy::{ }, prelude::*, }; -use bevy_asset::AssetPath; fn main() { App::new() @@ -223,15 +222,14 @@ fn setup_parallax( // The normal map. Note that to generate it in the GIMP image editor, you should // open the depth map, and do Filters → Generic → Normal Map // You should enable the "flip X" checkbox. - let normal_handle = asset_server.load( - AssetPath::from("textures/parallax_example/cube_normal.png").with_settings( - // The normal map texture is in linear color space. Lighting won't look correct - // if `is_srgb` is `true`, which is the default. - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, - ), + let normal_handle = asset_server.load_with_settings( + "textures/parallax_example/cube_normal.png", + // The normal map texture is in linear color space. Lighting won't look correct + // if `is_srgb` is `true`, which is the default. + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, ); let mut cube = Mesh::from(Cuboid::new(0.15, 0.15, 0.15)); diff --git a/examples/3d/parallax_mapping.rs b/examples/3d/parallax_mapping.rs index 41639e6fe2bb5..f3a24a434c637 100644 --- a/examples/3d/parallax_mapping.rs +++ b/examples/3d/parallax_mapping.rs @@ -4,7 +4,6 @@ use std::fmt; use bevy::{image::ImageLoaderSettings, math::ops, prelude::*}; -use bevy_asset::AssetPath; fn main() { App::new() @@ -205,15 +204,14 @@ fn setup( // The normal map. Note that to generate it in the GIMP image editor, you should // open the depth map, and do Filters → Generic → Normal Map // You should enable the "flip X" checkbox. - let normal_handle = asset_server.load( - AssetPath::from("textures/parallax_example/cube_normal.png").with_settings( - // The normal map texture is in linear color space. Lighting won't look correct - // if `is_srgb` is `true`, which is the default. - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, - ), + let normal_handle = asset_server.load_with_settings( + "textures/parallax_example/cube_normal.png", + // The normal map texture is in linear color space. Lighting won't look correct + // if `is_srgb` is `true`, which is the default. + ImageLoaderSettings { + is_srgb: false, + ..Default::default() + }, ); // Camera diff --git a/examples/3d/scrolling_fog.rs b/examples/3d/scrolling_fog.rs index 0b276397455a8..e42e3d3773fa1 100644 --- a/examples/3d/scrolling_fog.rs +++ b/examples/3d/scrolling_fog.rs @@ -20,7 +20,6 @@ use bevy::{ pbr::{DirectionalLightShadowMap, FogVolume, VolumetricFog, VolumetricLight}, prelude::*, }; -use bevy_asset::AssetPath; /// Initializes the example. fn main() { @@ -95,7 +94,8 @@ fn setup( // Load a repeating 3d noise texture. Make sure to set ImageAddressMode to Repeat // so that the texture wraps around as the density texture offset is moved along. // Also set ImageFilterMode to Linear so that the fog isn't pixelated. - let noise_texture = assets.load(AssetPath::from("volumes/fog_noise.ktx2").with_settings( + let noise_texture = assets.load_with_settings( + "volumes/fog_noise.ktx2", ImageLoaderSettings { sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: ImageAddressMode::Repeat, @@ -108,7 +108,7 @@ fn setup( }), ..default() }, - )); + ); // Spawn a FogVolume and use the repeating noise texture as its density texture. commands.spawn(( diff --git a/examples/3d/ssr.rs b/examples/3d/ssr.rs index 1f4db0f5c0d60..61148b2c5d126 100644 --- a/examples/3d/ssr.rs +++ b/examples/3d/ssr.rs @@ -4,7 +4,6 @@ use std::ops::Range; use bevy::{ anti_aliasing::fxaa::Fxaa, - asset::AssetPath, color::palettes::css::{BLACK, WHITE}, core_pipeline::Skybox, image::{ @@ -189,20 +188,19 @@ fn spawn_water( ..default() }, extension: Water { - normals: asset_server.load::( - AssetPath::from("textures/water_normals.png").with_settings( - ImageLoaderSettings { - is_srgb: false, - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, - mag_filter: ImageFilterMode::Linear, - min_filter: ImageFilterMode::Linear, - ..default() - }), - ..Default::default() - }, - ), + normals: asset_server.load_with_settings::( + "textures/water_normals.png", + ImageLoaderSettings { + is_srgb: false, + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, + mag_filter: ImageFilterMode::Linear, + min_filter: ImageFilterMode::Linear, + ..default() + }), + ..Default::default() + }, ), // These water settings are just random values to create some // variety. diff --git a/examples/asset/alter_mesh.rs b/examples/asset/alter_mesh.rs index e2cfcaccafc80..fe1ea5108d722 100644 --- a/examples/asset/alter_mesh.rs +++ b/examples/asset/alter_mesh.rs @@ -57,7 +57,7 @@ fn setup( // In normal use, you can call `asset_server.load`, however see below for an explanation of // `RenderAssetUsages`. - let left_shape_model = asset_server.load( + let left_shape_model = asset_server.load_with_settings( GltfAssetLabel::Primitive { mesh: 0, // This field stores an index to this primitive in its parent mesh. In this case, we @@ -68,7 +68,7 @@ fn setup( // which accomplishes the same thing. primitive: 0, } - .from_asset(left_shape.get_model_path()) + .from_asset(left_shape.get_model_path()), // `RenderAssetUsages::all()` is already the default, so the line below could be omitted. // It's helpful to know it exists, however. // @@ -83,10 +83,10 @@ fn setup( // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the mesh in // RAM. For this example however, this would not work, as we need to inspect and modify the // mesh at runtime. - .with_settings(GltfLoaderSettings { + GltfLoaderSettings { load_meshes: RenderAssetUsages::all(), ..Default::default() - }), + }, ); // Here, we rely on the default loader settings to achieve a similar result to the above. diff --git a/examples/asset/alter_sprite.rs b/examples/asset/alter_sprite.rs index 7fa4b948a1e29..c1d6736ed97d2 100644 --- a/examples/asset/alter_sprite.rs +++ b/examples/asset/alter_sprite.rs @@ -4,7 +4,6 @@ use bevy::{ image::ImageLoaderSettings, input::common_conditions::input_just_pressed, prelude::*, render::render_asset::RenderAssetUsages, }; -use bevy_asset::AssetPath; fn main() { App::new() @@ -51,26 +50,26 @@ fn setup(mut commands: Commands, asset_server: Res) { let bird_right = Bird::Normal; commands.spawn(Camera2d); - let texture_left = asset_server.load( - AssetPath::from(bird_left.get_texture_path()) - // `RenderAssetUsages::all()` is already the default, so the line below could be omitted. - // It's helpful to know it exists, however. - // - // `RenderAssetUsages` tell Bevy whether to keep the data around: - // - for the GPU (`RenderAssetUsages::RENDER_WORLD`), - // - for the CPU (`RenderAssetUsages::MAIN_WORLD`), - // - or both. - // `RENDER_WORLD` is necessary to render the image, `MAIN_WORLD` is necessary to inspect - // and modify the image (via `ResMut>`). - // - // Since most games will not need to modify textures at runtime, many developers opt to pass - // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the image in - // RAM. For this example however, this would not work, as we need to inspect and modify the - // image at runtime. - .with_settings(ImageLoaderSettings { - asset_usage: RenderAssetUsages::all(), - ..Default::default() - }), + let texture_left = asset_server.load_with_settings( + bird_left.get_texture_path(), + // `RenderAssetUsages::all()` is already the default, so the line below could be omitted. + // It's helpful to know it exists, however. + // + // `RenderAssetUsages` tell Bevy whether to keep the data around: + // - for the GPU (`RenderAssetUsages::RENDER_WORLD`), + // - for the CPU (`RenderAssetUsages::MAIN_WORLD`), + // - or both. + // `RENDER_WORLD` is necessary to render the image, `MAIN_WORLD` is necessary to inspect + // and modify the image (via `ResMut>`). + // + // Since most games will not need to modify textures at runtime, many developers opt to pass + // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the image in + // RAM. For this example however, this would not work, as we need to inspect and modify the + // image at runtime. + ImageLoaderSettings { + asset_usage: RenderAssetUsages::all(), + ..Default::default() + }, ); commands.spawn(( diff --git a/examples/asset/asset_settings.rs b/examples/asset/asset_settings.rs index 7ea9c1ce2feba..c8cf01909bec5 100644 --- a/examples/asset/asset_settings.rs +++ b/examples/asset/asset_settings.rs @@ -4,7 +4,6 @@ use bevy::{ image::{ImageLoaderSettings, ImageSampler}, prelude::*, }; -use bevy_asset::AssetPath; fn main() { App::new() @@ -63,13 +62,12 @@ fn setup(mut commands: Commands, asset_server: Res) { // same one as without a .meta file. commands.spawn(( Sprite { - image: asset_server.load( - AssetPath::from("bevy_pixel_dark_with_settings.png").with_settings( - ImageLoaderSettings { - sampler: ImageSampler::nearest(), - ..Default::default() - }, - ), + image: asset_server.load_with_settings( + "bevy_pixel_dark_with_settings.png", + ImageLoaderSettings { + sampler: ImageSampler::nearest(), + ..Default::default() + }, ), custom_size: Some(Vec2 { x: 160.0, y: 120.0 }), ..Default::default() diff --git a/examples/asset/repeated_texture.rs b/examples/asset/repeated_texture.rs index 3118af6320743..a39f8755c4033 100644 --- a/examples/asset/repeated_texture.rs +++ b/examples/asset/repeated_texture.rs @@ -6,7 +6,6 @@ use bevy::{ math::Affine2, prelude::*, }; -use bevy_asset::AssetPath; fn main() { App::new() @@ -37,31 +36,25 @@ fn setup( // left cube with repeated texture commands.spawn(( Mesh3d(meshes.add(Cuboid::new(1.0, 1.0, 1.0))), - MeshMaterial3d( - materials.add(StandardMaterial { - base_color_texture: Some( - asset_server.load( - AssetPath::from( - "textures/fantasy_ui_borders/panel-border-010-repeated.png", - ) - .with_settings(ImageLoaderSettings { - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - // rewriting mode to repeat image, - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, - ..default() - }), - ..default() - }), - ), - ), + MeshMaterial3d(materials.add(StandardMaterial { + base_color_texture: Some(asset_server.load_with_settings( + "textures/fantasy_ui_borders/panel-border-010-repeated.png", + ImageLoaderSettings { + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + // rewriting mode to repeat image, + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, + ..default() + }), + ..default() + }, + )), - // uv_transform used here for proportions only, but it is full Affine2 - // that's why you can use rotation and shift also - uv_transform: Affine2::from_scale(Vec2::new(2., 3.)), - ..default() - }), - ), + // uv_transform used here for proportions only, but it is full Affine2 + // that's why you can use rotation and shift also + uv_transform: Affine2::from_scale(Vec2::new(2., 3.)), + ..default() + })), Transform::from_xyz(-1.5, 0.0, 0.0), )); diff --git a/examples/ui/ui_texture_slice_flip_and_tile.rs b/examples/ui/ui_texture_slice_flip_and_tile.rs index c0b9dc1637668..7977f52f1a417 100644 --- a/examples/ui/ui_texture_slice_flip_and_tile.rs +++ b/examples/ui/ui_texture_slice_flip_and_tile.rs @@ -6,7 +6,6 @@ use bevy::{ ui::widget::NodeImageMode, winit::WinitSettings, }; -use bevy_asset::AssetPath; fn main() { App::new() @@ -19,14 +18,13 @@ fn main() { } fn setup(mut commands: Commands, asset_server: Res) { - let image = asset_server.load( - AssetPath::from("textures/fantasy_ui_borders/numbered_slices.png").with_settings( - ImageLoaderSettings { - // Need to use nearest filtering to avoid bleeding between the slices with tiling - sampler: ImageSampler::nearest(), - ..Default::default() - }, - ), + let image = asset_server.load_with_settings( + "textures/fantasy_ui_borders/numbered_slices.png", + ImageLoaderSettings { + // Need to use nearest filtering to avoid bleeding between the slices with tiling + sampler: ImageSampler::nearest(), + ..Default::default() + }, ); let slicer = TextureSlicer { From 2e271c7e2ffa244ce3dee0339155e720592f34e1 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 10:53:28 +0100 Subject: [PATCH 14/32] Revert "Updated examples." This reverts commit 5779ba27d0dc84e339ce6974be123a6afc8b0631. --- examples/2d/mesh2d_repeated_texture.rs | 16 +++++++++------- examples/3d/deferred_rendering.rs | 5 +---- examples/3d/parallax_mapping.rs | 5 +---- examples/3d/scrolling_fog.rs | 9 ++++----- examples/asset/alter_mesh.rs | 5 +---- examples/asset/alter_sprite.rs | 5 +---- examples/asset/asset_decompression.rs | 4 ---- examples/asset/asset_settings.rs | 5 ++--- examples/asset/repeated_texture.rs | 16 +++++++++------- examples/ui/ui_texture_slice_flip_and_tile.rs | 5 ++--- 10 files changed, 30 insertions(+), 45 deletions(-) diff --git a/examples/2d/mesh2d_repeated_texture.rs b/examples/2d/mesh2d_repeated_texture.rs index 8436b221f8093..096a1a5f99d3e 100644 --- a/examples/2d/mesh2d_repeated_texture.rs +++ b/examples/2d/mesh2d_repeated_texture.rs @@ -34,14 +34,16 @@ fn setup( asset_server.load("textures/fantasy_ui_borders/panel-border-010.png"); let image_with_repeated_sampler = asset_server.load_with_settings( "textures/fantasy_ui_borders/panel-border-010-repeated.png", - ImageLoaderSettings { - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - // rewriting mode to repeat image, - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, + |s: &mut _| { + *s = ImageLoaderSettings { + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + // rewriting mode to repeat image, + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, + ..default() + }), ..default() - }), - ..default() + } }, ); diff --git a/examples/3d/deferred_rendering.rs b/examples/3d/deferred_rendering.rs index c709c170ad090..d6e58fe5058a8 100644 --- a/examples/3d/deferred_rendering.rs +++ b/examples/3d/deferred_rendering.rs @@ -226,10 +226,7 @@ fn setup_parallax( "textures/parallax_example/cube_normal.png", // The normal map texture is in linear color space. Lighting won't look correct // if `is_srgb` is `true`, which is the default. - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, + |settings: &mut ImageLoaderSettings| settings.is_srgb = false, ); let mut cube = Mesh::from(Cuboid::new(0.15, 0.15, 0.15)); diff --git a/examples/3d/parallax_mapping.rs b/examples/3d/parallax_mapping.rs index f3a24a434c637..47d83a4e76960 100644 --- a/examples/3d/parallax_mapping.rs +++ b/examples/3d/parallax_mapping.rs @@ -208,10 +208,7 @@ fn setup( "textures/parallax_example/cube_normal.png", // The normal map texture is in linear color space. Lighting won't look correct // if `is_srgb` is `true`, which is the default. - ImageLoaderSettings { - is_srgb: false, - ..Default::default() - }, + |settings: &mut ImageLoaderSettings| settings.is_srgb = false, ); // Camera diff --git a/examples/3d/scrolling_fog.rs b/examples/3d/scrolling_fog.rs index e42e3d3773fa1..f65b4131701cd 100644 --- a/examples/3d/scrolling_fog.rs +++ b/examples/3d/scrolling_fog.rs @@ -94,9 +94,8 @@ fn setup( // Load a repeating 3d noise texture. Make sure to set ImageAddressMode to Repeat // so that the texture wraps around as the density texture offset is moved along. // Also set ImageFilterMode to Linear so that the fog isn't pixelated. - let noise_texture = assets.load_with_settings( - "volumes/fog_noise.ktx2", - ImageLoaderSettings { + let noise_texture = assets.load_with_settings("volumes/fog_noise.ktx2", |settings: &mut _| { + *settings = ImageLoaderSettings { sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: ImageAddressMode::Repeat, address_mode_v: ImageAddressMode::Repeat, @@ -107,8 +106,8 @@ fn setup( ..default() }), ..default() - }, - ); + } + }); // Spawn a FogVolume and use the repeating noise texture as its density texture. commands.spawn(( diff --git a/examples/asset/alter_mesh.rs b/examples/asset/alter_mesh.rs index fe1ea5108d722..7462c47ae1f85 100644 --- a/examples/asset/alter_mesh.rs +++ b/examples/asset/alter_mesh.rs @@ -83,10 +83,7 @@ fn setup( // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the mesh in // RAM. For this example however, this would not work, as we need to inspect and modify the // mesh at runtime. - GltfLoaderSettings { - load_meshes: RenderAssetUsages::all(), - ..Default::default() - }, + |settings: &mut GltfLoaderSettings| settings.load_meshes = RenderAssetUsages::all(), ); // Here, we rely on the default loader settings to achieve a similar result to the above. diff --git a/examples/asset/alter_sprite.rs b/examples/asset/alter_sprite.rs index c1d6736ed97d2..d47c303921521 100644 --- a/examples/asset/alter_sprite.rs +++ b/examples/asset/alter_sprite.rs @@ -66,10 +66,7 @@ fn setup(mut commands: Commands, asset_server: Res) { // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the image in // RAM. For this example however, this would not work, as we need to inspect and modify the // image at runtime. - ImageLoaderSettings { - asset_usage: RenderAssetUsages::all(), - ..Default::default() - }, + |settings: &mut ImageLoaderSettings| settings.asset_usage = RenderAssetUsages::all(), ); commands.spawn(( diff --git a/examples/asset/asset_decompression.rs b/examples/asset/asset_decompression.rs index 68270951142a8..e514924ca9d0a 100644 --- a/examples/asset/asset_decompression.rs +++ b/examples/asset/asset_decompression.rs @@ -23,10 +23,6 @@ struct GzAssetLoader; /// Possible errors that can be produced by [`GzAssetLoader`] #[non_exhaustive] #[derive(Debug, Error)] -#[expect( - clippy::large_enum_variant, - reason = "XXX TODO: Why did this start happening?" -)] enum GzAssetLoaderError { /// An [IO](std::io) Error #[error("Could not load asset: {0}")] diff --git a/examples/asset/asset_settings.rs b/examples/asset/asset_settings.rs index c8cf01909bec5..cfc76d774a960 100644 --- a/examples/asset/asset_settings.rs +++ b/examples/asset/asset_settings.rs @@ -64,9 +64,8 @@ fn setup(mut commands: Commands, asset_server: Res) { Sprite { image: asset_server.load_with_settings( "bevy_pixel_dark_with_settings.png", - ImageLoaderSettings { - sampler: ImageSampler::nearest(), - ..Default::default() + |settings: &mut ImageLoaderSettings| { + settings.sampler = ImageSampler::nearest(); }, ), custom_size: Some(Vec2 { x: 160.0, y: 120.0 }), diff --git a/examples/asset/repeated_texture.rs b/examples/asset/repeated_texture.rs index a39f8755c4033..34664a9e42b5c 100644 --- a/examples/asset/repeated_texture.rs +++ b/examples/asset/repeated_texture.rs @@ -39,14 +39,16 @@ fn setup( MeshMaterial3d(materials.add(StandardMaterial { base_color_texture: Some(asset_server.load_with_settings( "textures/fantasy_ui_borders/panel-border-010-repeated.png", - ImageLoaderSettings { - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { - // rewriting mode to repeat image, - address_mode_u: ImageAddressMode::Repeat, - address_mode_v: ImageAddressMode::Repeat, + |s: &mut _| { + *s = ImageLoaderSettings { + sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + // rewriting mode to repeat image, + address_mode_u: ImageAddressMode::Repeat, + address_mode_v: ImageAddressMode::Repeat, + ..default() + }), ..default() - }), - ..default() + } }, )), diff --git a/examples/ui/ui_texture_slice_flip_and_tile.rs b/examples/ui/ui_texture_slice_flip_and_tile.rs index 7977f52f1a417..3eaf4277f5b70 100644 --- a/examples/ui/ui_texture_slice_flip_and_tile.rs +++ b/examples/ui/ui_texture_slice_flip_and_tile.rs @@ -20,10 +20,9 @@ fn main() { fn setup(mut commands: Commands, asset_server: Res) { let image = asset_server.load_with_settings( "textures/fantasy_ui_borders/numbered_slices.png", - ImageLoaderSettings { + |settings: &mut ImageLoaderSettings| { // Need to use nearest filtering to avoid bleeding between the slices with tiling - sampler: ImageSampler::nearest(), - ..Default::default() + settings.sampler = ImageSampler::nearest(); }, ); From d4c55fde8c7e46bfcfb4bb9ba5559ca329406667 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 12:07:42 +0100 Subject: [PATCH 15/32] Temporarily disabled clippy warning. --- examples/asset/asset_decompression.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/asset/asset_decompression.rs b/examples/asset/asset_decompression.rs index e514924ca9d0a..8cafe76563f16 100644 --- a/examples/asset/asset_decompression.rs +++ b/examples/asset/asset_decompression.rs @@ -23,6 +23,7 @@ struct GzAssetLoader; /// Possible errors that can be produced by [`GzAssetLoader`] #[non_exhaustive] #[derive(Debug, Error)] +#[expect(clippy::large_enum_variant, reason = "XXX TODO: Is this ok?")] enum GzAssetLoaderError { /// An [IO](std::io) Error #[error("Could not load asset: {0}")] From 01b4471de32367a6c378340169d9306144bec835 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 12:10:40 +0100 Subject: [PATCH 16/32] Changed `load_with_settings` and variants to take a closure instead of a setting value. This makes the interface backwards compatible. --- crates/bevy_asset/src/direct_access_ext.rs | 8 ++++---- crates/bevy_asset/src/path.rs | 18 ++++++++++++++++ crates/bevy_asset/src/server/mod.rs | 24 +++++++++++----------- examples/3d/ssr.rs | 9 ++++---- 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/crates/bevy_asset/src/direct_access_ext.rs b/crates/bevy_asset/src/direct_access_ext.rs index 7d77538063e99..d29df80b0bfdc 100644 --- a/crates/bevy_asset/src/direct_access_ext.rs +++ b/crates/bevy_asset/src/direct_access_ext.rs @@ -14,10 +14,10 @@ pub trait DirectAssetAccessExt { fn load_asset<'a, A: Asset>(&self, path: impl Into>) -> Handle; /// Load an asset with settings, similarly to [`AssetServer::load_with_settings`]. - fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( + fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize + Default>( &self, path: impl Into>, - settings: S, + settings: impl FnOnce(&mut S) + Send + Sync + 'static, ) -> Handle; } impl DirectAssetAccessExt for World { @@ -40,10 +40,10 @@ impl DirectAssetAccessExt for World { /// /// # Panics /// If `self` doesn't have an [`AssetServer`] resource initialized yet. - fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( + fn load_asset_with_settings<'a, A: Asset, S: Settings + serde::Serialize + Default>( &self, path: impl Into>, - settings: S, + settings: impl FnOnce(&mut S) + Send + Sync + 'static, ) -> Handle { self.resource::() .load_with_settings(path, settings) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index d2193c81a177c..14986f786d973 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -394,6 +394,9 @@ impl<'a> AssetPath<'a> { } } + // XXX TODO: This shouldn't be an Option. + // XXX TODO: Should this take an Arc? Potential optimization if the input + // settings are already Arc'd. Or can we do Into? #[inline] pub fn with_erased_settings(self, settings: Option) -> AssetPath<'a> { AssetPath { @@ -404,6 +407,21 @@ impl<'a> AssetPath<'a> { } } + #[inline] + pub fn with_settings_fn( + self, + settings_fn: impl FnOnce(&mut S) + Send + Sync + 'static, + ) -> AssetPath<'a> { + let mut settings = S::default(); + settings_fn(&mut settings); + AssetPath { + source: self.source, + path: self.path, + label: self.label, + settings: Some(Arc::new(ErasedSettings::new(settings))), + } + } + /// Returns an [`AssetPath`] for the parent folder of this path, if there is a parent folder in the path. pub fn parent(&self) -> Option> { let path = match &self.path { diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index abfc67e5def84..4354edc16a3de 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -372,12 +372,12 @@ impl AssetServer { /// [`AssetLoader`] settings. The type `S` _must_ match the configured [`AssetLoader::Settings`] or `settings` changes /// will be ignored and an error will be printed to the log. #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"] - pub fn load_with_settings<'a, A: Asset, S: Settings + serde::Serialize>( + pub fn load_with_settings<'a, A: Asset, S: Settings + serde::Serialize + Default>( &self, path: impl Into>, - settings: S, + settings: impl FnOnce(&mut S) + Send + Sync + 'static, ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), (), false) + self.load_with_meta_transform(path.into().with_settings_fn(settings), (), false) } /// Same as [`load`](AssetServer::load_with_settings), but you can load assets from unaproved paths @@ -385,12 +385,12 @@ impl AssetServer { /// is [`Deny`](UnapprovedPathMode::Deny). /// /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`] - pub fn load_with_settings_override<'a, A: Asset, S: Settings + serde::Serialize>( + pub fn load_with_settings_override<'a, A: Asset, S: Settings + serde::Serialize + Default>( &self, path: impl Into>, - settings: S, + settings: impl FnOnce(&mut S) + Send + Sync + 'static, ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), (), true) + self.load_with_meta_transform(path.into().with_settings_fn(settings), (), true) } /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item. @@ -406,15 +406,15 @@ impl AssetServer { pub fn load_acquire_with_settings< 'a, A: Asset, - S: Settings + serde::Serialize, + S: Settings + serde::Serialize + Default, G: Send + Sync + 'static, >( &self, path: impl Into>, - settings: S, + settings: impl FnOnce(&mut S) + Send + Sync + 'static, guard: G, ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), guard, false) + self.load_with_meta_transform(path.into().with_settings_fn(settings), guard, false) } /// Same as [`load`](AssetServer::load_acquire_with_settings), but you can load assets from unaproved paths @@ -425,15 +425,15 @@ impl AssetServer { pub fn load_acquire_with_settings_override< 'a, A: Asset, - S: Settings + serde::Serialize, + S: Settings + serde::Serialize + Default, G: Send + Sync + 'static, >( &self, path: impl Into>, - settings: S, + settings: impl FnOnce(&mut S) + Send + Sync + 'static, guard: G, ) -> Handle { - self.load_with_meta_transform(path.into().with_settings(settings), guard, true) + self.load_with_meta_transform(path.into().with_settings_fn(settings), guard, true) } pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>( diff --git a/examples/3d/ssr.rs b/examples/3d/ssr.rs index 61148b2c5d126..a4d2c1b742c0f 100644 --- a/examples/3d/ssr.rs +++ b/examples/3d/ssr.rs @@ -190,16 +190,15 @@ fn spawn_water( extension: Water { normals: asset_server.load_with_settings::( "textures/water_normals.png", - ImageLoaderSettings { - is_srgb: false, - sampler: ImageSampler::Descriptor(ImageSamplerDescriptor { + |settings| { + settings.is_srgb = false; + settings.sampler = ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: ImageAddressMode::Repeat, address_mode_v: ImageAddressMode::Repeat, mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, ..default() - }), - ..Default::default() + }); }, ), // These water settings are just random values to create some From a086ff70733a907c8dfdff62cc5760dcd26c5b2e Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 12:26:09 +0100 Subject: [PATCH 17/32] Removed settings from `NestedLoader`. This was no longer necessary since clients can provide the settings in the `AssetPath` parameter of `load`. --- crates/bevy_asset/src/loader_builders.rs | 28 +++++------------------- crates/bevy_asset/src/path.rs | 13 ----------- crates/bevy_gltf/src/loader/mod.rs | 7 +++--- 3 files changed, 8 insertions(+), 40 deletions(-) diff --git a/crates/bevy_asset/src/loader_builders.rs b/crates/bevy_asset/src/loader_builders.rs index c289551d741c5..f4e39a3a2394f 100644 --- a/crates/bevy_asset/src/loader_builders.rs +++ b/crates/bevy_asset/src/loader_builders.rs @@ -2,9 +2,8 @@ //! [`LoadContext::loader`]. use crate::{ - io::Reader, meta::Settings, Asset, AssetLoadError, AssetPath, ErasedAssetLoader, - ErasedLoadedAsset, ErasedSettings, Handle, LoadContext, LoadDirectError, LoadedAsset, - LoadedUntypedAsset, UntypedHandle, + io::Reader, Asset, AssetLoadError, AssetPath, ErasedAssetLoader, ErasedLoadedAsset, Handle, + LoadContext, LoadDirectError, LoadedAsset, LoadedUntypedAsset, UntypedHandle, }; use alloc::{borrow::ToOwned, boxed::Box, sync::Arc}; use core::any::TypeId; @@ -116,7 +115,6 @@ impl ReaderRef<'_> { /// [`LoadTransformAndSave`]: crate::processor::LoadTransformAndSave pub struct NestedLoader<'ctx, 'builder, T, M> { load_context: &'builder mut LoadContext<'ctx>, - settings: Option, typing: T, mode: M, } @@ -167,7 +165,6 @@ impl<'ctx, 'builder> NestedLoader<'ctx, 'builder, StaticTyped, Deferred> { pub(crate) fn new(load_context: &'builder mut LoadContext<'ctx>) -> Self { NestedLoader { load_context, - settings: None, typing: StaticTyped(()), mode: Deferred(()), } @@ -175,16 +172,6 @@ impl<'ctx, 'builder> NestedLoader<'ctx, 'builder, StaticTyped, Deferred> { } impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'builder, T, M> { - /// Configure the settings used to load the asset. - /// - /// If the settings type `S` does not match the settings expected by `A`'s asset loader, an error will be printed to the log - /// and the asset load will fail. - #[must_use] - pub fn with_settings(mut self, settings: S) -> Self { - self.settings = Some(ErasedSettings::new(settings)); - self - } - // convert between `T`s /// When [`load`]ing, you must pass in the asset type as a type parameter @@ -200,7 +187,6 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui pub fn with_static_type(self) -> NestedLoader<'ctx, 'builder, StaticTyped, M> { NestedLoader { load_context: self.load_context, - settings: self.settings, typing: StaticTyped(()), mode: self.mode, } @@ -217,7 +203,6 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui ) -> NestedLoader<'ctx, 'builder, DynamicTyped, M> { NestedLoader { load_context: self.load_context, - settings: self.settings, typing: DynamicTyped { asset_type_id }, mode: self.mode, } @@ -231,7 +216,6 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui pub fn with_unknown_type(self) -> NestedLoader<'ctx, 'builder, UnknownTyped, M> { NestedLoader { load_context: self.load_context, - settings: self.settings, typing: UnknownTyped(()), mode: self.mode, } @@ -246,7 +230,6 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui pub fn deferred(self) -> NestedLoader<'ctx, 'builder, T, Deferred> { NestedLoader { load_context: self.load_context, - settings: self.settings, typing: self.typing, mode: Deferred(()), } @@ -263,7 +246,6 @@ impl<'ctx, 'builder, T: sealed::Typing, M: sealed::Mode> NestedLoader<'ctx, 'bui pub fn immediate<'c>(self) -> NestedLoader<'ctx, 'builder, T, Immediate<'builder, 'c>> { NestedLoader { load_context: self.load_context, - settings: self.settings, typing: self.typing, mode: Immediate { reader: None }, } @@ -285,7 +267,7 @@ impl NestedLoader<'_, '_, StaticTyped, Deferred> { /// [`with_dynamic_type`]: Self::with_dynamic_type /// [`with_unknown_type`]: Self::with_unknown_type pub fn load<'c, A: Asset>(self, path: impl Into>) -> Handle { - let path = path.into().with_erased_settings(self.settings).to_owned(); + let path = path.into().to_owned(); let handle = if self.load_context.should_load_dependencies { self.load_context .asset_server @@ -309,7 +291,7 @@ impl NestedLoader<'_, '_, DynamicTyped, Deferred> { /// /// [`with_dynamic_type`]: Self::with_dynamic_type pub fn load<'p>(self, path: impl Into>) -> UntypedHandle { - let path = path.into().with_erased_settings(self.settings).to_owned(); + let path = path.into().to_owned(); let handle = if self.load_context.should_load_dependencies { self.load_context .asset_server @@ -330,7 +312,7 @@ impl NestedLoader<'_, '_, UnknownTyped, Deferred> { /// /// This will infer the asset type from metadata. pub fn load<'p>(self, path: impl Into>) -> Handle { - let path = path.into().with_erased_settings(self.settings).to_owned(); + let path = path.into().to_owned(); let handle = if self.load_context.should_load_dependencies { self.load_context .asset_server diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 14986f786d973..49d00bd5b8b64 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -394,19 +394,6 @@ impl<'a> AssetPath<'a> { } } - // XXX TODO: This shouldn't be an Option. - // XXX TODO: Should this take an Arc? Potential optimization if the input - // settings are already Arc'd. Or can we do Into? - #[inline] - pub fn with_erased_settings(self, settings: Option) -> AssetPath<'a> { - AssetPath { - source: self.source, - path: self.path, - label: self.label, - settings: settings.map(Arc::new), - } - } - #[inline] pub fn with_settings_fn( self, diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index f87b6cf4ad1c9..fd7c55f73f632 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -11,7 +11,7 @@ use std::{ #[cfg(feature = "bevy_animation")] use bevy_animation::{prelude::*, AnimationTarget, AnimationTargetId}; use bevy_asset::{ - io::Reader, AssetLoadError, AssetLoader, Handle, LoadContext, ReadAssetBytesError, + io::Reader, AssetLoadError, AssetLoader, AssetPath, Handle, LoadContext, ReadAssetBytesError, RenderAssetUsages, }; use bevy_color::{Color, LinearRgba}; @@ -1719,12 +1719,11 @@ impl ImageOrPath { sampler_descriptor, } => load_context .loader() - .with_settings(ImageLoaderSettings { + .load(AssetPath::from(path).with_settings(ImageLoaderSettings { is_srgb, sampler: ImageSampler::Descriptor(sampler_descriptor.clone()), ..Default::default() - }) - .load(path), + })), }; handles.push(handle); } From 416432c0881734d047dd0c1346526ef7f2698e14 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 12:33:20 +0100 Subject: [PATCH 18/32] Added comment about `load_with_meta_transform` function name. --- crates/bevy_asset/src/server/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 4354edc16a3de..1d15b6cc0d17c 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -436,6 +436,8 @@ impl AssetServer { self.load_with_meta_transform(path.into().with_settings_fn(settings), guard, true) } + // XXX TODO: This should probably be renamed, but I'm not sure what to. + // `load_internal` is taken. `load_with_params`? pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>( &self, path: impl Into>, From 15432f2033dbba4953b19cf55218c775dd760055 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 13:17:10 +0100 Subject: [PATCH 19/32] Updated `asset_processing` example. --- examples/asset/processing/asset_processing.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/asset/processing/asset_processing.rs b/examples/asset/processing/asset_processing.rs index 47a12fc6bb202..4b4fc25782dc8 100644 --- a/examples/asset/processing/asset_processing.rs +++ b/examples/asset/processing/asset_processing.rs @@ -12,6 +12,7 @@ use bevy::{ prelude::*, reflect::TypePath, }; +use bevy_asset::AssetPath; use serde::{Deserialize, Serialize}; use std::convert::Infallible; use thiserror::Error; @@ -159,11 +160,10 @@ impl AssetLoader for CoolTextLoader { for (path, settings_override) in ron.dependencies_with_settings { let loaded = load_context .loader() - .with_settings(move |settings| { - *settings = settings_override.clone(); - }) .immediate() - .load::(&path) + .load::(AssetPath::from(path).with_settings_fn(move |settings| { + *settings = settings_override.clone(); + })) .await?; base_text.push_str(&loaded.get().0); } From 94e09a948b8a2fd423de8175a57ebf81df6f8574 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 13:57:59 +0100 Subject: [PATCH 20/32] Changed `ErasedSettings` and `ErasedSettingsId` members to be private, and added `ErasedSettings::value()` that correctly derefs the box. Also updated various comments. --- crates/bevy_asset/src/loader_builders.rs | 3 +-- crates/bevy_asset/src/path.rs | 30 +++++++++++++++++++----- crates/bevy_asset/src/server/mod.rs | 5 +--- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/crates/bevy_asset/src/loader_builders.rs b/crates/bevy_asset/src/loader_builders.rs index f4e39a3a2394f..160463b3c280c 100644 --- a/crates/bevy_asset/src/loader_builders.rs +++ b/crates/bevy_asset/src/loader_builders.rs @@ -381,8 +381,7 @@ impl<'builder, 'reader, T> NestedLoader<'_, '_, T, Immediate<'builder, 'reader>> }; if let Some(settings) = path.settings() { - // XXX TODO: Does this need the same deref hack as `load_internal`? - meta.apply_settings(settings); + meta.apply_settings(settings.value()); } let asset = self diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 49d00bd5b8b64..d0d3415374705 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -18,9 +18,12 @@ use serde::{de::Visitor, Deserialize, Serialize}; use std::path::{Path, PathBuf}; use thiserror::Error; +/// Identifies an erased settings value. This is used to compare and hash values +/// without having to read the underlying value. #[derive(Eq, PartialEq, Hash, Copy, Clone)] pub struct ErasedSettingsId { - // XXX TODO: Unclear if we want type id? Does make debugging a bit easier. + // XXX TODO: Should we store the type id separately or just include it in the + // hash? Separately might be nicer for debugging. type_id: TypeId, hash: u64, } @@ -32,18 +35,21 @@ impl Display for ErasedSettingsId { } } +/// An erased settings value and its id. pub struct ErasedSettings { - pub value: Box, - pub id: ErasedSettingsId, + value: Box, + id: ErasedSettingsId, } impl ErasedSettings { pub fn new(settings: S) -> ErasedSettings { - // XXX TODO: Serializing to string is probably not the right solution. - let string = ron::ser::to_string(&settings).unwrap(); // XXX TODO: Avoid unwrap. + // XXX TODO: Hash by serializing to a string. This is bad but + // convenient - Settings are already required to implement serialize + // (for meta filesupport) and this avoids them having to implement Hash. + // XXX TODO: More efficient to serialize to a writer that hashes? + let string = ron::ser::to_string(&settings).unwrap(); // XXX TODO: Avoid unwrap? // XXX TODO: What's the appropriate hasher? - // XXX TODO: Should we hash the type id as well? let id = ErasedSettingsId { type_id: TypeId::of::(), hash: FixedHasher.hash_one(&string), @@ -56,6 +62,18 @@ impl ErasedSettings { } } +impl<'a> ErasedSettings { + pub fn value(&'a self) -> &'a dyn Settings { + // The `deref` means we're returning the underlying type and not Box's + // implementation. This is required so that the value can be downcast + // by `AssetMeta::apply_settings`. + // + // XXX TODO: Maybe this should all be done in `AssetMeta::apply_settings`? + // Then everything's in one place. + self.value.deref() + } +} + impl Hash for ErasedSettings { fn hash(&self, state: &mut H) { self.id.hash(state); diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 1d15b6cc0d17c..3c0a68a390e94 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -644,10 +644,7 @@ impl AssetServer { })?; if let Some(settings) = path.settings() { - // Deref to avoid `apply_settings` downcast failing because the type - // is `Box` when it expected `T`. XXX TODO: Better solution. - let settings = core::ops::Deref::deref(&settings.value); - meta.apply_settings(settings); + meta.apply_settings(settings.value()); } // downgrade the input handle so we don't keep the asset alive just because we're loading it // note we can't just pass a weak handle in, as only strong handles contain the asset meta transform From e2e9c3097909501cca7b6085750817d1b69ea7cb Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 14:48:51 +0100 Subject: [PATCH 21/32] Replaced `ron::ser::to_string` with a (hopefully?) more efficient `ron::ser::to_write` + hash writer. --- crates/bevy_asset/src/path.rs | 60 +++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 16 deletions(-) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index d0d3415374705..d389223f25f84 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -6,7 +6,7 @@ use alloc::{ sync::Arc, }; use atomicow::CowArc; -use bevy_platform::hash::FixedHasher; +use bevy_platform::hash::{DefaultHasher, FixedHasher}; use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize}; use core::{ any::TypeId, @@ -20,6 +20,7 @@ use thiserror::Error; /// Identifies an erased settings value. This is used to compare and hash values /// without having to read the underlying value. +/// XXX TODO: Maybe this should be `ErasedSettingsType`? #[derive(Eq, PartialEq, Hash, Copy, Clone)] pub struct ErasedSettingsId { // XXX TODO: Should we store the type id separately or just include it in the @@ -41,32 +42,59 @@ pub struct ErasedSettings { id: ErasedSettingsId, } +// XXX TODO: This should go somewhere more shared? +struct HashWriter { + hasher: DefaultHasher, +} + +impl HashWriter { + fn new() -> Self { + HashWriter { + hasher: FixedHasher.build_hasher(), + } + } + + fn finish(self) -> u64 { + self.hasher.finish() + } +} + +impl std::io::Write for HashWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.hasher.write(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + impl ErasedSettings { pub fn new(settings: S) -> ErasedSettings { - // XXX TODO: Hash by serializing to a string. This is bad but - // convenient - Settings are already required to implement serialize - // (for meta filesupport) and this avoids them having to implement Hash. - // XXX TODO: More efficient to serialize to a writer that hashes? - let string = ron::ser::to_string(&settings).unwrap(); // XXX TODO: Avoid unwrap? - - // XXX TODO: What's the appropriate hasher? - let id = ErasedSettingsId { - type_id: TypeId::of::(), - hash: FixedHasher.hash_one(&string), - }; + // Hash by serializing to RON. This means settings are not required to + // implement Hash. + // XXX TODO: Hashing via RON serialization is very debatable. + // XXX TODO: Could do ron::ser::to_string? Simpler but probably(?) slower. + let mut hash_writer = HashWriter::new(); + ron::ser::to_writer(&mut hash_writer, &settings).expect("XXX TODO?"); + let hash = hash_writer.finish(); ErasedSettings { value: Box::new(settings), - id, + id: ErasedSettingsId { + type_id: TypeId::of::(), + hash, + }, } } } impl<'a> ErasedSettings { pub fn value(&'a self) -> &'a dyn Settings { - // The `deref` means we're returning the underlying type and not Box's - // implementation. This is required so that the value can be downcast - // by `AssetMeta::apply_settings`. + // The `deref` means we're returning the underlying type's implementation + // of Settings - not Box's wrapper. This is required so that the value + // can be downcast by `AssetMeta::apply_settings`. // // XXX TODO: Maybe this should all be done in `AssetMeta::apply_settings`? // Then everything's in one place. From e8cc845af304feba2a1cb01a9f46c044848752ed Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 14:53:53 +0100 Subject: [PATCH 22/32] Updated comments. --- crates/bevy_asset/src/path.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index d389223f25f84..e63273e625577 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -20,7 +20,6 @@ use thiserror::Error; /// Identifies an erased settings value. This is used to compare and hash values /// without having to read the underlying value. -/// XXX TODO: Maybe this should be `ErasedSettingsType`? #[derive(Eq, PartialEq, Hash, Copy, Clone)] pub struct ErasedSettingsId { // XXX TODO: Should we store the type id separately or just include it in the @@ -75,7 +74,9 @@ impl ErasedSettings { // Hash by serializing to RON. This means settings are not required to // implement Hash. // XXX TODO: Hashing via RON serialization is very debatable. - // XXX TODO: Could do ron::ser::to_string? Simpler but probably(?) slower. + // XXX TODO: Could do ron::ser::to_string? Simpler and avoids needing + // HashWriter, but probably slower? + // XXX TODO: Could get fancy and implement a Serializer that hashes. let mut hash_writer = HashWriter::new(); ron::ser::to_writer(&mut hash_writer, &settings).expect("XXX TODO?"); let hash = hash_writer.finish(); From 760a32a7e4907adc3ff2112439d8f8a18a113ae4 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 15:08:16 +0100 Subject: [PATCH 23/32] Moved HashWriter. --- crates/bevy_asset/src/path.rs | 50 +++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index e63273e625577..b705c5c3a6698 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -18,29 +18,6 @@ use serde::{de::Visitor, Deserialize, Serialize}; use std::path::{Path, PathBuf}; use thiserror::Error; -/// Identifies an erased settings value. This is used to compare and hash values -/// without having to read the underlying value. -#[derive(Eq, PartialEq, Hash, Copy, Clone)] -pub struct ErasedSettingsId { - // XXX TODO: Should we store the type id separately or just include it in the - // hash? Separately might be nicer for debugging. - type_id: TypeId, - hash: u64, -} - -impl Display for ErasedSettingsId { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - // XXX TODO: Reconsider formatting. Also we're using Debug for type_id... - write!(f, "{:?}/{}", self.type_id, self.hash) - } -} - -/// An erased settings value and its id. -pub struct ErasedSettings { - value: Box, - id: ErasedSettingsId, -} - // XXX TODO: This should go somewhere more shared? struct HashWriter { hasher: DefaultHasher, @@ -69,6 +46,33 @@ impl std::io::Write for HashWriter { } } +/// Identifies an erased settings value. This is used to compare and hash values +/// without having to read the underlying value. +/// +/// XXX TODO: Reconsider auto-deriving Hash. This currently means we hash our +/// own hash + type id. Could be avoided if hash includes the type id. This +/// also starts to look suspiciously like `bevy_platform::Hashed`... +#[derive(Eq, PartialEq, Hash, Copy, Clone)] +pub struct ErasedSettingsId { + // XXX TODO: Should we store the type id separately or just include it in the + // hash? Separately might be nicer for debugging. + type_id: TypeId, + hash: u64, +} + +impl Display for ErasedSettingsId { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + // XXX TODO: Reconsider formatting. Also we're using Debug for type_id... + write!(f, "{:?}/{}", self.type_id, self.hash) + } +} + +/// An erased settings value and its id. +pub struct ErasedSettings { + value: Box, + id: ErasedSettingsId, +} + impl ErasedSettings { pub fn new(settings: S) -> ErasedSettings { // Hash by serializing to RON. This means settings are not required to From 888779b2aef02bf7725890c41411c7bc39b4f740 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 15:55:09 +0100 Subject: [PATCH 24/32] Moved `HashWriter` into its own module. --- crates/bevy_asset/src/hash_writer.rs | 31 ++++++++++++++++++++++++++ crates/bevy_asset/src/lib.rs | 1 + crates/bevy_asset/src/path.rs | 33 ++-------------------------- 3 files changed, 34 insertions(+), 31 deletions(-) create mode 100644 crates/bevy_asset/src/hash_writer.rs diff --git a/crates/bevy_asset/src/hash_writer.rs b/crates/bevy_asset/src/hash_writer.rs new file mode 100644 index 0000000000000..4d9b8a5492a48 --- /dev/null +++ b/crates/bevy_asset/src/hash_writer.rs @@ -0,0 +1,31 @@ +// XXX TODO: Does this deserve its own file? Where should the file go? + +use bevy_platform::hash::{DefaultHasher, FixedHasher}; +use core::hash::{BuildHasher, Hasher}; + +pub(crate) struct HashWriter { + hasher: DefaultHasher, +} + +impl HashWriter { + pub fn new() -> Self { + HashWriter { + hasher: FixedHasher.build_hasher(), + } + } + + pub fn finish(self) -> u64 { + self.hasher.finish() + } +} + +impl std::io::Write for HashWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.hasher.write(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 98cfd527c2086..2c1b79341678a 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -178,6 +178,7 @@ mod direct_access_ext; mod event; mod folder; mod handle; +mod hash_writer; mod id; mod loader; mod loader_builders; diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index b705c5c3a6698..9429ac9f17df8 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -1,4 +1,4 @@ -use crate::{io::AssetSourceId, meta::Settings}; +use crate::{hash_writer::HashWriter, io::AssetSourceId, meta::Settings}; use alloc::{ borrow::ToOwned, boxed::Box, @@ -6,46 +6,17 @@ use alloc::{ sync::Arc, }; use atomicow::CowArc; -use bevy_platform::hash::{DefaultHasher, FixedHasher}; use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize}; use core::{ any::TypeId, fmt::{Debug, Display}, - hash::{BuildHasher, Hash, Hasher}, + hash::{Hash, Hasher}, ops::Deref, }; use serde::{de::Visitor, Deserialize, Serialize}; use std::path::{Path, PathBuf}; use thiserror::Error; -// XXX TODO: This should go somewhere more shared? -struct HashWriter { - hasher: DefaultHasher, -} - -impl HashWriter { - fn new() -> Self { - HashWriter { - hasher: FixedHasher.build_hasher(), - } - } - - fn finish(self) -> u64 { - self.hasher.finish() - } -} - -impl std::io::Write for HashWriter { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - self.hasher.write(buf); - Ok(buf.len()) - } - - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } -} - /// Identifies an erased settings value. This is used to compare and hash values /// without having to read the underlying value. /// From 90218b20a209effb5398f0431a560a7283dd3e1b Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Sat, 10 May 2025 16:04:09 +0100 Subject: [PATCH 25/32] Reverted doc example. --- crates/bevy_gltf/src/loader/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index fd7c55f73f632..d881d7554ab63 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -162,11 +162,11 @@ pub struct GltfLoader { /// # use bevy_asset::{AssetPath, AssetServer, Handle}; /// # use bevy_gltf::*; /// # let asset_server: AssetServer = panic!(); -/// let gltf_handle: Handle = asset_server.load( -/// AssetPath::from("my.gltf").with_settings(GltfLoaderSettings { -/// load_cameras: false, -/// ..Default::default() -/// }) +/// let gltf_handle: Handle = asset_server.load_with_settings( +/// "my.gltf", +/// |s: &mut GltfLoaderSettings| { +/// s.load_cameras = false; +/// } /// ); /// ``` #[derive(Serialize, Deserialize, Clone)] From 4d817531451339f756106ff90e0c717ea10696fc Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Mon, 12 May 2025 10:12:35 +0100 Subject: [PATCH 26/32] Fixed the `alter_sprite` and `alter_mesh` examples. They needed to consistently use `load_with_settings` rather than mixing them with `load`. --- examples/asset/alter_mesh.rs | 47 +++++++++++++++++++-------------- examples/asset/alter_sprite.rs | 48 +++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 40 deletions(-) diff --git a/examples/asset/alter_mesh.rs b/examples/asset/alter_mesh.rs index 7462c47ae1f85..722b67d5ab2c4 100644 --- a/examples/asset/alter_mesh.rs +++ b/examples/asset/alter_mesh.rs @@ -47,6 +47,28 @@ impl Shape { #[derive(Component, Debug)] struct Left; +// A function that can be passed to `AssetServer::load_with_settings` and sets +// our desired `RenderAssetUsages`. +// +// `RenderAssetUsages::all()` is already the default, so this function could have been +// omitted and `load_with_settings` replaced with a simple `load`. It's helpful to know it +// exists, however. +// +// `RenderAssetUsages` tells Bevy whether to keep the data around: +// - for the GPU (`RenderAssetUsages::RENDER_WORLD`), +// - for the CPU (`RenderAssetUsages::MAIN_WORLD`), +// - or both. +// `RENDER_WORLD` is necessary to render the mesh, `MAIN_WORLD` is necessary to inspect +// and modify the mesh (via `ResMut>`). +// +// Since most games will not need to modify meshes at runtime, many developers opt to pass +// only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the mesh in +// RAM. For this example however, this would not work, as we need to inspect and modify the +// mesh at runtime. +fn settings(settings: &mut GltfLoaderSettings) { + settings.load_meshes = RenderAssetUsages::all(); +} + fn setup( mut commands: Commands, asset_server: Res, @@ -55,7 +77,7 @@ fn setup( let left_shape = Shape::Cube; let right_shape = Shape::Cube; - // In normal use, you can call `asset_server.load`, however see below for an explanation of + // In normal use, you can call `asset_server.load`, however see above for an explanation of // `RenderAssetUsages`. let left_shape_model = asset_server.load_with_settings( GltfAssetLabel::Primitive { @@ -69,30 +91,16 @@ fn setup( primitive: 0, } .from_asset(left_shape.get_model_path()), - // `RenderAssetUsages::all()` is already the default, so the line below could be omitted. - // It's helpful to know it exists, however. - // - // `RenderAssetUsages` tell Bevy whether to keep the data around: - // - for the GPU (`RenderAssetUsages::RENDER_WORLD`), - // - for the CPU (`RenderAssetUsages::MAIN_WORLD`), - // - or both. - // `RENDER_WORLD` is necessary to render the mesh, `MAIN_WORLD` is necessary to inspect - // and modify the mesh (via `ResMut>`). - // - // Since most games will not need to modify meshes at runtime, many developers opt to pass - // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the mesh in - // RAM. For this example however, this would not work, as we need to inspect and modify the - // mesh at runtime. - |settings: &mut GltfLoaderSettings| settings.load_meshes = RenderAssetUsages::all(), + settings, ); - // Here, we rely on the default loader settings to achieve a similar result to the above. - let right_shape_model = asset_server.load( + let right_shape_model = asset_server.load_with_settings( GltfAssetLabel::Primitive { mesh: 0, primitive: 0, } .from_asset(right_shape.get_model_path()), + settings, ); // Add a material asset directly to the materials storage @@ -161,12 +169,13 @@ fn alter_handle( // Modify the handle associated with the Shape 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. - mesh.0 = asset_server.load( + mesh.0 = asset_server.load_with_settings( GltfAssetLabel::Primitive { mesh: 0, primitive: 0, } .from_asset(shape.get_model_path()), + settings, ); } diff --git a/examples/asset/alter_sprite.rs b/examples/asset/alter_sprite.rs index d47c303921521..ce9056e426231 100644 --- a/examples/asset/alter_sprite.rs +++ b/examples/asset/alter_sprite.rs @@ -45,29 +45,34 @@ impl Bird { #[derive(Component, Debug)] struct Left; +// A function that can be passed to `AssetServer::load_with_settings` and sets +// our desired `RenderAssetUsages`. +// +// `RenderAssetUsages::all()` is already the default, so this function could have been +// omitted and `load_with_settings` replaced with a simple `load`. It's helpful to know it +// exists, however. +// +// `RenderAssetUsages` tells Bevy whether to keep the data around: +// - for the GPU (`RenderAssetUsages::RENDER_WORLD`), +// - for the CPU (`RenderAssetUsages::MAIN_WORLD`), +// - or both. +// `RENDER_WORLD` is necessary to render the image, `MAIN_WORLD` is necessary to inspect +// and modify the image (via `ResMut>`). +// +// Since most games will not need to modify textures at runtime, many developers opt to pass +// only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the image in +// RAM. For this example however, this would not work, as we need to inspect and modify the +// image at runtime. +fn settings(settings: &mut ImageLoaderSettings) { + settings.asset_usage = RenderAssetUsages::all(); +} + fn setup(mut commands: Commands, asset_server: Res) { let bird_left = Bird::Normal; let bird_right = Bird::Normal; commands.spawn(Camera2d); - let texture_left = asset_server.load_with_settings( - bird_left.get_texture_path(), - // `RenderAssetUsages::all()` is already the default, so the line below could be omitted. - // It's helpful to know it exists, however. - // - // `RenderAssetUsages` tell Bevy whether to keep the data around: - // - for the GPU (`RenderAssetUsages::RENDER_WORLD`), - // - for the CPU (`RenderAssetUsages::MAIN_WORLD`), - // - or both. - // `RENDER_WORLD` is necessary to render the image, `MAIN_WORLD` is necessary to inspect - // and modify the image (via `ResMut>`). - // - // Since most games will not need to modify textures at runtime, many developers opt to pass - // only `RENDER_WORLD`. This is more memory efficient, as we don't need to keep the image in - // RAM. For this example however, this would not work, as we need to inspect and modify the - // image at runtime. - |settings: &mut ImageLoaderSettings| settings.asset_usage = RenderAssetUsages::all(), - ); + let texture_left = asset_server.load_with_settings(bird_left.get_texture_path(), settings); commands.spawn(( Name::new("Bird Left"), @@ -81,8 +86,9 @@ fn setup(mut commands: Commands, asset_server: Res) { commands.spawn(( Name::new("Bird Right"), - // In contrast to the above, here we rely on the default `RenderAssetUsages` loader setting - Sprite::from_image(asset_server.load(bird_right.get_texture_path())), + Sprite::from_image( + asset_server.load_with_settings(bird_right.get_texture_path(), settings), + ), Transform::from_xyz(200.0, 0.0, 0.0), bird_right, )); @@ -118,7 +124,7 @@ 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. - sprite.image = asset_server.load(bird.get_texture_path()); + sprite.image = asset_server.load_with_settings(bird.get_texture_path(), settings); } fn alter_asset(mut images: ResMut>, left_bird: Single<&Sprite, With>) { From 810ad787ef9bf2883b5d4b4f181c38177bdaae08 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Mon, 12 May 2025 13:25:12 +0100 Subject: [PATCH 27/32] Added proper tests for HashWriter. Currently failing as I suspect it's dependent on how the writes are chunked. --- crates/bevy_asset/src/hash_writer.rs | 54 ++++++++++++++++++++++++++-- crates/bevy_asset/src/lib.rs | 3 +- crates/bevy_asset/src/path.rs | 2 +- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/crates/bevy_asset/src/hash_writer.rs b/crates/bevy_asset/src/hash_writer.rs index 4d9b8a5492a48..6fab3dd65f738 100644 --- a/crates/bevy_asset/src/hash_writer.rs +++ b/crates/bevy_asset/src/hash_writer.rs @@ -1,19 +1,36 @@ // XXX TODO: Does this deserve its own file? Where should the file go? +// XXX TODO: Annoyingly this has to be pub to support doc tests? Should be pub(crate). use bevy_platform::hash::{DefaultHasher, FixedHasher}; use core::hash::{BuildHasher, Hasher}; -pub(crate) struct HashWriter { +/// A `std::io::Write` implementation that hashes the inputs. +/// +/// This is typically used to hash something without having to write a temporary +/// buffer. For example, it can be used to hash the output of a serializer: +/// +/// ``` +/// # use bevy_asset::hash_writer::HashWriter; +/// # use bevy_platform::hash::FixedHasher; +/// # use std::hash::BuildHasher; +/// # let value = 0u32; +/// let mut hash_writer = HashWriter::default(); +/// ron::ser::to_writer(&mut hash_writer, &value); +/// let hash: u64 = hash_writer.finish(); +/// ``` +pub struct HashWriter { hasher: DefaultHasher, } -impl HashWriter { - pub fn new() -> Self { +impl Default for HashWriter { + fn default() -> Self { HashWriter { hasher: FixedHasher.build_hasher(), } } +} +impl HashWriter { pub fn finish(self) -> u64 { self.hasher.finish() } @@ -21,6 +38,7 @@ impl HashWriter { impl std::io::Write for HashWriter { fn write(&mut self, buf: &[u8]) -> std::io::Result { + std::dbg!(buf); self.hasher.write(buf); Ok(buf.len()) } @@ -29,3 +47,33 @@ impl std::io::Write for HashWriter { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hash_writer_equivalence() { + #[derive(serde::Serialize)] + struct Test { + s: &'static str, + i: u32, + } + + let value = Test { + s: "hello", + i: 1234, + }; + + let mut hash_writer = HashWriter::default(); + ron::ser::to_writer(&mut hash_writer, &value).unwrap(); + let hash: u64 = hash_writer.finish(); + + let mut vec = alloc::vec::Vec::::new(); + ron::ser::to_writer(&mut vec, &value).unwrap(); + std::dbg!(&vec); + let mut vec_hasher = FixedHasher.build_hasher(); + vec_hasher.write(&vec); + assert_eq!(hash, vec_hasher.finish()); + } +} diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index 2c1b79341678a..b7883bde58c0c 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -153,6 +153,8 @@ extern crate std; extern crate self as bevy_asset; pub mod io; +// XXX TODO: Would prefer this not to be pub. See comment at top of `hash_writer.rs`. +pub mod hash_writer; pub mod meta; pub mod processor; pub mod saver; @@ -178,7 +180,6 @@ mod direct_access_ext; mod event; mod folder; mod handle; -mod hash_writer; mod id; mod loader; mod loader_builders; diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 9429ac9f17df8..93a9f07e5b1c7 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -52,7 +52,7 @@ impl ErasedSettings { // XXX TODO: Could do ron::ser::to_string? Simpler and avoids needing // HashWriter, but probably slower? // XXX TODO: Could get fancy and implement a Serializer that hashes. - let mut hash_writer = HashWriter::new(); + let mut hash_writer = HashWriter::default(); ron::ser::to_writer(&mut hash_writer, &settings).expect("XXX TODO?"); let hash = hash_writer.finish(); From 65d5e28bce316ae1bace60246938fa7466546640 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Mon, 12 May 2025 13:34:21 +0100 Subject: [PATCH 28/32] Removed `HashWriter` and went back to serializing a string. Seems like `HashWriter` is flawed because separate calls to FixedHasher's write will give a different result from accumulating all the bytes and writing once. Maybe that's fine since the hash is not supposed to be persistent, but I don't want to take a chance. Could also consider a HashWriter that consistently writes the same size slices via buffering. --- crates/bevy_asset/src/hash_writer.rs | 79 ---------------------------- crates/bevy_asset/src/lib.rs | 2 - crates/bevy_asset/src/path.rs | 14 +++-- 3 files changed, 6 insertions(+), 89 deletions(-) delete mode 100644 crates/bevy_asset/src/hash_writer.rs diff --git a/crates/bevy_asset/src/hash_writer.rs b/crates/bevy_asset/src/hash_writer.rs deleted file mode 100644 index 6fab3dd65f738..0000000000000 --- a/crates/bevy_asset/src/hash_writer.rs +++ /dev/null @@ -1,79 +0,0 @@ -// XXX TODO: Does this deserve its own file? Where should the file go? -// XXX TODO: Annoyingly this has to be pub to support doc tests? Should be pub(crate). - -use bevy_platform::hash::{DefaultHasher, FixedHasher}; -use core::hash::{BuildHasher, Hasher}; - -/// A `std::io::Write` implementation that hashes the inputs. -/// -/// This is typically used to hash something without having to write a temporary -/// buffer. For example, it can be used to hash the output of a serializer: -/// -/// ``` -/// # use bevy_asset::hash_writer::HashWriter; -/// # use bevy_platform::hash::FixedHasher; -/// # use std::hash::BuildHasher; -/// # let value = 0u32; -/// let mut hash_writer = HashWriter::default(); -/// ron::ser::to_writer(&mut hash_writer, &value); -/// let hash: u64 = hash_writer.finish(); -/// ``` -pub struct HashWriter { - hasher: DefaultHasher, -} - -impl Default for HashWriter { - fn default() -> Self { - HashWriter { - hasher: FixedHasher.build_hasher(), - } - } -} - -impl HashWriter { - pub fn finish(self) -> u64 { - self.hasher.finish() - } -} - -impl std::io::Write for HashWriter { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - std::dbg!(buf); - self.hasher.write(buf); - Ok(buf.len()) - } - - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn hash_writer_equivalence() { - #[derive(serde::Serialize)] - struct Test { - s: &'static str, - i: u32, - } - - let value = Test { - s: "hello", - i: 1234, - }; - - let mut hash_writer = HashWriter::default(); - ron::ser::to_writer(&mut hash_writer, &value).unwrap(); - let hash: u64 = hash_writer.finish(); - - let mut vec = alloc::vec::Vec::::new(); - ron::ser::to_writer(&mut vec, &value).unwrap(); - std::dbg!(&vec); - let mut vec_hasher = FixedHasher.build_hasher(); - vec_hasher.write(&vec); - assert_eq!(hash, vec_hasher.finish()); - } -} diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs index b7883bde58c0c..98cfd527c2086 100644 --- a/crates/bevy_asset/src/lib.rs +++ b/crates/bevy_asset/src/lib.rs @@ -153,8 +153,6 @@ extern crate std; extern crate self as bevy_asset; pub mod io; -// XXX TODO: Would prefer this not to be pub. See comment at top of `hash_writer.rs`. -pub mod hash_writer; pub mod meta; pub mod processor; pub mod saver; diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 93a9f07e5b1c7..8fd9e5a9d742b 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -1,4 +1,4 @@ -use crate::{hash_writer::HashWriter, io::AssetSourceId, meta::Settings}; +use crate::{io::AssetSourceId, meta::Settings}; use alloc::{ borrow::ToOwned, boxed::Box, @@ -6,11 +6,12 @@ use alloc::{ sync::Arc, }; use atomicow::CowArc; +use bevy_platform::hash::FixedHasher; use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize}; use core::{ any::TypeId, fmt::{Debug, Display}, - hash::{Hash, Hasher}, + hash::{BuildHasher, Hash, Hasher}, ops::Deref, }; use serde::{de::Visitor, Deserialize, Serialize}; @@ -49,12 +50,9 @@ impl ErasedSettings { // Hash by serializing to RON. This means settings are not required to // implement Hash. // XXX TODO: Hashing via RON serialization is very debatable. - // XXX TODO: Could do ron::ser::to_string? Simpler and avoids needing - // HashWriter, but probably slower? - // XXX TODO: Could get fancy and implement a Serializer that hashes. - let mut hash_writer = HashWriter::default(); - ron::ser::to_writer(&mut hash_writer, &settings).expect("XXX TODO?"); - let hash = hash_writer.finish(); + // XXX TODO: Allocating a string is bad. Should consider a small vec or + // hashing directly through the serializer. + let hash = FixedHasher.hash_one(ron::ser::to_string(&settings).expect("XXX TODO?")); ErasedSettings { value: Box::new(settings), From 27b8d21a09fc09cf4a4fbe9e368ed5404821bbad Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Mon, 12 May 2025 14:43:11 +0100 Subject: [PATCH 29/32] Updated comments. --- crates/bevy_asset/src/meta.rs | 2 +- crates/bevy_asset/src/path.rs | 14 +++++++------- crates/bevy_asset/src/server/mod.rs | 5 +++-- crates/bevy_gltf/src/loader/mod.rs | 4 ++-- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/crates/bevy_asset/src/meta.rs b/crates/bevy_asset/src/meta.rs index 01d8e51879a46..a8e1bc258f213 100644 --- a/crates/bevy_asset/src/meta.rs +++ b/crates/bevy_asset/src/meta.rs @@ -122,7 +122,7 @@ pub struct ProcessedInfoMinimal { pub trait AssetMetaDyn: Downcast + Send + Sync { /// Returns a reference to the [`AssetLoader`] settings, if they exist. fn loader_settings(&self) -> Option<&dyn Settings>; - /// XXX TODO. + /// XXX TODO: Document. fn apply_settings(&mut self, settings: &dyn Settings); /// Serializes the internal [`AssetMeta`]. fn serialize(&self) -> Vec; diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 8fd9e5a9d742b..7a4bcd3eac34a 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -23,7 +23,7 @@ use thiserror::Error; /// /// XXX TODO: Reconsider auto-deriving Hash. This currently means we hash our /// own hash + type id. Could be avoided if hash includes the type id. This -/// also starts to look suspiciously like `bevy_platform::Hashed`... +/// also starts to look suspiciously like `bevy_platform::Hashed`. #[derive(Eq, PartialEq, Hash, Copy, Clone)] pub struct ErasedSettingsId { // XXX TODO: Should we store the type id separately or just include it in the @@ -34,7 +34,7 @@ pub struct ErasedSettingsId { impl Display for ErasedSettingsId { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - // XXX TODO: Reconsider formatting. Also we're using Debug for type_id... + // XXX TODO: Reconsider formatting. Also we're using Debug for type_id. write!(f, "{:?}/{}", self.type_id, self.hash) } } @@ -222,7 +222,7 @@ impl<'a> AssetPath<'a> { }, path: CowArc::Borrowed(path), label: label.map(CowArc::Borrowed), - settings: None, // XXX TODO? + settings: None, // XXX TODO: Do we need to document this behaviour? }) } @@ -322,7 +322,7 @@ impl<'a> AssetPath<'a> { path: CowArc::Borrowed(path), source: AssetSourceId::Default, label: None, - settings: None, // XXX TODO? + settings: None, } } @@ -440,7 +440,7 @@ impl<'a> AssetPath<'a> { source: self.source.clone(), label: None, path, - settings: self.settings.clone(), // XXX TODO: Bit weird? + settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behaviour. }) } @@ -454,7 +454,7 @@ impl<'a> AssetPath<'a> { source: self.source.into_owned(), path: self.path.into_owned(), label: self.label.map(CowArc::into_owned), - settings: self.settings.clone(), // XXX TODO? + settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behaviour. } } @@ -576,7 +576,7 @@ impl<'a> AssetPath<'a> { }, path: CowArc::Owned(result_path.into()), label: rlabel.map(|l| CowArc::Owned(l.into())), - settings: self.settings.clone(), // XXX TODO? + settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behaviour. }) } } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 3c0a68a390e94..f4f944de2d3ba 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -436,8 +436,9 @@ impl AssetServer { self.load_with_meta_transform(path.into().with_settings_fn(settings), guard, true) } - // XXX TODO: This should probably be renamed, but I'm not sure what to. - // `load_internal` is taken. `load_with_params`? + // XXX TODO: This should probably be renamed if `MetaTransform` is removed, + // but I'm not sure what would be a good name. `load_internal` is taken. + // Maybe `load_with_params`? pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>( &self, path: impl Into>, diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index d881d7554ab63..6cda331116e28 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -529,7 +529,7 @@ async fn load_gltf<'a, 'b, 'c>( // later in the loader when looking up handles for materials. However this would mean // that the material's load context would no longer track those images as dependencies. // - // XXX TODO: This has been changed to use texture handles. But the comment above suggests that will break something. + // XXX TODO: Don't reuse texture handles. Should calculate `AssetPath` once. let mut texture_handles = Vec::new(); if gltf.textures().len() == 1 || cfg!(target_arch = "wasm32") { for texture in gltf.textures() { @@ -1053,7 +1053,7 @@ fn load_material( texture_handles: &[Handle], ) -> Handle { let material_label = material_label(material, is_scale_inverted); - // XXX TODO: Unclear if this is needed assume we follow through on using texture handles? + // XXX TODO: Double check this works correctly if we rearrange texture loading. load_context.labeled_asset_scope(material_label.to_string(), |_| { let pbr = material.pbr_metallic_roughness(); From 85d7fea1a6f97e88935fc23ed68272d7ab246d85 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Mon, 12 May 2025 15:07:27 +0100 Subject: [PATCH 30/32] Fixed typos. --- crates/bevy_asset/src/path.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_asset/src/path.rs b/crates/bevy_asset/src/path.rs index 7a4bcd3eac34a..854ab202c28b2 100644 --- a/crates/bevy_asset/src/path.rs +++ b/crates/bevy_asset/src/path.rs @@ -222,7 +222,7 @@ impl<'a> AssetPath<'a> { }, path: CowArc::Borrowed(path), label: label.map(CowArc::Borrowed), - settings: None, // XXX TODO: Do we need to document this behaviour? + settings: None, // XXX TODO: Do we need to document this behavior? }) } @@ -440,7 +440,7 @@ impl<'a> AssetPath<'a> { source: self.source.clone(), label: None, path, - settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behaviour. + settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behavior. }) } @@ -454,7 +454,7 @@ impl<'a> AssetPath<'a> { source: self.source.into_owned(), path: self.path.into_owned(), label: self.label.map(CowArc::into_owned), - settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behaviour. + settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behavior. } } @@ -576,7 +576,7 @@ impl<'a> AssetPath<'a> { }, path: CowArc::Owned(result_path.into()), label: rlabel.map(|l| CowArc::Owned(l.into())), - settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behaviour. + settings: self.settings.clone(), // XXX TODO: Reconsider `Arc` behavior. }) } } From e05641af4669b9536ce79f83595ddbe6bcd89ad4 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Mon, 12 May 2025 15:09:37 +0100 Subject: [PATCH 31/32] Fixed imports in `asset_processing` example. --- examples/asset/processing/asset_processing.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/asset/processing/asset_processing.rs b/examples/asset/processing/asset_processing.rs index 4b4fc25782dc8..a7bb770c8f1f2 100644 --- a/examples/asset/processing/asset_processing.rs +++ b/examples/asset/processing/asset_processing.rs @@ -1,6 +1,7 @@ //! This example illustrates how to define custom `AssetLoader`s, `AssetTransformer`s, and `AssetSaver`s, how to configure them, and how to register asset processors. use bevy::{ + asset::AssetPath, asset::{ embedded_asset, io::{Reader, Writer}, @@ -12,7 +13,6 @@ use bevy::{ prelude::*, reflect::TypePath, }; -use bevy_asset::AssetPath; use serde::{Deserialize, Serialize}; use std::convert::Infallible; use thiserror::Error; From b4c942694786f949a6be5069be016b825cf38bd8 Mon Sep 17 00:00:00 2001 From: Greeble <166992735+greeble-dev@users.noreply.github.com> Date: Tue, 13 May 2025 14:12:33 +0100 Subject: [PATCH 32/32] Redid glTF changes. Now we keep around both the path and the handle - the material code can reload the path to register the dependency, and also verify that the handle matches. --- .../extensions/khr_materials_anisotropy.rs | 13 ++- .../extensions/khr_materials_clearcoat.rs | 23 +++-- .../extensions/khr_materials_specular.rs | 20 ++-- .../bevy_gltf/src/loader/gltf_ext/material.rs | 9 +- .../bevy_gltf/src/loader/gltf_ext/texture.rs | 26 +++-- crates/bevy_gltf/src/loader/mod.rs | 94 ++++++++++++------- 6 files changed, 126 insertions(+), 59 deletions(-) diff --git a/crates/bevy_gltf/src/loader/extensions/khr_materials_anisotropy.rs b/crates/bevy_gltf/src/loader/extensions/khr_materials_anisotropy.rs index e85fc9a0074f1..9b3ed91fb6c9f 100644 --- a/crates/bevy_gltf/src/loader/extensions/khr_materials_anisotropy.rs +++ b/crates/bevy_gltf/src/loader/extensions/khr_materials_anisotropy.rs @@ -1,12 +1,16 @@ -use bevy_asset::Handle; -use bevy_image::Image; +use bevy_asset::LoadContext; + use gltf::{Document, Material}; use serde_json::Value; +use crate::loader::LoadedTexture; + #[cfg(feature = "pbr_anisotropy_texture")] use { crate::loader::gltf_ext::{material::uv_channel, texture::texture_handle_from_info}, + bevy_asset::Handle, + bevy_image::Image, bevy_pbr::UvChannel, gltf::json::texture::Info, serde_json::value, @@ -36,7 +40,8 @@ impl AnisotropyExtension { reason = "Depending on what features are used to compile this crate, certain parameters may end up unused." )] pub(crate) fn parse( - _texture_handles: &[Handle], + load_context: &mut LoadContext, + loaded_textures: &[LoadedTexture], document: &Document, material: &Material, ) -> Option { @@ -52,7 +57,7 @@ impl AnisotropyExtension { .map(|json_info| { ( uv_channel(material, "anisotropy", json_info.tex_coord), - texture_handle_from_info(&json_info, document, _texture_handles), + texture_handle_from_info(&json_info, document, load_context, loaded_textures), ) }) .unzip(); diff --git a/crates/bevy_gltf/src/loader/extensions/khr_materials_clearcoat.rs b/crates/bevy_gltf/src/loader/extensions/khr_materials_clearcoat.rs index fc76a7a0a5334..45fdbbc28598d 100644 --- a/crates/bevy_gltf/src/loader/extensions/khr_materials_clearcoat.rs +++ b/crates/bevy_gltf/src/loader/extensions/khr_materials_clearcoat.rs @@ -1,11 +1,16 @@ -use bevy_asset::Handle; -use bevy_image::Image; +use bevy_asset::LoadContext; + use gltf::{Document, Material}; use serde_json::Value; +use crate::loader::LoadedTexture; + #[cfg(feature = "pbr_multi_layer_material_textures")] -use {crate::loader::gltf_ext::material::parse_material_extension_texture, bevy_pbr::UvChannel}; +use { + crate::loader::gltf_ext::material::parse_material_extension_texture, bevy_asset::Handle, + bevy_image::Image, bevy_pbr::UvChannel, +}; /// Parsed data from the `KHR_materials_clearcoat` extension. /// @@ -39,7 +44,8 @@ impl ClearcoatExtension { reason = "Depending on what features are used to compile this crate, certain parameters may end up unused." )] pub(crate) fn parse( - _texture_handles: &[Handle], + load_context: &mut LoadContext, + loaded_textures: &[LoadedTexture], document: &Document, material: &Material, ) -> Option { @@ -51,7 +57,8 @@ impl ClearcoatExtension { #[cfg(feature = "pbr_multi_layer_material_textures")] let (clearcoat_channel, clearcoat_texture) = parse_material_extension_texture( material, - _texture_handles, + load_context, + loaded_textures, document, extension, "clearcoatTexture", @@ -62,7 +69,8 @@ impl ClearcoatExtension { let (clearcoat_roughness_channel, clearcoat_roughness_texture) = parse_material_extension_texture( material, - _texture_handles, + load_context, + loaded_textures, document, extension, "clearcoatRoughnessTexture", @@ -72,7 +80,8 @@ impl ClearcoatExtension { #[cfg(feature = "pbr_multi_layer_material_textures")] let (clearcoat_normal_channel, clearcoat_normal_texture) = parse_material_extension_texture( material, - _texture_handles, + load_context, + loaded_textures, document, extension, "clearcoatNormalTexture", diff --git a/crates/bevy_gltf/src/loader/extensions/khr_materials_specular.rs b/crates/bevy_gltf/src/loader/extensions/khr_materials_specular.rs index 1287e4366c5c7..dc7571d95ebdd 100644 --- a/crates/bevy_gltf/src/loader/extensions/khr_materials_specular.rs +++ b/crates/bevy_gltf/src/loader/extensions/khr_materials_specular.rs @@ -1,11 +1,16 @@ -use bevy_asset::Handle; -use bevy_image::Image; +use bevy_asset::LoadContext; + use gltf::{Document, Material}; use serde_json::Value; +use crate::loader::LoadedTexture; + #[cfg(feature = "pbr_specular_textures")] -use {crate::loader::gltf_ext::material::parse_material_extension_texture, bevy_pbr::UvChannel}; +use { + crate::loader::gltf_ext::material::parse_material_extension_texture, bevy_asset::Handle, + bevy_image::Image, bevy_pbr::UvChannel, +}; /// Parsed data from the `KHR_materials_specular` extension. /// @@ -39,7 +44,8 @@ pub(crate) struct SpecularExtension { impl SpecularExtension { pub(crate) fn parse( - _texture_handles: &[Handle], + _load_context: &mut LoadContext, + _loaded_textures: &[LoadedTexture], _document: &Document, material: &Material, ) -> Option { @@ -51,7 +57,8 @@ impl SpecularExtension { #[cfg(feature = "pbr_specular_textures")] let (_specular_channel, _specular_texture) = parse_material_extension_texture( material, - _texture_handles, + _load_context, + _loaded_textures, _document, extension, "specularTexture", @@ -61,7 +68,8 @@ impl SpecularExtension { #[cfg(feature = "pbr_specular_textures")] let (_specular_color_channel, _specular_color_texture) = parse_material_extension_texture( material, - _texture_handles, + _load_context, + _loaded_textures, _document, extension, "specularColorTexture", diff --git a/crates/bevy_gltf/src/loader/gltf_ext/material.rs b/crates/bevy_gltf/src/loader/gltf_ext/material.rs index ada870cd154d0..47df3539ba2b5 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/material.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/material.rs @@ -16,7 +16,8 @@ use super::texture::texture_transform_to_affine2; ))] use { super::texture::texture_handle_from_info, - bevy_asset::Handle, + crate::loader::LoadedTexture, + bevy_asset::{Handle, LoadContext}, bevy_image::Image, gltf::Document, serde_json::{Map, Value}, @@ -30,7 +31,8 @@ use { ))] pub(crate) fn parse_material_extension_texture( material: &Material, - texture_handles: &[Handle], + load_context: &mut LoadContext, + loaded_textures: &[LoadedTexture], document: &Document, extension: &Map, texture_name: &str, @@ -45,7 +47,8 @@ pub(crate) fn parse_material_extension_texture( Some(texture_handle_from_info( &json_info, document, - texture_handles, + load_context, + loaded_textures, )), ), None => (UvChannel::default(), None), diff --git a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs index 0b0414f1fdc98..f5677d54f1d05 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs @@ -1,4 +1,4 @@ -use bevy_asset::Handle; +use bevy_asset::{Handle, LoadContext}; use bevy_image::{Image, ImageAddressMode, ImageFilterMode, ImageSamplerDescriptor}; use bevy_math::Affine2; @@ -11,12 +11,25 @@ use gltf::texture::{MagFilter, MinFilter, Texture, TextureTransform, WrappingMod ))] use gltf::{json::texture::Info, Document}; +use crate::loader::{LabelOrAssetPath, LoadedTexture}; + +// XXX TODO: Documentation. pub(crate) fn texture_handle( texture: &Texture<'_>, - texture_handles: &[Handle], + load_context: &mut LoadContext, + loaded_textures: &[LoadedTexture], ) -> Handle { - // XXX TODO: Handle out of range request? - texture_handles[texture.index()].clone() + let loaded_texture = &loaded_textures[texture.index()]; // XXX TODO: Guard against invalid indices? + + let handle = match &loaded_texture.path { + LabelOrAssetPath::Label(label) => load_context.get_label_handle(label.to_string()), + LabelOrAssetPath::AssetPath(path) => load_context.load(path), + }; + + // XXX TODO: Document and decide on error handling. + assert_eq!(handle, loaded_texture.handle); + + handle } /// Extracts the texture sampler data from the glTF [`Texture`]. @@ -93,11 +106,12 @@ pub(crate) fn texture_transform_to_affine2(texture_transform: TextureTransform) pub(crate) fn texture_handle_from_info( info: &Info, document: &Document, - texture_handles: &[Handle], + load_context: &mut LoadContext, + loaded_textures: &[LoadedTexture], ) -> Handle { let texture = document .textures() .nth(info.index.value()) .expect("Texture info references a nonexistent texture"); - texture_handle(&texture, texture_handles) + texture_handle(&texture, load_context, loaded_textures) } diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index 6cda331116e28..0b668c9dba52c 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -159,7 +159,7 @@ pub struct GltfLoader { /// /// To load a gltf but exclude the cameras, replace a call to `asset_server.load("my.gltf")` with /// ```no_run -/// # use bevy_asset::{AssetPath, AssetServer, Handle}; +/// # use bevy_asset::{AssetServer, Handle}; /// # use bevy_gltf::*; /// # let asset_server: AssetServer = panic!(); /// let gltf_handle: Handle = asset_server.load_with_settings( @@ -529,8 +529,8 @@ async fn load_gltf<'a, 'b, 'c>( // later in the loader when looking up handles for materials. However this would mean // that the material's load context would no longer track those images as dependencies. // - // XXX TODO: Don't reuse texture handles. Should calculate `AssetPath` once. - let mut texture_handles = Vec::new(); + // XXX TODO: Update comment. + let mut loaded_textures: Vec = Vec::new(); if gltf.textures().len() == 1 || cfg!(target_arch = "wasm32") { for texture in gltf.textures() { let parent_path = load_context.path().parent().unwrap(); @@ -544,7 +544,7 @@ async fn load_gltf<'a, 'b, 'c>( settings, ) .await?; - image.process_loaded_texture(load_context, &mut texture_handles); + image.process_loaded_texture(load_context, &mut loaded_textures); } } else { #[cfg(not(target_arch = "wasm32"))] @@ -571,7 +571,7 @@ async fn load_gltf<'a, 'b, 'c>( .into_iter() .for_each(|result| match result { Ok(image) => { - image.process_loaded_texture(load_context, &mut texture_handles); + image.process_loaded_texture(load_context, &mut loaded_textures); } Err(err) => { warn!("Error loading glTF texture: {}", err); @@ -590,7 +590,7 @@ async fn load_gltf<'a, 'b, 'c>( load_context, &gltf.document, false, - &texture_handles, + &loaded_textures, ); if let Some(name) = material.name() { named_materials.insert(name.into(), handle.clone()); @@ -893,7 +893,7 @@ async fn load_gltf<'a, 'b, 'c>( #[cfg(feature = "bevy_animation")] None, &gltf.document, - &texture_handles, + &loaded_textures, ); if result.is_err() { err = Some(result); @@ -1050,11 +1050,10 @@ fn load_material( load_context: &mut LoadContext, document: &Document, is_scale_inverted: bool, - texture_handles: &[Handle], + loaded_textures: &[LoadedTexture], ) -> Handle { let material_label = material_label(material, is_scale_inverted); - // XXX TODO: Double check this works correctly if we rearrange texture loading. - load_context.labeled_asset_scope(material_label.to_string(), |_| { + load_context.labeled_asset_scope(material_label.to_string(), |load_context| { let pbr = material.pbr_metallic_roughness(); // TODO: handle missing label handle errors here? @@ -1065,7 +1064,7 @@ fn load_material( .unwrap_or_default(); let base_color_texture = pbr .base_color_texture() - .map(|info| texture_handle(&info.texture(), texture_handles)); + .map(|info| texture_handle(&info.texture(), load_context, loaded_textures)); let uv_transform = pbr .base_color_texture() @@ -1079,7 +1078,7 @@ fn load_material( let normal_map_texture: Option> = material.normal_texture().map(|normal_texture| { // TODO: handle normal_texture.scale - texture_handle(&normal_texture.texture(), texture_handles) + texture_handle(&normal_texture.texture(), load_context, loaded_textures) }); let metallic_roughness_channel = pbr @@ -1093,7 +1092,7 @@ fn load_material( uv_transform, "metallic/roughness", ); - texture_handle(&info.texture(), texture_handles) + texture_handle(&info.texture(), load_context, loaded_textures) }); let occlusion_channel = material @@ -1102,7 +1101,7 @@ fn load_material( .unwrap_or_default(); let occlusion_texture = material.occlusion_texture().map(|occlusion_texture| { // TODO: handle occlusion_texture.strength() (a scalar multiplier for occlusion strength) - texture_handle(&occlusion_texture.texture(), texture_handles) + texture_handle(&occlusion_texture.texture(), load_context, loaded_textures) }); let emissive = material.emissive_factor(); @@ -1113,7 +1112,7 @@ fn load_material( let emissive_texture = material.emissive_texture().map(|info| { // TODO: handle occlusion_texture.strength() (a scalar multiplier for occlusion strength) warn_on_differing_texture_transforms(material, &info, uv_transform, "emissive"); - texture_handle(&info.texture(), texture_handles) + texture_handle(&info.texture(), load_context, loaded_textures) }); #[cfg(feature = "pbr_transmission_textures")] @@ -1128,7 +1127,11 @@ fn load_material( let transmission_texture: Option> = transmission .transmission_texture() .map(|transmission_texture| { - texture_handle(&transmission_texture.texture(), texture_handles) + texture_handle( + &transmission_texture.texture(), + load_context, + loaded_textures, + ) }); ( @@ -1159,7 +1162,7 @@ fn load_material( .unwrap_or_default(); let thickness_texture: Option> = volume.thickness_texture().map(|thickness_texture| { - texture_handle(&thickness_texture.texture(), texture_handles) + texture_handle(&thickness_texture.texture(), load_context, loaded_textures) }); ( @@ -1188,20 +1191,24 @@ fn load_material( // Parse the `KHR_materials_clearcoat` extension data if necessary. let clearcoat = - ClearcoatExtension::parse(texture_handles, document, material).unwrap_or_default(); + ClearcoatExtension::parse(load_context, loaded_textures, document, material) + .unwrap_or_default(); // Parse the `KHR_materials_anisotropy` extension data if necessary. let anisotropy = - AnisotropyExtension::parse(texture_handles, document, material).unwrap_or_default(); + AnisotropyExtension::parse(load_context, loaded_textures, document, material) + .unwrap_or_default(); // Parse the `KHR_materials_specular` extension data if necessary. - let specular = - SpecularExtension::parse(texture_handles, document, material).unwrap_or_default(); + let specular = SpecularExtension::parse(load_context, loaded_textures, document, material) + .unwrap_or_default(); // We need to operate in the Linear color space and be willing to exceed 1.0 in our channels let base_emissive = LinearRgba::rgb(emissive[0], emissive[1], emissive[2]); let emissive = base_emissive * material.emissive_strength().unwrap_or(1.0); + // XXX TODO: Need some way to dump the load_context dependencies and confirm they're right. + StandardMaterial { base_color: Color::linear_rgba(color[0], color[1], color[2], color[3]), base_color_channel, @@ -1307,7 +1314,7 @@ fn load_node( #[cfg(feature = "bevy_animation")] animation_roots: &HashSet, #[cfg(feature = "bevy_animation")] mut animation_context: Option, document: &Document, - texture_handles: &[Handle], + loaded_textures: &[LoadedTexture], ) -> Result<(), GltfError> { let mut gltf_error = None; let transform = node_transform(gltf_node); @@ -1421,7 +1428,7 @@ fn load_node( load_context, document, is_scale_inverted, - texture_handles, + loaded_textures, ); } @@ -1580,7 +1587,7 @@ fn load_node( #[cfg(feature = "bevy_animation")] animation_context.clone(), document, - texture_handles, + loaded_textures, ) { gltf_error = Some(err); return; @@ -1707,28 +1714,49 @@ impl ImageOrPath { fn process_loaded_texture( self, load_context: &mut LoadContext, - handles: &mut Vec>, + loaded: &mut Vec, ) { - let handle = match self { + loaded.push(match self { ImageOrPath::Image { label, image } => { - load_context.add_labeled_asset(label.to_string(), image) + let handle = load_context.add_labeled_asset(label.to_string(), image); + LoadedTexture { + handle, + path: LabelOrAssetPath::Label(label), + } } ImageOrPath::Path { path, is_srgb, sampler_descriptor, - } => load_context - .loader() - .load(AssetPath::from(path).with_settings(ImageLoaderSettings { + } => { + let path = AssetPath::from(path).with_settings(ImageLoaderSettings { is_srgb, sampler: ImageSampler::Descriptor(sampler_descriptor.clone()), ..Default::default() - })), - }; - handles.push(handle); + }); + let handle = load_context.loader().load(path.clone()); + LoadedTexture { + handle, + path: LabelOrAssetPath::AssetPath(path), + } + } + }); } } +// XXX TODO: Document. +enum LabelOrAssetPath<'a> { + Label(GltfAssetLabel), + AssetPath(AssetPath<'a>), +} + +// XXX TODO: Document. +struct LoadedTexture<'a> { + handle: Handle, + // XXX TODO: Reconsider variable name? + path: LabelOrAssetPath<'a>, +} + struct PrimitiveMorphAttributesIter<'s>( pub ( Option>,