Image Sampler Improvements (bevyengine#10254)

# Objective

- Build on the changes in bevyengine#9982
- Use `ImageSamplerDescriptor` as the "public image sampler descriptor"
interface in all places (for consistency)
- Make it possible to configure textures to use the "default" sampler
(as configured in the `DefaultImageSampler` resource)
- Fix a bug introduced in bevyengine#9982 that prevents configured samplers from
being used in Basis, KTX2, and DDS textures

---

## Migration Guide

- When using the `Image` API, use `ImageSamplerDescriptor` instead of
`wgpu::SamplerDescriptor`
- If writing custom wgpu renderer features that work with `Image`, call
`&image_sampler.as_wgpu()` to convert to a wgpu descriptor.

crates/bevy_core_pipeline/src/tonemapping/mod.rs

@@ -346,16 +346,18 @@ pub fn get_lut_bind_group_layout_entries(bindings: [u32; 2]) -> [BindGroupLayout
 // allow(dead_code) so it doesn't complain when the tonemapping_luts feature is disabled
 #[allow(dead_code)]
 fn setup_tonemapping_lut_image(bytes: &[u8], image_type: ImageType) -> Image {
-    let image_sampler = bevy_render::texture::ImageSampler::Descriptor(SamplerDescriptor {
-        label: Some("Tonemapping LUT sampler"),
-        address_mode_u: AddressMode::ClampToEdge,
-        address_mode_v: AddressMode::ClampToEdge,
-        address_mode_w: AddressMode::ClampToEdge,
-        mag_filter: FilterMode::Linear,
-        min_filter: FilterMode::Linear,
-        mipmap_filter: FilterMode::Linear,
-        ..default()
-    });
+    let image_sampler = bevy_render::texture::ImageSampler::Descriptor(
+        bevy_render::texture::ImageSamplerDescriptor {
+            label: Some("Tonemapping LUT sampler".to_string()),
+            address_mode_u: bevy_render::texture::ImageAddressMode::ClampToEdge,
+            address_mode_v: bevy_render::texture::ImageAddressMode::ClampToEdge,
+            address_mode_w: bevy_render::texture::ImageAddressMode::ClampToEdge,
+            mag_filter: bevy_render::texture::ImageFilterMode::Linear,
+            min_filter: bevy_render::texture::ImageFilterMode::Linear,
+            mipmap_filter: bevy_render::texture::ImageFilterMode::Linear,
+            ..default()
+        },
+    );
     Image::from_buffer(
         bytes,
         image_type,
@@ -385,7 +387,7 @@ pub fn lut_placeholder() -> Image {
             usage: TextureUsages::TEXTURE_BINDING | TextureUsages::COPY_DST,
             view_formats: &[],
         },
-        sampler_descriptor: ImageSampler::Default,
+        sampler: ImageSampler::Default,
         texture_view_descriptor: None,
     }
 }

crates/bevy_gltf/src/loader.rs

@@ -264,7 +264,7 @@ async fn load_gltf<'a, 'b, 'c>(
             } => {
                 load_context.load_with_settings(path, move |settings: &mut ImageLoaderSettings| {
                     settings.is_srgb = is_srgb;
-                    settings.sampler_descriptor = sampler_descriptor;
+                    settings.sampler = ImageSampler::Descriptor(sampler_descriptor.clone());
                 })
             }
         };
@@ -684,7 +684,7 @@ async fn load_image<'a, 'b>(
                 ImageType::MimeType(mime_type),
                 supported_compressed_formats,
                 is_srgb,
-                ImageSampler::Descriptor(sampler_descriptor.into()),
+                ImageSampler::Descriptor(sampler_descriptor),
             )?;
             Ok(ImageOrPath::Image {
                 image,
@@ -705,7 +705,7 @@ async fn load_image<'a, 'b>(
                         mime_type.map(ImageType::MimeType).unwrap_or(image_type),
                         supported_compressed_formats,
                         is_srgb,
-                        ImageSampler::Descriptor(sampler_descriptor.into()),
+                        ImageSampler::Descriptor(sampler_descriptor),
                     )?,
                     label: texture_label(&gltf_texture),
                 })

crates/bevy_pbr/src/pbr_material.rs

@@ -267,7 +267,7 @@ pub struct StandardMaterial {
     /// Use the [`parallax_mapping_method`] and [`max_parallax_layer_count`] fields
     /// to tweak the shader, trading graphical quality for performance.
     ///
-    /// To improve performance, set your `depth_map`'s [`Image::sampler_descriptor`]
+    /// To improve performance, set your `depth_map`'s [`Image::sampler`]
     /// filter mode to `FilterMode::Nearest`, as [this paper] indicates, it improves
     /// performance a bit.
     ///

crates/bevy_pbr/src/render/mesh.rs

@@ -361,10 +361,10 @@ impl FromWorld for MeshPipeline {
         let dummy_white_gpu_image = {
             let image = Image::default();
             let texture = render_device.create_texture(&image.texture_descriptor);
-            let sampler = match image.sampler_descriptor {
+            let sampler = match image.sampler {
                 ImageSampler::Default => (**default_sampler).clone(),
                 ImageSampler::Descriptor(ref descriptor) => {
-                    render_device.create_sampler(descriptor)
+                    render_device.create_sampler(&descriptor.as_wgpu())
                 }
             };
 

crates/bevy_render/src/texture/compressed_image_saver.rs

@@ -1,7 +1,4 @@
-use crate::texture::{
-    Image, ImageFormat, ImageFormatSetting, ImageLoader, ImageLoaderSettings, ImageSampler,
-    ImageSamplerDescriptor,
-};
+use crate::texture::{Image, ImageFormat, ImageFormatSetting, ImageLoader, ImageLoaderSettings};
 use bevy_asset::saver::{AssetSaver, SavedAsset};
 use futures_lite::{AsyncWriteExt, FutureExt};
 use thiserror::Error;
@@ -33,10 +30,6 @@ impl AssetSaver for CompressedImageSaver {
         compressor_params.set_basis_format(basis_universal::BasisTextureFormat::UASTC4x4);
         compressor_params.set_generate_mipmaps(true);
         let is_srgb = image.texture_descriptor.format.is_srgb();
-        let sampler_descriptor = match &image.sampler_descriptor {
-            ImageSampler::Default => ImageSamplerDescriptor::default(),
-            ImageSampler::Descriptor(sampler_descriptor) => sampler_descriptor.clone().into(),
-        };
         let color_space = if is_srgb {
             basis_universal::ColorSpace::Srgb
         } else {
@@ -62,7 +55,7 @@ impl AssetSaver for CompressedImageSaver {
             Ok(ImageLoaderSettings {
                 format: ImageFormatSetting::Format(ImageFormat::Basis),
                 is_srgb,
-                sampler_descriptor,
+                sampler: image.sampler.clone(),
             })
         }
         .boxed()

crates/bevy_render/src/texture/fallback_image.rs

@@ -100,9 +100,11 @@ fn fallback_image_new(
         array_layer_count: Some(extents.depth_or_array_layers),
         ..TextureViewDescriptor::default()
     });
-    let sampler = match image.sampler_descriptor {
+    let sampler = match image.sampler {
         ImageSampler::Default => (**default_sampler).clone(),
-        ImageSampler::Descriptor(ref descriptor) => render_device.create_sampler(descriptor),
+        ImageSampler::Descriptor(ref descriptor) => {
+            render_device.create_sampler(&descriptor.as_wgpu())
+        }
     };
     GpuImage {
         texture,

crates/bevy_render/src/texture/image.rs

@@ -108,55 +108,33 @@ pub struct Image {
     // TODO: this nesting makes accessing Image metadata verbose. Either flatten out descriptor or add accessors
     pub texture_descriptor: wgpu::TextureDescriptor<'static>,
     /// The [`ImageSampler`] to use during rendering.
-    pub sampler_descriptor: ImageSampler,
+    pub sampler: ImageSampler,
     pub texture_view_descriptor: Option<wgpu::TextureViewDescriptor<'static>>,
 }
 
 /// Used in [`Image`], this determines what image sampler to use when rendering. The default setting,
 /// [`ImageSampler::Default`], will read the sampler from the [`ImagePlugin`](super::ImagePlugin) at setup.
 /// Setting this to [`ImageSampler::Descriptor`] will override the global default descriptor for this [`Image`].
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Default, Clone, Serialize, Deserialize)]
 pub enum ImageSampler {
     /// Default image sampler, derived from the [`ImagePlugin`](super::ImagePlugin) setup.
     #[default]
     Default,
     /// Custom sampler for this image which will override global default.
-    Descriptor(wgpu::SamplerDescriptor<'static>),
+    Descriptor(ImageSamplerDescriptor),
 }
 
 impl ImageSampler {
-    /// Returns an image sampler with [`Linear`](crate::render_resource::FilterMode::Linear) min and mag filters
+    /// Returns an image sampler with [`ImageFilterMode::Linear`] min and mag filters
     #[inline]
     pub fn linear() -> ImageSampler {
-        ImageSampler::Descriptor(Self::linear_descriptor())
+        ImageSampler::Descriptor(ImageSamplerDescriptor::linear())
     }
 
-    /// Returns an image sampler with [`Nearest`](crate::render_resource::FilterMode::Nearest) min and mag filters
+    /// Returns an image sampler with [`ImageFilterMode::Nearest`] min and mag filters
     #[inline]
     pub fn nearest() -> ImageSampler {
-        ImageSampler::Descriptor(Self::nearest_descriptor())
-    }
-
-    /// Returns a sampler descriptor with [`Linear`](crate::render_resource::FilterMode::Linear) min and mag filters
-    #[inline]
-    pub fn linear_descriptor() -> wgpu::SamplerDescriptor<'static> {
-        wgpu::SamplerDescriptor {
-            mag_filter: wgpu::FilterMode::Linear,
-            min_filter: wgpu::FilterMode::Linear,
-            mipmap_filter: wgpu::FilterMode::Linear,
-            ..Default::default()
-        }
-    }
-
-    /// Returns a sampler descriptor with [`Nearest`](crate::render_resource::FilterMode::Nearest) min and mag filters
-    #[inline]
-    pub fn nearest_descriptor() -> wgpu::SamplerDescriptor<'static> {
-        wgpu::SamplerDescriptor {
-            mag_filter: wgpu::FilterMode::Nearest,
-            min_filter: wgpu::FilterMode::Nearest,
-            mipmap_filter: wgpu::FilterMode::Nearest,
-            ..Default::default()
-        }
+        ImageSampler::Descriptor(ImageSamplerDescriptor::nearest())
     }
 }
 
@@ -265,8 +243,9 @@ pub enum ImageSamplerBorderColor {
 /// a breaking change.
 ///
 /// This types mirrors [`wgpu::SamplerDescriptor`], but that might change in future versions.
-#[derive(Clone, Copy, Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct ImageSamplerDescriptor {
+    pub label: Option<String>,
     /// How to deal with out of bounds accesses in the u (i.e. x) direction.
     pub address_mode_u: ImageAddressMode,
     /// How to deal with out of bounds accesses in the v (i.e. y) direction.
@@ -305,6 +284,48 @@ impl Default for ImageSamplerDescriptor {
             compare: None,
             anisotropy_clamp: 1,
             border_color: None,
+            label: None,
+        }
+    }
+}
+
+impl ImageSamplerDescriptor {
+    /// Returns a sampler descriptor with [`Linear`](crate::render_resource::FilterMode::Linear) min and mag filters
+    #[inline]
+    pub fn linear() -> ImageSamplerDescriptor {
+        ImageSamplerDescriptor {
+            mag_filter: ImageFilterMode::Linear,
+            min_filter: ImageFilterMode::Linear,
+            mipmap_filter: ImageFilterMode::Linear,
+            ..Default::default()
+        }
+    }
+
+    /// Returns a sampler descriptor with [`Nearest`](crate::render_resource::FilterMode::Nearest) min and mag filters
+    #[inline]
+    pub fn nearest() -> ImageSamplerDescriptor {
+        ImageSamplerDescriptor {
+            mag_filter: ImageFilterMode::Nearest,
+            min_filter: ImageFilterMode::Nearest,
+            mipmap_filter: ImageFilterMode::Nearest,
+            ..Default::default()
+        }
+    }
+
+    pub fn as_wgpu(&self) -> wgpu::SamplerDescriptor {
+        wgpu::SamplerDescriptor {
+            label: self.label.as_deref(),
+            address_mode_u: self.address_mode_u.into(),
+            address_mode_v: self.address_mode_v.into(),
+            address_mode_w: self.address_mode_w.into(),
+            mag_filter: self.mag_filter.into(),
+            min_filter: self.min_filter.into(),
+            mipmap_filter: self.mipmap_filter.into(),
+            lod_min_clamp: self.lod_min_clamp,
+            lod_max_clamp: self.lod_max_clamp,
+            compare: self.compare.map(Into::into),
+            anisotropy_clamp: self.anisotropy_clamp,
+            border_color: self.border_color.map(Into::into),
         }
     }
 }
@@ -355,25 +376,6 @@ impl From<ImageSamplerBorderColor> for wgpu::SamplerBorderColor {
     }
 }
 
-impl From<ImageSamplerDescriptor> for wgpu::SamplerDescriptor<'static> {
-    fn from(value: ImageSamplerDescriptor) -> Self {
-        wgpu::SamplerDescriptor {
-            label: None,
-            address_mode_u: value.address_mode_u.into(),
-            address_mode_v: value.address_mode_v.into(),
-            address_mode_w: value.address_mode_w.into(),
-            mag_filter: value.mag_filter.into(),
-            min_filter: value.min_filter.into(),
-            mipmap_filter: value.mipmap_filter.into(),
-            lod_min_clamp: value.lod_min_clamp,
-            lod_max_clamp: value.lod_max_clamp,
-            compare: value.compare.map(Into::into),
-            anisotropy_clamp: value.anisotropy_clamp,
-            border_color: value.border_color.map(Into::into),
-        }
-    }
-}
-
 impl From<wgpu::AddressMode> for ImageAddressMode {
     fn from(value: wgpu::AddressMode) -> Self {
         match value {
@@ -423,6 +425,7 @@ impl From<wgpu::SamplerBorderColor> for ImageSamplerBorderColor {
 impl<'a> From<wgpu::SamplerDescriptor<'a>> for ImageSamplerDescriptor {
     fn from(value: wgpu::SamplerDescriptor) -> Self {
         ImageSamplerDescriptor {
+            label: value.label.map(|l| l.to_string()),
             address_mode_u: value.address_mode_u.into(),
             address_mode_v: value.address_mode_v.into(),
             address_mode_w: value.address_mode_w.into(),
@@ -459,7 +462,7 @@ impl Default for Image {
                 usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
                 view_formats: &[],
             },
-            sampler_descriptor: ImageSampler::Default,
+            sampler: ImageSampler::Default,
             texture_view_descriptor: None,
         }
     }
@@ -645,16 +648,16 @@ impl Image {
         // needs to be added, so the image data needs to be converted in those
         // cases.
 
-        match format {
+        let mut image = match format {
             #[cfg(feature = "basis-universal")]
             ImageFormat::Basis => {
-                basis_buffer_to_image(buffer, supported_compressed_formats, is_srgb)
+                basis_buffer_to_image(buffer, supported_compressed_formats, is_srgb)?
             }
             #[cfg(feature = "dds")]
-            ImageFormat::Dds => dds_buffer_to_image(buffer, supported_compressed_formats, is_srgb),
+            ImageFormat::Dds => dds_buffer_to_image(buffer, supported_compressed_formats, is_srgb)?,
             #[cfg(feature = "ktx2")]
             ImageFormat::Ktx2 => {
-                ktx2_buffer_to_image(buffer, supported_compressed_formats, is_srgb)
+                ktx2_buffer_to_image(buffer, supported_compressed_formats, is_srgb)?
             }
             _ => {
                 let image_crate_format = format
@@ -664,11 +667,11 @@ impl Image {
                 reader.set_format(image_crate_format);
                 reader.no_limits();
                 let dyn_img = reader.decode()?;
-                let mut img = Self::from_dynamic(dyn_img, is_srgb);
-                img.sampler_descriptor = image_sampler;
-                Ok(img)
+                Self::from_dynamic(dyn_img, is_srgb)
             }
-        }
+        };
+        image.sampler = image_sampler;
+        Ok(image)
     }
 
     /// Whether the texture format is compressed or uncompressed
@@ -832,9 +835,11 @@ impl RenderAsset for Image {
             image.texture_descriptor.size.width as f32,
             image.texture_descriptor.size.height as f32,
         );
-        let sampler = match image.sampler_descriptor {
+        let sampler = match image.sampler {
             ImageSampler::Default => (***default_sampler).clone(),
-            ImageSampler::Descriptor(descriptor) => render_device.create_sampler(&descriptor),
+            ImageSampler::Descriptor(descriptor) => {
+                render_device.create_sampler(&descriptor.as_wgpu())
+            }
         };
 
         Ok(GpuImage {

crates/bevy_render/src/texture/image_loader.rs

@@ -7,7 +7,7 @@ use crate::{
     texture::{Image, ImageFormat, ImageType, TextureError},
 };
 
-use super::{CompressedImageFormats, ImageSampler, ImageSamplerDescriptor};
+use super::{CompressedImageFormats, ImageSampler};
 use serde::{Deserialize, Serialize};
 
 /// Loader for images that can be read by the `image` crate.
@@ -45,26 +45,26 @@ pub(crate) const IMG_FILE_EXTENSIONS: &[&str] = &[
     "ppm",
 ];
 
-#[derive(Serialize, Deserialize, Default)]
+#[derive(Serialize, Deserialize, Default, Debug)]
 pub enum ImageFormatSetting {
     #[default]
     FromExtension,
     Format(ImageFormat),
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
 pub struct ImageLoaderSettings {
     pub format: ImageFormatSetting,
     pub is_srgb: bool,
-    pub sampler_descriptor: ImageSamplerDescriptor,
+    pub sampler: ImageSampler,
 }
 
 impl Default for ImageLoaderSettings {
     fn default() -> Self {
         Self {
             format: ImageFormatSetting::default(),
             is_srgb: true,
-            sampler_descriptor: ImageSamplerDescriptor::default(),
+            sampler: ImageSampler::Default,
         }
     }
 }
@@ -103,7 +103,7 @@ impl AssetLoader for ImageLoader {
                 image_type,
                 self.supported_compressed_formats,
                 settings.is_srgb,
-                ImageSampler::Descriptor(settings.sampler_descriptor.into()),
+                settings.sampler.clone(),
             )
             .map_err(|err| FileTextureError {
                 error: err,