diff --git a/CHANGELOG.md b/CHANGELOG.md index 28241eb7b5..8942f6d34e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,21 @@ The following `Features` have been renamed. By @teoxoy in [#3534](https://github.com/gfx-rs/wgpu/pull/3534) +#### Anisotropic Filtering + +Anisotropic filtering has been brought in line with the spec. The anisotropic clamp is now a f32 which must be between 1.0 and 16.0 inclusive. + +If the anisotropy clamp is not 1.0, all the filters in a sampler must be `Linear`. + +```diff +SamplerDescriptor { +- anisotropic_clamp: None, ++ anisotropic_clamp: 1.0, +} +``` + +By @cwfitzgerald in [#3610](https://github.com/gfx-rs/wgpu/pull/3610). + #### General - Change type of `mip_level_count` and `array_layer_count` (members of `TextureViewDescriptor` and `ImageSubresourceRange`) from `Option` to `Option`. By @teoxoy in [#3445](https://github.com/gfx-rs/wgpu/pull/3445) @@ -99,6 +114,7 @@ By @teoxoy in [#3534](https://github.com/gfx-rs/wgpu/pull/3534) - Improve attachment related errors. By @cwfitzgerald in [#3549](https://github.com/gfx-rs/wgpu/pull/3549) - Make error descriptions all upper case. By @cwfitzgerald in [#3549](https://github.com/gfx-rs/wgpu/pull/3549) - Don't include ANSI terminal color escape sequences in shader module validation error messages. By @jimblandy in [#3591](https://github.com/gfx-rs/wgpu/pull/3591) +- Bring anisotropic filtering in line with the spec. #### WebGPU @@ -115,7 +131,7 @@ By @teoxoy in [#3534](https://github.com/gfx-rs/wgpu/pull/3534) ### Bug Fixes #### Metal -- Fix incorrect mipmap being sampled when using `MinLod <= 0.0` and `MaxLod >= 32.0` or when the fragment shaders in the same squad sample different Lods. By @cwfitzgerald in [#3610](https://github.com/gfx-rs/wgpu/pull/3610). +- Fix incorrect mipmap being sampled when using `MinLod <= 0.0` and `MaxLod >= 32.0` or when the fragment shader samples different Lods in the same quad. By @cwfitzgerald in [#3610](https://github.com/gfx-rs/wgpu/pull/3610). #### DX12 diff --git a/deno_webgpu/sampler.rs b/deno_webgpu/sampler.rs index e5f230b2dd..6a9fe285d3 100644 --- a/deno_webgpu/sampler.rs +++ b/deno_webgpu/sampler.rs @@ -40,7 +40,7 @@ pub struct CreateSamplerArgs { lod_min_clamp: f32, lod_max_clamp: f32, compare: Option, - max_anisotropy: u8, + max_anisotropy: f32, } #[op] @@ -67,7 +67,7 @@ pub fn op_webgpu_create_sampler( lod_min_clamp: args.lod_min_clamp, lod_max_clamp: args.lod_max_clamp, compare: args.compare, - anisotropy_clamp: std::num::NonZeroU8::new(args.max_anisotropy), + anisotropy_clamp: args.max_anisotropy, border_color: None, // native-only }; diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 5f6a148129..3967d226da 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1310,37 +1310,53 @@ impl Device { self.require_features(wgt::Features::ADDRESS_MODE_CLAMP_TO_ZERO)?; } - if desc.lod_min_clamp < 0.0 || desc.lod_max_clamp < desc.lod_min_clamp { - return Err(resource::CreateSamplerError::InvalidLodClamp( - desc.lod_min_clamp..desc.lod_max_clamp, + if desc.lod_min_clamp < 0.0 { + return Err(resource::CreateSamplerError::InvalidLodMinClamp( + desc.lod_min_clamp, )); } + if desc.lod_max_clamp < desc.lod_min_clamp { + return Err(resource::CreateSamplerError::InvalidLodMaxClamp { + lod_min_clamp: desc.lod_min_clamp, + lod_max_clamp: desc.lod_max_clamp, + }); + } - let lod_clamp = if desc.lod_min_clamp > 0.0 || desc.lod_max_clamp < 32.0 { - Some(desc.lod_min_clamp..desc.lod_max_clamp) - } else { - None - }; + if !(1.0..=16.0).contains(&desc.anisotropy_clamp) { + return Err(resource::CreateSamplerError::InvalidAnisotropy( + desc.anisotropy_clamp, + )); + } - let anisotropy_clamp = if let Some(clamp) = desc.anisotropy_clamp { - let clamp = clamp.get(); - let valid_clamp = - clamp <= hal::MAX_ANISOTROPY && conv::is_power_of_two_u32(clamp as u32); - if !valid_clamp { - return Err(resource::CreateSamplerError::InvalidClamp(clamp)); + if desc.anisotropy_clamp != 1.0 { + if !matches!(desc.min_filter, wgt::FilterMode::Linear) { + return Err( + resource::CreateSamplerError::InvalidFilterModeWithAnisotropy { + filter_type: resource::SamplerFilterErrorType::MinFilter, + filter_mode: desc.min_filter, + anisotropic_clamp: desc.anisotropy_clamp, + }, + ); } - if self - .downlevel - .flags - .contains(wgt::DownlevelFlags::ANISOTROPIC_FILTERING) - { - std::num::NonZeroU8::new(clamp) - } else { - None + if !matches!(desc.mag_filter, wgt::FilterMode::Linear) { + return Err( + resource::CreateSamplerError::InvalidFilterModeWithAnisotropy { + filter_type: resource::SamplerFilterErrorType::MagFilter, + filter_mode: desc.mag_filter, + anisotropic_clamp: desc.anisotropy_clamp, + }, + ); } - } else { - None - }; + if !matches!(desc.mipmap_filter, wgt::FilterMode::Linear) { + return Err( + resource::CreateSamplerError::InvalidFilterModeWithAnisotropy { + filter_type: resource::SamplerFilterErrorType::MipmapFilter, + filter_mode: desc.mipmap_filter, + anisotropic_clamp: desc.anisotropy_clamp, + }, + ); + } + } //TODO: check for wgt::DownlevelFlags::COMPARISON_SAMPLERS @@ -1350,9 +1366,9 @@ impl Device { mag_filter: desc.mag_filter, min_filter: desc.min_filter, mipmap_filter: desc.mipmap_filter, - lod_clamp, + lod_clamp: desc.lod_min_clamp..desc.lod_max_clamp, compare: desc.compare, - anisotropy_clamp, + anisotropy_clamp: desc.anisotropy_clamp, border_color: desc.border_color, }; diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 648b7f9e7e..5f4fcf83f1 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -11,7 +11,7 @@ use crate::{ use smallvec::SmallVec; use thiserror::Error; -use std::{borrow::Borrow, num::NonZeroU8, ops::Range, ptr::NonNull}; +use std::{borrow::Borrow, ops::Range, ptr::NonNull}; /// The status code provided to the buffer mapping callback. /// @@ -689,8 +689,8 @@ pub struct SamplerDescriptor<'a> { pub lod_max_clamp: f32, /// If this is enabled, this is a comparison sampler using the given comparison function. pub compare: Option, - /// Valid values: 1, 2, 4, 8, and 16. - pub anisotropy_clamp: Option, + /// Valid values between 1 and 16 inclusive. If this is not 1.0, all filter modes must be linear. + pub anisotropy_clamp: f32, /// Border color to use when address_mode is /// [`AddressMode::ClampToBorder`](wgt::AddressMode::ClampToBorder) pub border_color: Option, @@ -707,7 +707,7 @@ impl Default for SamplerDescriptor<'_> { lod_min_clamp: 0.0, lod_max_clamp: std::f32::MAX, compare: None, - anisotropy_clamp: None, + anisotropy_clamp: 1.0, border_color: None, } } @@ -724,14 +724,42 @@ pub struct Sampler { pub(crate) filtering: bool, } +#[derive(Copy, Clone)] +pub enum SamplerFilterErrorType { + MagFilter, + MinFilter, + MipmapFilter, +} + +impl std::fmt::Debug for SamplerFilterErrorType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match *self { + SamplerFilterErrorType::MagFilter => write!(f, "magFilter"), + SamplerFilterErrorType::MinFilter => write!(f, "minFilter"), + SamplerFilterErrorType::MipmapFilter => write!(f, "mipmapFilter"), + } + } +} + #[derive(Clone, Debug, Error)] pub enum CreateSamplerError { #[error(transparent)] Device(#[from] DeviceError), - #[error("Invalid lod clamp lod_min_clamp:{} lod_max_clamp:{}, must satisfy lod_min_clamp >= 0 and lod_max_clamp >= lod_min_clamp ", .0.start, .0.end)] - InvalidLodClamp(Range), - #[error("Invalid anisotropic clamp {0}, must be one of 1, 2, 4, 8 or 16")] - InvalidClamp(u8), + #[error("Invalid lodMinClamp: {0}. Must be greater or equal to 0.0")] + InvalidLodMinClamp(f32), + #[error("Invalid lodMaxClamp: {lod_max_clamp}. Must be greater or equal to lodMinClamp (which is {lod_min_clamp}).")] + InvalidLodMaxClamp { + lod_min_clamp: f32, + lod_max_clamp: f32, + }, + #[error("Invalid anisotropic clamp: {0}. Must be in the range 1 to 16 inclusive.")] + InvalidAnisotropy(f32), + #[error("Invalid filter mode for {filter_type:?}: {filter_mode:?}. When anistropic clamp is not 1.0 (it is {anisotropic_clamp}), all filter modes must be linear.")] + InvalidFilterModeWithAnisotropy { + filter_type: SamplerFilterErrorType, + filter_mode: wgt::FilterMode, + anisotropic_clamp: f32, + }, #[error("Cannot create any more samplers")] TooManyObjects, /// AddressMode::ClampToBorder requires feature ADDRESS_MODE_CLAMP_TO_BORDER. diff --git a/wgpu-hal/examples/halmark/main.rs b/wgpu-hal/examples/halmark/main.rs index b4f25c9179..d70908fc6c 100644 --- a/wgpu-hal/examples/halmark/main.rs +++ b/wgpu-hal/examples/halmark/main.rs @@ -355,9 +355,9 @@ impl Example { mag_filter: wgt::FilterMode::Linear, min_filter: wgt::FilterMode::Nearest, mipmap_filter: wgt::FilterMode::Nearest, - lod_clamp: None, + lod_clamp: 0.0..32.0, compare: None, - anisotropy_clamp: None, + anisotropy_clamp: 1.0, border_color: None, }; let sampler = unsafe { device.create_sampler(&sampler_desc).unwrap() }; diff --git a/wgpu-hal/src/dx12/device.rs b/wgpu-hal/src/dx12/device.rs index 24fea55663..16b9918f38 100644 --- a/wgpu-hal/src/dx12/device.rs +++ b/wgpu-hal/src/dx12/device.rs @@ -583,13 +583,14 @@ impl crate::Device for super::Device { Some(_) => d3d12_ty::D3D12_FILTER_REDUCTION_TYPE_COMPARISON, None => d3d12_ty::D3D12_FILTER_REDUCTION_TYPE_STANDARD, }; - let filter = conv::map_filter_mode(desc.min_filter) << d3d12_ty::D3D12_MIN_FILTER_SHIFT + let mut filter = conv::map_filter_mode(desc.min_filter) << d3d12_ty::D3D12_MIN_FILTER_SHIFT | conv::map_filter_mode(desc.mag_filter) << d3d12_ty::D3D12_MAG_FILTER_SHIFT | conv::map_filter_mode(desc.mipmap_filter) << d3d12_ty::D3D12_MIP_FILTER_SHIFT - | reduction << d3d12_ty::D3D12_FILTER_REDUCTION_TYPE_SHIFT - | desc - .anisotropy_clamp - .map_or(0, |_| d3d12_ty::D3D12_FILTER_ANISOTROPIC); + | reduction << d3d12_ty::D3D12_FILTER_REDUCTION_TYPE_SHIFT; + + if desc.anisotropy_clamp != 1.0 { + filter |= d3d12_ty::D3D12_FILTER_ANISOTROPIC; + }; let border_color = conv::map_border_color(desc.border_color); @@ -602,10 +603,10 @@ impl crate::Device for super::Device { conv::map_address_mode(desc.address_modes[2]), ], 0.0, - desc.anisotropy_clamp.map_or(0, |aniso| aniso.get() as u32), + desc.anisotropy_clamp as u32, conv::map_comparison(desc.compare.unwrap_or(wgt::CompareFunction::Always)), border_color, - desc.lod_clamp.clone().unwrap_or(0.0..16.0), + desc.lod_clamp.clone(), ); Ok(super::Sampler { handle }) diff --git a/wgpu-hal/src/gles/device.rs b/wgpu-hal/src/gles/device.rs index d994aa1d56..208fd0a9bc 100644 --- a/wgpu-hal/src/gles/device.rs +++ b/wgpu-hal/src/gles/device.rs @@ -864,16 +864,12 @@ impl crate::Device for super::Device { unsafe { gl.sampler_parameter_f32_slice(raw, glow::TEXTURE_BORDER_COLOR, &border) }; } - if let Some(ref range) = desc.lod_clamp { - unsafe { gl.sampler_parameter_f32(raw, glow::TEXTURE_MIN_LOD, range.start) }; - unsafe { gl.sampler_parameter_f32(raw, glow::TEXTURE_MAX_LOD, range.end) }; - } + unsafe { gl.sampler_parameter_f32(raw, glow::TEXTURE_MIN_LOD, desc.lod_clamp.start) }; + unsafe { gl.sampler_parameter_f32(raw, glow::TEXTURE_MAX_LOD, desc.lod_clamp.end) }; - if let Some(anisotropy) = desc.anisotropy_clamp { - unsafe { - gl.sampler_parameter_i32(raw, glow::TEXTURE_MAX_ANISOTROPY, anisotropy.get() as i32) - }; - } + unsafe { + gl.sampler_parameter_f32(raw, glow::TEXTURE_MAX_ANISOTROPY, desc.anisotropy_clamp) + }; //set_param_float(glow::TEXTURE_LOD_BIAS, info.lod_bias.0); diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index 814c451f06..bafa4d8427 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -87,7 +87,7 @@ pub mod api { use std::{ borrow::{Borrow, Cow}, fmt, - num::{NonZeroU32, NonZeroU8}, + num::NonZeroU32, ops::{Range, RangeInclusive}, ptr::NonNull, sync::atomic::AtomicBool, @@ -919,9 +919,10 @@ pub struct SamplerDescriptor<'a> { pub mag_filter: wgt::FilterMode, pub min_filter: wgt::FilterMode, pub mipmap_filter: wgt::FilterMode, - pub lod_clamp: Option>, + pub lod_clamp: Range, pub compare: Option, - pub anisotropy_clamp: Option, + // Must be in the range 1.0 to 16.0 inclusive. Anisotropic filtering must be supported if this is not 1.0. + pub anisotropy_clamp: f32, pub border_color: Option, } diff --git a/wgpu-hal/src/metal/device.rs b/wgpu-hal/src/metal/device.rs index 6084a7164a..48a87757bd 100644 --- a/wgpu-hal/src/metal/device.rs +++ b/wgpu-hal/src/metal/device.rs @@ -415,6 +415,9 @@ impl crate::Device for super::Device { descriptor.set_min_filter(conv::map_filter_mode(desc.min_filter)); descriptor.set_mag_filter(conv::map_filter_mode(desc.mag_filter)); descriptor.set_mip_filter(match desc.mipmap_filter { + wgt::FilterMode::Nearest if desc.lod_clamp == (0.0..0.0) => { + metal::MTLSamplerMipFilter::NotMipmapped + } wgt::FilterMode::Nearest => metal::MTLSamplerMipFilter::Nearest, wgt::FilterMode::Linear => metal::MTLSamplerMipFilter::Linear, }); @@ -424,14 +427,10 @@ impl crate::Device for super::Device { descriptor.set_address_mode_t(conv::map_address_mode(t)); descriptor.set_address_mode_r(conv::map_address_mode(r)); - if let Some(aniso) = desc.anisotropy_clamp { - descriptor.set_max_anisotropy(aniso.get() as _); - } + descriptor.set_max_anisotropy(desc.anisotropy_clamp as _); - if let Some(ref range) = desc.lod_clamp { - descriptor.set_lod_min_clamp(range.start); - descriptor.set_lod_max_clamp(range.end); - } + descriptor.set_lod_min_clamp(desc.lod_clamp.start); + descriptor.set_lod_max_clamp(desc.lod_clamp.end); if let Some(fun) = desc.compare { descriptor.set_compare_function(conv::map_compare_function(fun)); diff --git a/wgpu-hal/src/vulkan/device.rs b/wgpu-hal/src/vulkan/device.rs index 1d10d69b0a..b476ad7d75 100644 --- a/wgpu-hal/src/vulkan/device.rs +++ b/wgpu-hal/src/vulkan/device.rs @@ -1103,8 +1103,6 @@ impl crate::Device for super::Device { &self, desc: &crate::SamplerDescriptor, ) -> Result { - let lod_range = desc.lod_clamp.clone().unwrap_or(0.0..16.0); - let mut vk_info = vk::SamplerCreateInfo::builder() .flags(vk::SamplerCreateFlags::empty()) .mag_filter(conv::map_filter_mode(desc.mag_filter)) @@ -1113,8 +1111,8 @@ impl crate::Device for super::Device { .address_mode_u(conv::map_address_mode(desc.address_modes[0])) .address_mode_v(conv::map_address_mode(desc.address_modes[1])) .address_mode_w(conv::map_address_mode(desc.address_modes[2])) - .min_lod(lod_range.start) - .max_lod(lod_range.end); + .min_lod(desc.lod_clamp.start) + .max_lod(desc.lod_clamp.end); if let Some(fun) = desc.compare { vk_info = vk_info @@ -1122,16 +1120,14 @@ impl crate::Device for super::Device { .compare_op(conv::map_comparison(fun)); } - if let Some(aniso) = desc.anisotropy_clamp { - if self - .shared - .downlevel_flags - .contains(wgt::DownlevelFlags::ANISOTROPIC_FILTERING) - { - vk_info = vk_info - .anisotropy_enable(true) - .max_anisotropy(aniso.get() as f32); - } + if self + .shared + .downlevel_flags + .contains(wgt::DownlevelFlags::ANISOTROPIC_FILTERING) + { + vk_info = vk_info + .anisotropy_enable(true) + .max_anisotropy(desc.anisotropy_clamp); } if let Some(color) = desc.border_color { diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index 5d937db745..8d4efbe561 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -19,7 +19,7 @@ use std::{ fmt::{Debug, Display}, future::Future, marker::PhantomData, - num::{NonZeroU32, NonZeroU8}, + num::NonZeroU32, ops::{Bound, Deref, DerefMut, Range, RangeBounds}, sync::Arc, thread, @@ -1008,8 +1008,8 @@ pub struct SamplerDescriptor<'a> { pub lod_max_clamp: f32, /// If this is enabled, this is a comparison sampler using the given comparison function. pub compare: Option, - /// Valid values: 1, 2, 4, 8, and 16. - pub anisotropy_clamp: Option, + /// Valid values between 1 and 16 inclusive. If this is not 1.0, all filter modes must be linear. + pub anisotropy_clamp: f32, /// Border color to use when address_mode is [`AddressMode::ClampToBorder`] pub border_color: Option, } @@ -1028,7 +1028,7 @@ impl Default for SamplerDescriptor<'_> { lod_min_clamp: 0.0, lod_max_clamp: std::f32::MAX, compare: None, - anisotropy_clamp: None, + anisotropy_clamp: 1.0, border_color: None, } }