Fix Metal Mipmap Behvior (#3610)

This commit is contained in:
Connor Fitzgerald 2023-03-21 12:37:30 -04:00 committed by GitHub
parent a502282307
commit 0c3ca5c08b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 158 additions and 110 deletions

View File

@ -39,6 +39,7 @@ Bottom level categories:
-->
## Unreleased
### Major changes
#### TextureFormat info API
@ -81,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 u16 (was a `Option<u8>`) which must be at least 1.
If the anisotropy clamp is not 1, all the filters in a sampler must be `Linear`.
```diff
SamplerDescriptor {
- anisotropic_clamp: None,
+ anisotropic_clamp: 1,
}
```
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<NonZeroU32>` to `Option<u32>`. By @teoxoy in [#3445](https://github.com/gfx-rs/wgpu/pull/3445)
@ -113,6 +129,9 @@ 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 shader samples different Lods in the same quad. By @cwfitzgerald in [#3610](https://github.com/gfx-rs/wgpu/pull/3610).
#### DX12
- Fix DXC validation issues when using a custom `dxil_path`. By @Elabajaba in [#3434](https://github.com/gfx-rs/wgpu/pull/3434)

View File

@ -40,7 +40,7 @@ pub struct CreateSamplerArgs {
lod_min_clamp: f32,
lod_max_clamp: f32,
compare: Option<wgpu_types::CompareFunction>,
max_anisotropy: u8,
max_anisotropy: u16,
}
#[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
};

View File

@ -1310,36 +1310,64 @@ impl<A: HalApi> Device<A> {
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,
});
}
if desc.anisotropy_clamp < 1 {
return Err(resource::CreateSamplerError::InvalidAnisotropy(
desc.anisotropy_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 desc.anisotropy_clamp != 1 {
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 !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,
},
);
}
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,
},
);
}
}
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 self
.downlevel
.flags
.contains(wgt::DownlevelFlags::ANISOTROPIC_FILTERING)
{
std::num::NonZeroU8::new(clamp)
} else {
None
}
let anisotropy_clamp = if self
.downlevel
.flags
.contains(wgt::DownlevelFlags::ANISOTROPIC_FILTERING)
{
// Clamp anisotropy clamp to [1, 16] per the wgpu-hal interface
desc.anisotropy_clamp.min(16)
} else {
None
// If it isn't supported, set this unconditionally to 1
1
};
//TODO: check for wgt::DownlevelFlags::COMPARISON_SAMPLERS
@ -1350,7 +1378,7 @@ impl<A: HalApi> Device<A> {
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,
border_color: desc.border_color,

View File

@ -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,30 +689,13 @@ 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<wgt::CompareFunction>,
/// Valid values: 1, 2, 4, 8, and 16.
pub anisotropy_clamp: Option<NonZeroU8>,
/// Must be at least 1. If this is not 1, all filter modes must be linear.
pub anisotropy_clamp: u16,
/// Border color to use when address_mode is
/// [`AddressMode::ClampToBorder`](wgt::AddressMode::ClampToBorder)
pub border_color: Option<wgt::SamplerBorderColor>,
}
impl Default for SamplerDescriptor<'_> {
fn default() -> Self {
Self {
label: None,
address_modes: Default::default(),
mag_filter: Default::default(),
min_filter: Default::default(),
mipmap_filter: Default::default(),
lod_min_clamp: 0.0,
lod_max_clamp: std::f32::MAX,
compare: None,
anisotropy_clamp: None,
border_color: None,
}
}
}
#[derive(Debug)]
pub struct Sampler<A: hal::Api> {
pub(crate) raw: A::Sampler,
@ -724,14 +707,42 @@ pub struct Sampler<A: hal::Api> {
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<f32>),
#[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 at least 1.")]
InvalidAnisotropy(u16),
#[error("Invalid filter mode for {filter_type:?}: {filter_mode:?}. When anistropic clamp is not 1 (it is {anisotropic_clamp}), all filter modes must be linear.")]
InvalidFilterModeWithAnisotropy {
filter_type: SamplerFilterErrorType,
filter_mode: wgt::FilterMode,
anisotropic_clamp: u16,
},
#[error("Cannot create any more samplers")]
TooManyObjects,
/// AddressMode::ClampToBorder requires feature ADDRESS_MODE_CLAMP_TO_BORDER.

View File

@ -355,9 +355,9 @@ impl<A: hal::Api> Example<A> {
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,
border_color: None,
};
let sampler = unsafe { device.create_sampler(&sampler_desc).unwrap() };

View File

@ -583,13 +583,14 @@ impl crate::Device<super::Api> 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 {
filter |= d3d12_ty::D3D12_FILTER_ANISOTROPIC;
};
let border_color = conv::map_border_color(desc.border_color);
@ -602,10 +603,10 @@ impl crate::Device<super::Api> 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 })

View File

@ -315,10 +315,11 @@ impl super::Adapter {
&& (vertex_shader_storage_blocks != 0 || vertex_ssbo_false_zero),
);
downlevel_flags.set(wgt::DownlevelFlags::FRAGMENT_STORAGE, supports_storage);
downlevel_flags.set(
wgt::DownlevelFlags::ANISOTROPIC_FILTERING,
extensions.contains("EXT_texture_filter_anisotropic"),
);
if extensions.contains("EXT_texture_filter_anisotropic") {
let max_aniso =
unsafe { gl.get_parameter_i32(glow::MAX_TEXTURE_MAX_ANISOTROPY_EXT) } as u32;
downlevel_flags.set(wgt::DownlevelFlags::ANISOTROPIC_FILTERING, max_aniso >= 16);
}
downlevel_flags.set(
wgt::DownlevelFlags::BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED,
!(cfg!(target_arch = "wasm32") || is_angle),

View File

@ -864,14 +864,17 @@ impl crate::Device<super::Api> 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 {
// If clamp is not 1, we know anisotropy is supported up to 16x
if desc.anisotropy_clamp != 1 {
unsafe {
gl.sampler_parameter_i32(raw, glow::TEXTURE_MAX_ANISOTROPY, anisotropy.get() as i32)
gl.sampler_parameter_i32(
raw,
glow::TEXTURE_MAX_ANISOTROPY,
desc.anisotropy_clamp as i32,
)
};
}

View File

@ -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,12 @@ pub struct SamplerDescriptor<'a> {
pub mag_filter: wgt::FilterMode,
pub min_filter: wgt::FilterMode,
pub mipmap_filter: wgt::FilterMode,
pub lod_clamp: Option<Range<f32>>,
pub lod_clamp: Range<f32>,
pub compare: Option<wgt::CompareFunction>,
pub anisotropy_clamp: Option<NonZeroU8>,
// Must in the range [1, 16].
//
// Anisotropic filtering must be supported if this is not 1.
pub anisotropy_clamp: u16,
pub border_color: Option<wgt::SamplerBorderColor>,
}

View File

@ -540,7 +540,6 @@ impl super::PrivateCapabilities {
MUTABLE_COMPARISON_SAMPLER_SUPPORT,
),
sampler_clamp_to_border: Self::supports_any(device, SAMPLER_CLAMP_TO_BORDER_SUPPORT),
sampler_lod_average: { version.at_least((11, 0), (9, 0), os_is_mac) },
base_instance: Self::supports_any(device, BASE_INSTANCE_SUPPORT),
base_vertex_instance_drawing: Self::supports_any(device, BASE_VERTEX_INSTANCE_SUPPORT),
dual_source_blending: Self::supports_any(device, DUAL_SOURCE_BLEND_SUPPORT),

View File

@ -423,14 +423,13 @@ impl crate::Device<super::Api> for super::Device {
&self,
desc: &crate::SamplerDescriptor,
) -> DeviceResult<super::Sampler> {
let caps = &self.shared.private_caps;
objc::rc::autoreleasepool(|| {
let descriptor = metal::SamplerDescriptor::new();
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.is_none() => {
wgt::FilterMode::Nearest if desc.lod_clamp == (0.0..0.0) => {
metal::MTLSamplerMipFilter::NotMipmapped
}
wgt::FilterMode::Nearest => metal::MTLSamplerMipFilter::Nearest,
@ -442,18 +441,11 @@ impl crate::Device<super::Api> 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 _);
}
// Anisotropy is always supported on mac up to 16x
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);
}
if caps.sampler_lod_average {
descriptor.set_lod_average(true); // optimization
}
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));

View File

@ -151,7 +151,6 @@ struct PrivateCapabilities {
shared_textures: bool,
mutable_comparison_samplers: bool,
sampler_clamp_to_border: bool,
sampler_lod_average: bool,
base_instance: bool,
base_vertex_instance_drawing: bool,
dual_source_blending: bool,

View File

@ -1320,7 +1320,6 @@ impl super::Adapter {
},
vendor_id: self.phd_capabilities.properties.vendor_id,
timestamp_period: self.phd_capabilities.properties.limits.timestamp_period,
downlevel_flags: self.downlevel_flags,
private_caps: self.private_caps.clone(),
workarounds: self.workarounds,
render_passes: Mutex::new(Default::default()),

View File

@ -1103,8 +1103,6 @@ impl crate::Device<super::Api> for super::Device {
&self,
desc: &crate::SamplerDescriptor,
) -> Result<super::Sampler, crate::DeviceError> {
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<super::Api> 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,12 @@ impl crate::Device<super::Api> 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 desc.anisotropy_clamp != 1 {
// We only enable anisotropy if it is supported, and wgpu-hal interface guarentees
// the clamp is in the range [1, 16] which is always supported if anisotropy is.
vk_info = vk_info
.anisotropy_enable(true)
.max_anisotropy(desc.anisotropy_clamp as f32);
}
if let Some(color) = desc.border_color {

View File

@ -249,7 +249,6 @@ struct DeviceShared {
extension_fns: DeviceExtensionFunctions,
vendor_id: u32,
timestamp_period: f32,
downlevel_flags: wgt::DownlevelFlags,
private_caps: PrivateCapabilities,
workarounds: Workarounds,
render_passes: Mutex<rustc_hash::FxHashMap<RenderPassKey, vk::RenderPass>>,

View File

@ -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<CompareFunction>,
/// Valid values: 1, 2, 4, 8, and 16.
pub anisotropy_clamp: Option<NonZeroU8>,
/// Must be at least 1. If this is not 1, all filter modes must be linear.
pub anisotropy_clamp: u16,
/// Border color to use when address_mode is [`AddressMode::ClampToBorder`]
pub border_color: Option<SamplerBorderColor>,
}
@ -1026,9 +1026,9 @@ impl Default for SamplerDescriptor<'_> {
min_filter: Default::default(),
mipmap_filter: Default::default(),
lod_min_clamp: 0.0,
lod_max_clamp: std::f32::MAX,
lod_max_clamp: 32.0,
compare: None,
anisotropy_clamp: None,
anisotropy_clamp: 1,
border_color: None,
}
}