[Impeller] Add anisotropic filtering support to samplers#188056
Conversation
7e2f369 to
da3c015
Compare
There was a problem hiding this comment.
Code Review
This pull request adds support for anisotropic filtering across the GLES, Metal, and Vulkan backends in Impeller, exposing the capability to the Flutter GPU Dart API. It introduces max_anisotropy to SamplerDescriptor, implements device capability queries, and adds validation to ensure anisotropic filtering is only configured when linear filtering is active. Feedback points out a potential undefined behavior issue in the Vulkan backend where std::clamp could be invoked with a lower bound greater than the upper bound if the device's reported maximum anisotropy is less than 1.0.
Adds a max_anisotropy field to SamplerDescriptor (1 disables anisotropic filtering, matching WebGPU) and implements it on all three backends. - Metal sets maxAnisotropy on MTLSamplerDescriptor, always supported. - Vulkan enables the samplerAnisotropy device feature when available and clamps to maxSamplerAnisotropy in SamplerLibraryVK. - GLES applies GL_TEXTURE_MAX_ANISOTROPY_EXT when GL_EXT_texture_filter_anisotropic is present, using only core ES 2.0 entry points (GetFloatv/TexParameterfv). The device limit is exposed via Capabilities::GetMaxSamplerAnisotropy (1 means unsupported). Flutter GPU exposes SamplerOptions.maxAnisotropy and GpuContext.maxSamplerAnisotropy, requiring all filters to be linear when anisotropy is enabled.
da3c015 to
a5c1419
Compare
Guards against std::clamp seeing an inverted range if a driver reports a max anisotropy below 1. The real path always reports at least 1, so this is defensive.
The success-path assertion bound a sampler to a non-texture uniform slot, which fails at the FFI with "Failed to bind texture". The two validation throw assertions cover the test's purpose.
gaaclarke
left a comment
There was a problem hiding this comment.
lgtm, one question about types since we sometimes use float, sometimes double, sometimes int. Often times we opt for "Scalar"for floating point values, I'm not sure if that makes sense here though.
| return max_texture_size; | ||
| } | ||
|
|
||
| float CapabilitiesGLES::GetMaxSamplerAnisotropy() const { |
There was a problem hiding this comment.
is float the right type for this?
There was a problem hiding this comment.
Yeah we should change it to int. GL/VK use floats, but it's actually an int ceiling. Metal uses ints. Done.
| /// Anisotropic filtering samples across the mip chain, so populate mip | ||
| /// levels (via [Texture.overwrite] or by rendering into them) to get the | ||
| /// full quality benefit. | ||
| int maxAnisotropy; |
There was a problem hiding this comment.
Switched to int everywhere
The device max anisotropy was a float mirroring the Vulkan and GL source, but it is only ever used as an integer ceiling and the public value is an integer. Return uint32_t from the capability and int from the Dart getter so the value and the limit are consistent.
Adds anisotropic texture filtering to Impeller and surfaces it through Flutter GPU. Anisotropic filtering is a sampler read-time setting, so this needs no changes to attachments or texture upload.
A new
max_anisotropyfield onSamplerDescriptorsets the maximum anisotropy clamp, where 1 disables anisotropic filtering (matching WebGPU). It is implemented across all three backends.maxAnisotropyonMTLSamplerDescriptor, which every device supports.samplerAnisotropydevice feature when available and clamps tomaxSamplerAnisotropyinSamplerLibraryVK.GL_TEXTURE_MAX_ANISOTROPY_EXTwhenGL_EXT_texture_filter_anisotropicis present, using only core ES 2.0 entry points (GetFloatvandTexParameterfv).The device limit is reported by
Capabilities::GetMaxSamplerAnisotropy, where 1 means anisotropic filtering is unsupported. Flutter GPU addsSamplerOptions.maxAnisotropyandGpuContext.maxSamplerAnisotropy, and requires all filters to be linear when anisotropy is enabled.Anisotropic minification reads across the mip chain... so callers populate mips with
Texture.overwriteor by rendering into them, consistent with the existing mip and slice support.Pre-launch Checklist
///).