From 2dc9122a66d3d03f7de115d824a1fa6e91ab4c94 Mon Sep 17 00:00:00 2001 From: axlitEels Date: Sat, 15 Feb 2025 23:36:16 +0300 Subject: [PATCH 01/15] Add image sampler configuration in GLTF loader I can't underrate anisotropic filtering. --- crates/bevy_gltf/src/loader.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 1a8e170ee793f..75d7546e091d9 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -168,6 +168,11 @@ pub struct GltfLoaderSettings { pub load_lights: bool, /// If true, the loader will include the root of the gltf root node. pub include_source: bool, + /// If some, the loader will ignore the gltf sampler data and generate materials with provided ImageSamplerDescriptor. + pub override_sampler: Option, + /// Anisotropic filtering level. Must be a power 1, 2, 4, 8 or 16. + /// Is passed into ImageSamplerDescriptor when parsing gltf sampler data. + pub anisotropy_clamp: u16 } impl Default for GltfLoaderSettings { @@ -178,6 +183,8 @@ impl Default for GltfLoaderSettings { load_cameras: true, load_lights: true, include_source: false, + override_sampler: None, + anisotropy_clamp: 1, } } } @@ -574,7 +581,7 @@ async fn load_gltf<'a, 'b, 'c>( &linear_textures, parent_path, loader.supported_compressed_formats, - settings.load_materials, + settings, ) .await?; process_loaded_texture(load_context, &mut _texture_handles, image); @@ -594,7 +601,7 @@ async fn load_gltf<'a, 'b, 'c>( linear_textures, parent_path, loader.supported_compressed_formats, - settings.load_materials, + settings, ) .await }); @@ -1050,10 +1057,11 @@ async fn load_image<'a, 'b>( linear_textures: &HashSet, parent_path: &'b Path, supported_compressed_formats: CompressedImageFormats, - render_asset_usages: RenderAssetUsages, + settings: &GltfLoaderSettings, ) -> Result { let is_srgb = !linear_textures.contains(&gltf_texture.index()); - let sampler_descriptor = texture_sampler(&gltf_texture); + let sampler_descriptor = settings.override_sampler.clone() + .unwrap_or(texture_sampler(&gltf_texture, settings.anisotropy_clamp)); #[cfg(all(debug_assertions, feature = "dds"))] let name = gltf_texture .name() @@ -1071,7 +1079,7 @@ async fn load_image<'a, 'b>( supported_compressed_formats, is_srgb, ImageSampler::Descriptor(sampler_descriptor), - render_asset_usages, + settings.load_materials, )?; Ok(ImageOrPath::Image { image, @@ -1095,7 +1103,7 @@ async fn load_image<'a, 'b>( supported_compressed_formats, is_srgb, ImageSampler::Descriptor(sampler_descriptor), - render_asset_usages, + settings.load_materials, )?, label: GltfAssetLabel::Texture(gltf_texture.index()), }) @@ -1810,7 +1818,7 @@ fn inverse_bind_matrices_label(skin: &gltf::Skin) -> String { } /// Extracts the texture sampler data from the glTF texture. -fn texture_sampler(texture: &gltf::Texture) -> ImageSamplerDescriptor { +fn texture_sampler(texture: &gltf::Texture, anisotropy_clamp: u16) -> ImageSamplerDescriptor { let gltf_sampler = texture.sampler(); ImageSamplerDescriptor { @@ -1850,6 +1858,8 @@ fn texture_sampler(texture: &gltf::Texture) -> ImageSamplerDescriptor { }) .unwrap_or(ImageSamplerDescriptor::default().mipmap_filter), + anisotropy_clamp, + ..Default::default() } } From 49ba75c0585b00acddd409523858e8c114c89f0f Mon Sep 17 00:00:00 2001 From: axlitEels Date: Sun, 16 Feb 2025 01:33:41 +0300 Subject: [PATCH 02/15] Enforce anisotropy requirements Enforce anisotropy requirements to avoid wgpu Validation Error --- crates/bevy_gltf/src/loader.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 75d7546e091d9..1bce404658853 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -1831,7 +1831,10 @@ fn texture_sampler(texture: &gltf::Texture, anisotropy_clamp: u16) -> ImageSampl MagFilter::Nearest => ImageFilterMode::Nearest, MagFilter::Linear => ImageFilterMode::Linear, }) - .unwrap_or(ImageSamplerDescriptor::default().mag_filter), + .or(Some(ImageSamplerDescriptor::default().mag_filter)) + // Enabling anisotropy with Nearest filters causes wgpu Validation Error + .filter(|_| anisotropy_clamp==1) + .unwrap_or(ImageFilterMode::Linear), min_filter: gltf_sampler .min_filter() @@ -1843,7 +1846,9 @@ fn texture_sampler(texture: &gltf::Texture, anisotropy_clamp: u16) -> ImageSampl | MinFilter::LinearMipmapNearest | MinFilter::LinearMipmapLinear => ImageFilterMode::Linear, }) - .unwrap_or(ImageSamplerDescriptor::default().min_filter), + .or(Some(ImageSamplerDescriptor::default().min_filter)) + .filter(|_| anisotropy_clamp==1) + .unwrap_or(ImageFilterMode::Linear), mipmap_filter: gltf_sampler .min_filter() @@ -1856,7 +1861,9 @@ fn texture_sampler(texture: &gltf::Texture, anisotropy_clamp: u16) -> ImageSampl ImageFilterMode::Linear } }) - .unwrap_or(ImageSamplerDescriptor::default().mipmap_filter), + .or(Some(ImageSamplerDescriptor::default().mipmap_filter)) + .filter(|_| anisotropy_clamp==1) + .unwrap_or(ImageFilterMode::Linear), anisotropy_clamp, From 784903544efbcde1bbaa46289bf59d29aa7f9f66 Mon Sep 17 00:00:00 2001 From: axlitEels Date: Sun, 16 Feb 2025 01:50:06 +0300 Subject: [PATCH 03/15] cargo fmt --- crates/bevy_gltf/src/loader.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 1bce404658853..b9db59a344d2d 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -172,7 +172,7 @@ pub struct GltfLoaderSettings { pub override_sampler: Option, /// Anisotropic filtering level. Must be a power 1, 2, 4, 8 or 16. /// Is passed into ImageSamplerDescriptor when parsing gltf sampler data. - pub anisotropy_clamp: u16 + pub anisotropy_clamp: u16, } impl Default for GltfLoaderSettings { @@ -1060,7 +1060,9 @@ async fn load_image<'a, 'b>( settings: &GltfLoaderSettings, ) -> Result { let is_srgb = !linear_textures.contains(&gltf_texture.index()); - let sampler_descriptor = settings.override_sampler.clone() + let sampler_descriptor = settings + .override_sampler + .clone() .unwrap_or(texture_sampler(&gltf_texture, settings.anisotropy_clamp)); #[cfg(all(debug_assertions, feature = "dds"))] let name = gltf_texture @@ -1833,7 +1835,7 @@ fn texture_sampler(texture: &gltf::Texture, anisotropy_clamp: u16) -> ImageSampl }) .or(Some(ImageSamplerDescriptor::default().mag_filter)) // Enabling anisotropy with Nearest filters causes wgpu Validation Error - .filter(|_| anisotropy_clamp==1) + .filter(|_| anisotropy_clamp == 1) .unwrap_or(ImageFilterMode::Linear), min_filter: gltf_sampler @@ -1847,7 +1849,7 @@ fn texture_sampler(texture: &gltf::Texture, anisotropy_clamp: u16) -> ImageSampl | MinFilter::LinearMipmapLinear => ImageFilterMode::Linear, }) .or(Some(ImageSamplerDescriptor::default().min_filter)) - .filter(|_| anisotropy_clamp==1) + .filter(|_| anisotropy_clamp == 1) .unwrap_or(ImageFilterMode::Linear), mipmap_filter: gltf_sampler @@ -1862,11 +1864,11 @@ fn texture_sampler(texture: &gltf::Texture, anisotropy_clamp: u16) -> ImageSampl } }) .or(Some(ImageSamplerDescriptor::default().mipmap_filter)) - .filter(|_| anisotropy_clamp==1) + .filter(|_| anisotropy_clamp == 1) .unwrap_or(ImageFilterMode::Linear), anisotropy_clamp, - + ..Default::default() } } From c08e14bfd0ebf6b0f29cead92b8fd5bc84983574 Mon Sep 17 00:00:00 2001 From: axlitEels Date: Sun, 16 Feb 2025 01:57:11 +0300 Subject: [PATCH 04/15] Docs backticks :< --- crates/bevy_gltf/src/loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index b9db59a344d2d..d5f52930538f9 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -171,7 +171,7 @@ pub struct GltfLoaderSettings { /// If some, the loader will ignore the gltf sampler data and generate materials with provided ImageSamplerDescriptor. pub override_sampler: Option, /// Anisotropic filtering level. Must be a power 1, 2, 4, 8 or 16. - /// Is passed into ImageSamplerDescriptor when parsing gltf sampler data. + /// Is passed into `ImageSamplerDescriptor` when parsing gltf sampler data. pub anisotropy_clamp: u16, } From 3400dd1dfe065201bd965835e1edc8499afee40b Mon Sep 17 00:00:00 2001 From: axlitEels Date: Sun, 16 Feb 2025 02:02:02 +0300 Subject: [PATCH 05/15] Ticks again :< I can't believe I fell for that twice --- crates/bevy_gltf/src/loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index d5f52930538f9..7e4abad4b46a5 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -168,7 +168,7 @@ pub struct GltfLoaderSettings { pub load_lights: bool, /// If true, the loader will include the root of the gltf root node. pub include_source: bool, - /// If some, the loader will ignore the gltf sampler data and generate materials with provided ImageSamplerDescriptor. + /// If some, the loader will ignore the gltf sampler data and generate materials with provided `ImageSamplerDescriptor`. pub override_sampler: Option, /// Anisotropic filtering level. Must be a power 1, 2, 4, 8 or 16. /// Is passed into `ImageSamplerDescriptor` when parsing gltf sampler data. From cefffe92c7fdbceb52298da3f3761f0a734451aa Mon Sep 17 00:00:00 2001 From: axlitEels Date: Sun, 16 Feb 2025 02:24:17 +0300 Subject: [PATCH 06/15] Fix typo in commentary --- crates/bevy_gltf/src/loader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 7e4abad4b46a5..867b3f4bca8cc 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -170,7 +170,7 @@ pub struct GltfLoaderSettings { pub include_source: bool, /// If some, the loader will ignore the gltf sampler data and generate materials with provided `ImageSamplerDescriptor`. pub override_sampler: Option, - /// Anisotropic filtering level. Must be a power 1, 2, 4, 8 or 16. + /// Anisotropic filtering level. Allowed values: 1, 2, 4, 8 or 16. /// Is passed into `ImageSamplerDescriptor` when parsing gltf sampler data. pub anisotropy_clamp: u16, } From bc4efb1da2f3c37eeac4ad3f32e66206b89d24c6 Mon Sep 17 00:00:00 2001 From: axlitEels Date: Fri, 7 Mar 2025 04:29:27 +0300 Subject: [PATCH 07/15] Mutable default image sampler for GltfLoader --- crates/bevy_gltf/src/lib.rs | 14 +++- .../bevy_gltf/src/loader/gltf_ext/texture.rs | 76 +++++++++---------- crates/bevy_gltf/src/loader/mod.rs | 31 ++++++-- crates/bevy_render/src/texture/mod.rs | 38 ++++++++++ 4 files changed, 112 insertions(+), 47 deletions(-) diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 159cdf4c67b5b..5377e426051a7 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -97,12 +97,16 @@ mod vertex_attributes; extern crate alloc; +use std::sync::{Arc, Mutex}; + use bevy_platform_support::collections::HashMap; use bevy_app::prelude::*; use bevy_asset::AssetApp; -use bevy_image::CompressedImageFormats; -use bevy_render::{mesh::MeshVertexAttribute, renderer::RenderDevice}; +use bevy_image::{CompressedImageFormats, ImageSamplerDescriptor}; +use bevy_render::{ + mesh::MeshVertexAttribute, renderer::RenderDevice, texture::DefaultImageSamplerDescriptor, +}; /// The glTF prelude. /// @@ -156,9 +160,15 @@ impl Plugin for GltfPlugin { Some(render_device) => CompressedImageFormats::from_features(render_device.features()), None => CompressedImageFormats::NONE, }; + let default_sampler = match app.world().get_resource::() { + Some(resource) => resource.get_mutex(), + // Probably should drop a WARN here. + None => Arc::new(Mutex::new(ImageSamplerDescriptor::default())), + }; app.register_asset_loader(GltfLoader { supported_compressed_formats, custom_vertex_attributes: self.custom_vertex_attributes.clone(), + default_sampler, }); } } diff --git a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs index 5fb5bcce0d4c0..e18001f0423c7 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs @@ -39,48 +39,46 @@ pub(crate) fn texture_handle( } /// Extracts the texture sampler data from the glTF [`Texture`]. -pub(crate) fn texture_sampler(texture: &Texture<'_>) -> ImageSamplerDescriptor { +pub(crate) fn texture_sampler( + texture: &Texture<'_>, + default_sampler: &ImageSamplerDescriptor +) -> ImageSamplerDescriptor { let gltf_sampler = texture.sampler(); + let mut sampler = default_sampler.clone(); - ImageSamplerDescriptor { - address_mode_u: address_mode(&gltf_sampler.wrap_s()), - address_mode_v: address_mode(&gltf_sampler.wrap_t()), - - mag_filter: gltf_sampler - .mag_filter() - .map(|mf| match mf { - MagFilter::Nearest => ImageFilterMode::Nearest, - MagFilter::Linear => ImageFilterMode::Linear, - }) - .unwrap_or(ImageSamplerDescriptor::default().mag_filter), - - min_filter: gltf_sampler - .min_filter() - .map(|mf| match mf { - MinFilter::Nearest - | MinFilter::NearestMipmapNearest - | MinFilter::NearestMipmapLinear => ImageFilterMode::Nearest, - MinFilter::Linear - | MinFilter::LinearMipmapNearest - | MinFilter::LinearMipmapLinear => ImageFilterMode::Linear, - }) - .unwrap_or(ImageSamplerDescriptor::default().min_filter), - - mipmap_filter: gltf_sampler - .min_filter() - .map(|mf| match mf { - MinFilter::Nearest - | MinFilter::Linear - | MinFilter::NearestMipmapNearest - | MinFilter::LinearMipmapNearest => ImageFilterMode::Nearest, - MinFilter::NearestMipmapLinear | MinFilter::LinearMipmapLinear => { - ImageFilterMode::Linear - } - }) - .unwrap_or(ImageSamplerDescriptor::default().mipmap_filter), - - ..Default::default() + sampler.address_mode_u = address_mode(&gltf_sampler.wrap_s()); + sampler.address_mode_v = address_mode(&gltf_sampler.wrap_t()); + + // Shouldn't parse filters when anisotropic filtering is on. + if sampler.anisotropy_clamp != 1 { + if let Some(mag_filter) = gltf_sampler.mag_filter().map(|mf| match mf { + MagFilter::Nearest => ImageFilterMode::Nearest, + MagFilter::Linear => ImageFilterMode::Linear, + }) { + sampler.mag_filter = mag_filter + } + if let Some(min_filter) = gltf_sampler.min_filter().map(|mf| match mf { + MinFilter::Nearest + | MinFilter::NearestMipmapNearest + | MinFilter::NearestMipmapLinear => ImageFilterMode::Nearest, + MinFilter::Linear + | MinFilter::LinearMipmapNearest + | MinFilter::LinearMipmapLinear => ImageFilterMode::Linear, + }) { + sampler.min_filter = min_filter + } + if let Some(mipmap_filter) = gltf_sampler.min_filter().map(|mf| match mf { + MinFilter::Nearest + | MinFilter::Linear + | MinFilter::NearestMipmapNearest + | MinFilter::LinearMipmapNearest => ImageFilterMode::Nearest, + MinFilter::NearestMipmapLinear + | MinFilter::LinearMipmapLinear => ImageFilterMode::Linear, + }) { + sampler.mipmap_filter = mipmap_filter + } } + sampler } pub(crate) fn texture_label(texture: &Texture<'_>) -> GltfAssetLabel { diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index 9d400e44bc0cb..c56abd902d498 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -2,6 +2,7 @@ mod extensions; mod gltf_ext; use std::{ + sync::{Arc, Mutex}, io::Error, path::{Path, PathBuf}, }; @@ -143,6 +144,8 @@ pub struct GltfLoader { /// See [this section of the glTF specification](https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#meshes-overview) /// for additional details on custom attributes. pub custom_vertex_attributes: HashMap, MeshVertexAttribute>, + /// Default `ImageSamplerDescriptor`. + pub default_sampler: Arc>, } /// Specifies optional settings for processing gltfs at load time. By default, all recognized contents of @@ -178,6 +181,12 @@ pub struct GltfLoaderSettings { pub load_lights: bool, /// If true, the loader will include the root of the gltf root node. pub include_source: bool, + /// Overrides the default sampler. Data from sampler node is added on top of that. + /// + /// If None, uses global default which is stored in `DefaultImageSamplerDescriptor` resource. + pub default_sampler: Option, + /// If true, the loader will ignore sampler data from gltf and use the default sampler. + pub override_sampler: bool, } impl Default for GltfLoaderSettings { @@ -188,6 +197,8 @@ impl Default for GltfLoaderSettings { load_cameras: true, load_lights: true, include_source: false, + default_sampler: None, + override_sampler: false, } } } @@ -505,6 +516,10 @@ async fn load_gltf<'a, 'b, 'c>( (animations, named_animations, animation_roots) }; + let default_sampler = match settings.default_sampler.as_ref() { + Some(sampler) => sampler, + None => &loader.default_sampler.lock().unwrap().clone() + }; // We collect handles to ensure loaded images from paths are not unloaded before they are used elsewhere // in the loader. This prevents "reloads", but it also prevents dropping the is_srgb context on reload. // @@ -521,7 +536,8 @@ async fn load_gltf<'a, 'b, 'c>( &linear_textures, parent_path, loader.supported_compressed_formats, - settings.load_materials, + default_sampler, + settings, ) .await?; image.process_loaded_texture(load_context, &mut _texture_handles); @@ -541,7 +557,8 @@ async fn load_gltf<'a, 'b, 'c>( linear_textures, parent_path, loader.supported_compressed_formats, - settings.load_materials, + default_sampler, + settings, ) .await }); @@ -968,10 +985,12 @@ async fn load_image<'a, 'b>( linear_textures: &HashSet, parent_path: &'b Path, supported_compressed_formats: CompressedImageFormats, - render_asset_usages: RenderAssetUsages, + default_sampler: &ImageSamplerDescriptor, + settings: &GltfLoaderSettings, ) -> Result { let is_srgb = !linear_textures.contains(&gltf_texture.index()); - let sampler_descriptor = texture_sampler(&gltf_texture); + let sampler_descriptor = if settings.override_sampler {default_sampler.clone()} + else {texture_sampler(&gltf_texture, default_sampler)}; #[cfg(all(debug_assertions, feature = "dds"))] let name = gltf_texture .name() @@ -989,7 +1008,7 @@ async fn load_image<'a, 'b>( supported_compressed_formats, is_srgb, ImageSampler::Descriptor(sampler_descriptor), - render_asset_usages, + settings.load_materials, )?; Ok(ImageOrPath::Image { image, @@ -1013,7 +1032,7 @@ async fn load_image<'a, 'b>( supported_compressed_formats, is_srgb, ImageSampler::Descriptor(sampler_descriptor), - render_asset_usages, + settings.load_materials, )?, label: GltfAssetLabel::Texture(gltf_texture.index()), }) diff --git a/crates/bevy_render/src/texture/mod.rs b/crates/bevy_render/src/texture/mod.rs index 6955de7ff4fc2..0b766d547e32f 100644 --- a/crates/bevy_render/src/texture/mod.rs +++ b/crates/bevy_render/src/texture/mod.rs @@ -3,6 +3,8 @@ mod gpu_image; mod texture_attachment; mod texture_cache; +use std::sync::{Arc, Mutex}; + pub use crate::render_resource::DefaultImageSampler; #[cfg(feature = "basis-universal")] use bevy_image::CompressedImageSaver; @@ -29,10 +31,44 @@ use bevy_ecs::prelude::*; pub const TRANSPARENT_IMAGE_HANDLE: Handle = weak_handle!("d18ad97e-a322-4981-9505-44c59a4b5e46"); +#[derive(Resource)] +pub struct DefaultImageSamplerDescriptor(Arc>); + +/// Stores default [`ImageSamplerDescriptor`] in main world. +/// Default `ImageSampler` is syncronized during `ExtractSchedule`. +impl DefaultImageSamplerDescriptor { + pub fn new(descriptor: &ImageSamplerDescriptor) -> DefaultImageSamplerDescriptor { + DefaultImageSamplerDescriptor(Arc::new(Mutex::new(descriptor.clone()))) + } + + /// Returns the current default [`ImageSamplerDescriptor`]. + pub fn get(&self) -> ImageSamplerDescriptor { + self.0.lock().unwrap().clone() + } + + /// Makes a clone of internal `Arc` pointer. + /// + /// Intended to be only used by code with no access to ECS. + pub fn get_mutex(&self) -> Arc> { + self.0.clone() + } + + /// Replaces default image sampler descriptor. + /// + /// Default image sampler is replaced during `ExtractSchedule`. + /// Doesn't apply to samplers already built on top of it, e.g. `GltfLoader`'s output. + pub fn set(&self, descriptor: &ImageSamplerDescriptor) { + *self.0.lock().unwrap() = descriptor.clone(); + } +} + // TODO: replace Texture names with Image names? /// Adds the [`Image`] as an asset and makes sure that they are extracted and prepared for the GPU. pub struct ImagePlugin { + // TODO: allow modifying default image sampler at runtime. /// The default image sampler to use when [`bevy_image::ImageSampler`] is set to `Default`. + /// + /// A copy of this descriptor is stored in `DefaultImageSamplerDescriptor`. pub default_sampler: ImageSamplerDescriptor, } @@ -120,6 +156,8 @@ impl Plugin for ImagePlugin { app.register_asset_loader(ImageLoader::new(supported_compressed_formats)); } + app.insert_resource(DefaultImageSamplerDescriptor::new(&self.default_sampler)); + if let Some(render_app) = app.get_sub_app_mut(RenderApp) { let default_sampler = { let device = render_app.world().resource::(); From 239fa89618498d653f9db18e5da792ca0361cf2e Mon Sep 17 00:00:00 2001 From: axlitEels Date: Tue, 11 Mar 2025 11:27:48 +0300 Subject: [PATCH 08/15] Move default descriptor resource to bevy_gltf --- crates/bevy_gltf/src/lib.rs | 49 ++++++++++++++++++++++----- crates/bevy_gltf/src/loader/mod.rs | 4 +-- crates/bevy_render/src/texture/mod.rs | 38 --------------------- 3 files changed, 43 insertions(+), 48 deletions(-) diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 5377e426051a7..2e8c27ecd159f 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -104,9 +104,8 @@ use bevy_platform_support::collections::HashMap; use bevy_app::prelude::*; use bevy_asset::AssetApp; use bevy_image::{CompressedImageFormats, ImageSamplerDescriptor}; -use bevy_render::{ - mesh::MeshVertexAttribute, renderer::RenderDevice, texture::DefaultImageSamplerDescriptor, -}; +use bevy_render::{mesh::MeshVertexAttribute, renderer::RenderDevice}; +use bevy_ecs::prelude::Resource; /// The glTF prelude. /// @@ -118,9 +117,45 @@ pub mod prelude { pub use {assets::*, label::GltfAssetLabel, loader::*}; +// Has to store an Arc as there is no other way to mutate fields of asset loaders. +/// Stores default [`ImageSamplerDescriptor`] in main world. +#[derive(Resource)] +pub struct DefaultGltfImageSampler(Arc>); + +impl DefaultGltfImageSampler { + /// Creates a new [`DefaultGltfImageSampler`]. + pub fn new(descriptor: &ImageSamplerDescriptor) -> Self { + Self(Arc::new(Mutex::new(descriptor.clone()))) + } + + /// Returns the current default [`ImageSamplerDescriptor`]. + pub fn get(&self) -> ImageSamplerDescriptor { + self.0.lock().unwrap().clone() + } + + /// Makes a clone of internal [`Arc`] pointer. + /// + /// Intended only to be used by code with no access to ECS. + pub fn get_mutex(&self) -> Arc> { + self.0.clone() + } + + /// Replaces default [`ImageSamplerDescriptor`]. + /// + /// Doesn't apply to samplers already built on top of it, i.e. `GltfLoader`'s output. + /// Assets need to manually be reloaded. + pub fn set(&self, descriptor: &ImageSamplerDescriptor) { + *self.0.lock().unwrap() = descriptor.clone(); + } +} + /// Adds support for glTF file loading to the app. #[derive(Default)] pub struct GltfPlugin { + /// The default image sampler to lay glTF sampler data on top of. + /// + /// Can be modified with [`DefaultGltfImageSampler`] resource. + pub default_sampler: ImageSamplerDescriptor, custom_vertex_attributes: HashMap, MeshVertexAttribute>, } @@ -160,11 +195,9 @@ impl Plugin for GltfPlugin { Some(render_device) => CompressedImageFormats::from_features(render_device.features()), None => CompressedImageFormats::NONE, }; - let default_sampler = match app.world().get_resource::() { - Some(resource) => resource.get_mutex(), - // Probably should drop a WARN here. - None => Arc::new(Mutex::new(ImageSamplerDescriptor::default())), - }; + let default_sampler_resource = DefaultGltfImageSampler::new(&self.default_sampler); + let default_sampler = default_sampler_resource.get_mutex(); + app.insert_resource(default_sampler_resource); app.register_asset_loader(GltfLoader { supported_compressed_formats, custom_vertex_attributes: self.custom_vertex_attributes.clone(), diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index c56abd902d498..5dd04205331ef 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -144,7 +144,7 @@ pub struct GltfLoader { /// See [this section of the glTF specification](https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#meshes-overview) /// for additional details on custom attributes. pub custom_vertex_attributes: HashMap, MeshVertexAttribute>, - /// Default `ImageSamplerDescriptor`. + /// Arc to default [`ImageSamplerDescriptor`]. pub default_sampler: Arc>, } @@ -183,7 +183,7 @@ pub struct GltfLoaderSettings { pub include_source: bool, /// Overrides the default sampler. Data from sampler node is added on top of that. /// - /// If None, uses global default which is stored in `DefaultImageSamplerDescriptor` resource. + /// If None, uses global default which is stored in `DefaultGltfImageSampler` resource. pub default_sampler: Option, /// If true, the loader will ignore sampler data from gltf and use the default sampler. pub override_sampler: bool, diff --git a/crates/bevy_render/src/texture/mod.rs b/crates/bevy_render/src/texture/mod.rs index 0b766d547e32f..6955de7ff4fc2 100644 --- a/crates/bevy_render/src/texture/mod.rs +++ b/crates/bevy_render/src/texture/mod.rs @@ -3,8 +3,6 @@ mod gpu_image; mod texture_attachment; mod texture_cache; -use std::sync::{Arc, Mutex}; - pub use crate::render_resource::DefaultImageSampler; #[cfg(feature = "basis-universal")] use bevy_image::CompressedImageSaver; @@ -31,44 +29,10 @@ use bevy_ecs::prelude::*; pub const TRANSPARENT_IMAGE_HANDLE: Handle = weak_handle!("d18ad97e-a322-4981-9505-44c59a4b5e46"); -#[derive(Resource)] -pub struct DefaultImageSamplerDescriptor(Arc>); - -/// Stores default [`ImageSamplerDescriptor`] in main world. -/// Default `ImageSampler` is syncronized during `ExtractSchedule`. -impl DefaultImageSamplerDescriptor { - pub fn new(descriptor: &ImageSamplerDescriptor) -> DefaultImageSamplerDescriptor { - DefaultImageSamplerDescriptor(Arc::new(Mutex::new(descriptor.clone()))) - } - - /// Returns the current default [`ImageSamplerDescriptor`]. - pub fn get(&self) -> ImageSamplerDescriptor { - self.0.lock().unwrap().clone() - } - - /// Makes a clone of internal `Arc` pointer. - /// - /// Intended to be only used by code with no access to ECS. - pub fn get_mutex(&self) -> Arc> { - self.0.clone() - } - - /// Replaces default image sampler descriptor. - /// - /// Default image sampler is replaced during `ExtractSchedule`. - /// Doesn't apply to samplers already built on top of it, e.g. `GltfLoader`'s output. - pub fn set(&self, descriptor: &ImageSamplerDescriptor) { - *self.0.lock().unwrap() = descriptor.clone(); - } -} - // TODO: replace Texture names with Image names? /// Adds the [`Image`] as an asset and makes sure that they are extracted and prepared for the GPU. pub struct ImagePlugin { - // TODO: allow modifying default image sampler at runtime. /// The default image sampler to use when [`bevy_image::ImageSampler`] is set to `Default`. - /// - /// A copy of this descriptor is stored in `DefaultImageSamplerDescriptor`. pub default_sampler: ImageSamplerDescriptor, } @@ -156,8 +120,6 @@ impl Plugin for ImagePlugin { app.register_asset_loader(ImageLoader::new(supported_compressed_formats)); } - app.insert_resource(DefaultImageSamplerDescriptor::new(&self.default_sampler)); - if let Some(render_app) = app.get_sub_app_mut(RenderApp) { let default_sampler = { let device = render_app.world().resource::(); From adac31e5be085a4087834f757d86603ae4b6c538 Mon Sep 17 00:00:00 2001 From: axlitEels Date: Tue, 11 Mar 2025 12:20:29 +0300 Subject: [PATCH 09/15] cargo fmt --- crates/bevy_gltf/src/lib.rs | 8 ++++---- crates/bevy_gltf/src/loader/gltf_ext/texture.rs | 15 ++++++++------- crates/bevy_gltf/src/loader/mod.rs | 11 +++++++---- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 2e8c27ecd159f..0c82fd6decfe1 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -103,9 +103,9 @@ use bevy_platform_support::collections::HashMap; use bevy_app::prelude::*; use bevy_asset::AssetApp; +use bevy_ecs::prelude::Resource; use bevy_image::{CompressedImageFormats, ImageSamplerDescriptor}; use bevy_render::{mesh::MeshVertexAttribute, renderer::RenderDevice}; -use bevy_ecs::prelude::Resource; /// The glTF prelude. /// @@ -134,14 +134,14 @@ impl DefaultGltfImageSampler { } /// Makes a clone of internal [`Arc`] pointer. - /// + /// /// Intended only to be used by code with no access to ECS. pub fn get_mutex(&self) -> Arc> { self.0.clone() } /// Replaces default [`ImageSamplerDescriptor`]. - /// + /// /// Doesn't apply to samplers already built on top of it, i.e. `GltfLoader`'s output. /// Assets need to manually be reloaded. pub fn set(&self, descriptor: &ImageSamplerDescriptor) { @@ -153,7 +153,7 @@ impl DefaultGltfImageSampler { #[derive(Default)] pub struct GltfPlugin { /// The default image sampler to lay glTF sampler data on top of. - /// + /// /// Can be modified with [`DefaultGltfImageSampler`] resource. pub default_sampler: ImageSamplerDescriptor, custom_vertex_attributes: HashMap, MeshVertexAttribute>, diff --git a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs index e18001f0423c7..458a0c449311e 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs @@ -41,14 +41,14 @@ pub(crate) fn texture_handle( /// Extracts the texture sampler data from the glTF [`Texture`]. pub(crate) fn texture_sampler( texture: &Texture<'_>, - default_sampler: &ImageSamplerDescriptor + default_sampler: &ImageSamplerDescriptor, ) -> ImageSamplerDescriptor { let gltf_sampler = texture.sampler(); let mut sampler = default_sampler.clone(); sampler.address_mode_u = address_mode(&gltf_sampler.wrap_s()); sampler.address_mode_v = address_mode(&gltf_sampler.wrap_t()); - + // Shouldn't parse filters when anisotropic filtering is on. if sampler.anisotropy_clamp != 1 { if let Some(mag_filter) = gltf_sampler.mag_filter().map(|mf| match mf { @@ -61,9 +61,9 @@ pub(crate) fn texture_sampler( MinFilter::Nearest | MinFilter::NearestMipmapNearest | MinFilter::NearestMipmapLinear => ImageFilterMode::Nearest, - MinFilter::Linear - | MinFilter::LinearMipmapNearest - | MinFilter::LinearMipmapLinear => ImageFilterMode::Linear, + MinFilter::Linear | MinFilter::LinearMipmapNearest | MinFilter::LinearMipmapLinear => { + ImageFilterMode::Linear + } }) { sampler.min_filter = min_filter } @@ -72,8 +72,9 @@ pub(crate) fn texture_sampler( | MinFilter::Linear | MinFilter::NearestMipmapNearest | MinFilter::LinearMipmapNearest => ImageFilterMode::Nearest, - MinFilter::NearestMipmapLinear - | MinFilter::LinearMipmapLinear => ImageFilterMode::Linear, + MinFilter::NearestMipmapLinear | MinFilter::LinearMipmapLinear => { + ImageFilterMode::Linear + } }) { sampler.mipmap_filter = mipmap_filter } diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index 5dd04205331ef..7798b09ba0a88 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -2,9 +2,9 @@ mod extensions; mod gltf_ext; use std::{ - sync::{Arc, Mutex}, io::Error, path::{Path, PathBuf}, + sync::{Arc, Mutex}, }; #[cfg(feature = "bevy_animation")] @@ -518,7 +518,7 @@ async fn load_gltf<'a, 'b, 'c>( let default_sampler = match settings.default_sampler.as_ref() { Some(sampler) => sampler, - None => &loader.default_sampler.lock().unwrap().clone() + None => &loader.default_sampler.lock().unwrap().clone(), }; // We collect handles to ensure loaded images from paths are not unloaded before they are used elsewhere // in the loader. This prevents "reloads", but it also prevents dropping the is_srgb context on reload. @@ -989,8 +989,11 @@ async fn load_image<'a, 'b>( settings: &GltfLoaderSettings, ) -> Result { let is_srgb = !linear_textures.contains(&gltf_texture.index()); - let sampler_descriptor = if settings.override_sampler {default_sampler.clone()} - else {texture_sampler(&gltf_texture, default_sampler)}; + let sampler_descriptor = if settings.override_sampler { + default_sampler.clone() + } else { + texture_sampler(&gltf_texture, default_sampler) + }; #[cfg(all(debug_assertions, feature = "dds"))] let name = gltf_texture .name() From 06852c3462b1ef21d66029873de8d3815321877a Mon Sep 17 00:00:00 2001 From: axlitEels Date: Tue, 11 Mar 2025 13:16:07 +0300 Subject: [PATCH 10/15] GltfPlugin struct consistency. `ImagePlugin` uses `ImageSamplerDescriptor::linear()` instead of `ImageSamplerDescriptor::default()`, so I had to manually implement `Default` here too. As for making `custom_vertex_attributes` public, it is a necessary evil unless we make an additional setter method to specify default sampler and that would be inconsistent with `ImagePlugin`. --- crates/bevy_gltf/src/lib.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 0c82fd6decfe1..5e6aff4c0917c 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -150,13 +150,22 @@ impl DefaultGltfImageSampler { } /// Adds support for glTF file loading to the app. -#[derive(Default)] pub struct GltfPlugin { /// The default image sampler to lay glTF sampler data on top of. /// /// Can be modified with [`DefaultGltfImageSampler`] resource. pub default_sampler: ImageSamplerDescriptor, - custom_vertex_attributes: HashMap, MeshVertexAttribute>, + /// Registry for custom vertex attributes. + /// + /// To specify, use [`GltfPlugin::add_custom_vertex_attribute`]. + pub custom_vertex_attributes: HashMap, MeshVertexAttribute>, +} + +impl Default for GltfPlugin { + fn default() -> Self { GltfPlugin{ + default_sampler: ImageSamplerDescriptor::linear(), + custom_vertex_attributes: HashMap::default() + }} } impl GltfPlugin { From f0eac14454df4d5218431063e845aa17315aefb5 Mon Sep 17 00:00:00 2001 From: axlitEels Date: Tue, 11 Mar 2025 13:18:31 +0300 Subject: [PATCH 11/15] cargo fmt --- crates/bevy_gltf/src/lib.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 5e6aff4c0917c..83cc77b9450cf 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -156,16 +156,18 @@ pub struct GltfPlugin { /// Can be modified with [`DefaultGltfImageSampler`] resource. pub default_sampler: ImageSamplerDescriptor, /// Registry for custom vertex attributes. - /// + /// /// To specify, use [`GltfPlugin::add_custom_vertex_attribute`]. pub custom_vertex_attributes: HashMap, MeshVertexAttribute>, } impl Default for GltfPlugin { - fn default() -> Self { GltfPlugin{ - default_sampler: ImageSamplerDescriptor::linear(), - custom_vertex_attributes: HashMap::default() - }} + fn default() -> Self { + GltfPlugin { + default_sampler: ImageSamplerDescriptor::linear(), + custom_vertex_attributes: HashMap::default(), + } + } } impl GltfPlugin { From 2fdfb2d0131a4b5964534ebf513c02c63758d57f Mon Sep 17 00:00:00 2001 From: axlitEels Date: Tue, 11 Mar 2025 13:27:31 +0300 Subject: [PATCH 12/15] Clippy --- crates/bevy_gltf/src/lib.rs | 2 +- crates/bevy_gltf/src/loader/gltf_ext/texture.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 83cc77b9450cf..03da7c6d2bc7d 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -97,7 +97,7 @@ mod vertex_attributes; extern crate alloc; -use std::sync::{Arc, Mutex}; +use alloc::sync::{Arc, Mutex}; use bevy_platform_support::collections::HashMap; diff --git a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs index 458a0c449311e..2b3cb3434a25f 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs @@ -55,7 +55,7 @@ pub(crate) fn texture_sampler( MagFilter::Nearest => ImageFilterMode::Nearest, MagFilter::Linear => ImageFilterMode::Linear, }) { - sampler.mag_filter = mag_filter + sampler.mag_filter = mag_filter; } if let Some(min_filter) = gltf_sampler.min_filter().map(|mf| match mf { MinFilter::Nearest @@ -65,7 +65,7 @@ pub(crate) fn texture_sampler( ImageFilterMode::Linear } }) { - sampler.min_filter = min_filter + sampler.min_filter = min_filter; } if let Some(mipmap_filter) = gltf_sampler.min_filter().map(|mf| match mf { MinFilter::Nearest @@ -76,7 +76,7 @@ pub(crate) fn texture_sampler( ImageFilterMode::Linear } }) { - sampler.mipmap_filter = mipmap_filter + sampler.mipmap_filter = mipmap_filter; } } sampler From 2b4cbc9a96cc3b1677b415d7aeed6093bf53bda7 Mon Sep 17 00:00:00 2001 From: axlitEels Date: Tue, 11 Mar 2025 13:35:00 +0300 Subject: [PATCH 13/15] Fix incorrect clippy fix CI hates me --- crates/bevy_gltf/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 03da7c6d2bc7d..b0d7c2beb16f7 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -97,7 +97,8 @@ mod vertex_attributes; extern crate alloc; -use alloc::sync::{Arc, Mutex}; +use alloc::sync::Arc; +use std::sync::Mutex; use bevy_platform_support::collections::HashMap; From 8d98eb8816008c428307e5d0f5d29d6cce82af19 Mon Sep 17 00:00:00 2001 From: axlitEels Date: Wed, 12 Mar 2025 07:02:05 +0300 Subject: [PATCH 14/15] Fair suggestions by @thepackett --- crates/bevy_gltf/src/lib.rs | 6 +++--- crates/bevy_gltf/src/loader/gltf_ext/texture.rs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index b0d7c2beb16f7..baf93da6734f7 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -118,7 +118,7 @@ pub mod prelude { pub use {assets::*, label::GltfAssetLabel, loader::*}; -// Has to store an Arc as there is no other way to mutate fields of asset loaders. +// Has to store an Arc> as there is no other way to mutate fields of asset loaders. /// Stores default [`ImageSamplerDescriptor`] in main world. #[derive(Resource)] pub struct DefaultGltfImageSampler(Arc>); @@ -137,7 +137,7 @@ impl DefaultGltfImageSampler { /// Makes a clone of internal [`Arc`] pointer. /// /// Intended only to be used by code with no access to ECS. - pub fn get_mutex(&self) -> Arc> { + pub fn get_internal(&self) -> Arc> { self.0.clone() } @@ -208,7 +208,7 @@ impl Plugin for GltfPlugin { None => CompressedImageFormats::NONE, }; let default_sampler_resource = DefaultGltfImageSampler::new(&self.default_sampler); - let default_sampler = default_sampler_resource.get_mutex(); + let default_sampler = default_sampler_resource.get_internal(); app.insert_resource(default_sampler_resource); app.register_asset_loader(GltfLoader { supported_compressed_formats, diff --git a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs index 2b3cb3434a25f..f666752479bb6 100644 --- a/crates/bevy_gltf/src/loader/gltf_ext/texture.rs +++ b/crates/bevy_gltf/src/loader/gltf_ext/texture.rs @@ -49,7 +49,8 @@ pub(crate) fn texture_sampler( sampler.address_mode_u = address_mode(&gltf_sampler.wrap_s()); sampler.address_mode_v = address_mode(&gltf_sampler.wrap_t()); - // Shouldn't parse filters when anisotropic filtering is on. + // Shouldn't parse filters when anisotropic filtering is on, because trilinear is then required by wgpu. + // We also trust user to have provided a valid sampler. if sampler.anisotropy_clamp != 1 { if let Some(mag_filter) = gltf_sampler.mag_filter().map(|mf| match mf { MagFilter::Nearest => ImageFilterMode::Nearest, From c488723cdd2ef2cbc0968711bfdbe0da83ec8754 Mon Sep 17 00:00:00 2001 From: axlitEels <152332540+axlitEels@users.noreply.github.com> Date: Tue, 18 Mar 2025 16:16:19 +0300 Subject: [PATCH 15/15] Use alloc instead of std I don't understand how I didn't get this warn either Co-authored-by: Greeble <166992735+greeble-dev@users.noreply.github.com> --- crates/bevy_gltf/src/loader/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_gltf/src/loader/mod.rs b/crates/bevy_gltf/src/loader/mod.rs index 7798b09ba0a88..5cd506044e4aa 100644 --- a/crates/bevy_gltf/src/loader/mod.rs +++ b/crates/bevy_gltf/src/loader/mod.rs @@ -1,10 +1,11 @@ mod extensions; mod gltf_ext; +use alloc::sync::Arc; use std::{ io::Error, path::{Path, PathBuf}, - sync::{Arc, Mutex}, + sync::Mutex, }; #[cfg(feature = "bevy_animation")]