[wgsl-in] Ensure textureSampleLevel's level argument is an integer for depth textures #6529
Conversation
Looking at the code in the backend I think we do but there are caveats/restrictions. |
| crate::ImageClass::Depth { .. } => match resolver[expr] { | ||
| Ti::Scalar(Sc { | ||
| kind: Sk::Sint | Sk::Uint, | ||
| .. | ||
| }) => {} | ||
| _ => { | ||
| return Err(ExpressionError::InvalidSampleLevelExactType(expr)) | ||
| } | ||
| }, |
There was a problem hiding this comment.
Ideally we would modify the GLSL & SPIR-V frontends as well to not regress. I think we need to round the lod then cast it to a u32 so that the behavior is in line with a non-filtering sampler.
There was a problem hiding this comment.
I don't think any GLSL command can be converted to a depth texture textureSampleLevel(). GLSL's textureLod() with a shadow texture takes a vector with an extra component for the coords, and this component is used as the compare value when translated to textureSampleCompareLevel().
textureSampleCompareLevel always has a LOD of zero, so the LOD specified in the GLSL also appears to be lost in translation. I think that occurs here. So as I can tell I don't think there's anything to do for GLSL, but this is all very new to me so could be wrong!
There was a problem hiding this comment.
Looking at the GLSL frontend, that seems accurate. The SPIR-V frontend can produce a textureSampleLevel() with a depth texture though.
@teoxoy Could you point me towards this? I'm not very familiar with these things, but my understanding is GLSL's If I try to translate some wgsl with |
|
Oh wow that's a "nice" inconsistency between GLSL's sampling functions, some take the depth ref as a separate argument (ex On the spec side of things, we are also working on WebGPU Compat (WebGPU on top of OpenGL ES 3.1) and this hasn't been brought up by anyone yet, I will open a new issue. |
…r depth textures Until now we accepted a float, as is the case for non-depth textures. This makes us compliant with the spec. The validator is updated to expect an Sint or Uint when the ImageClass is ImageClass::Depth. The SPIR-V frontend converts the LOD argument from float to Sint (assuming that it is representable), likewise The SPIR-V backend now converts the LOD from either Sint or Uint to Float. HLSL and MSL backends require no changes as they implicitly do that conversion. GLSL does not support non-compare LOD samples, therefore no changes are required.
jamienicol
force-pushed
the
textureSampleLevel
branch
from
74de308
to
4b56262
Compare
| log::debug!( | ||
| "Assuming level {:?} converts losslessly to an integer", | ||
| lod_handle | ||
| ); | ||
| let expr = crate::Expression::As { | ||
| expr: lod_handle, | ||
| kind: crate::ScalarKind::Sint, | ||
| convert: Some(4), | ||
| }; |
There was a problem hiding this comment.
I think that if we round it first this will behave the same as the sampler doing it. Thoughts?
There was a problem hiding this comment.
My assumption was that a fractional lod interpolated between two mip levels. But I can't see anything in the spec which says that (but nor that it rounds the lod). Do you happen to know what the spec says?
There was a problem hiding this comment.
Depth textures can only be paired with a non-filtering (or comparison) sampler which means that the mipmapFilter field of the sampler descriptor can't be linear, it must be nearest which sounds like it rounds instead.
A few links:
https://www.w3.org/TR/webgpu/#ref-for-dom-gpusamplerbindingtype-filtering%E2%91%A2
https://www.w3.org/TR/webgpu/#dom-gpusamplerbindingtype-non-filtering
https://www.w3.org/TR/webgpu/#ref-for-dom-gpusampler-isfiltering-slot
https://www.w3.org/TR/webgpu/#enumdef-gpumipmapfiltermode
Connections
Fixes #4548
Description
The spec specifies that
textureSampleLevel()takes an integer type as thelevelparameter for depth textures (and a float for non-depth textures). We currently accept a float for all texture types. This makes us require the correct type. As a result we now need to convert the value in the SPIR-V backend. For HLSL and MSL,SampleLevel()andmetal::level()appear to implicitly convert the integer to floats just fine. And as far as I can tell we do not support non-compare sampling of depth textures on GLSL?Testing
Manual inspection of output shaders. Shader validation succeeds. Built gecko with change and verified relevant CTS tests (
webgpu:shader,validation,expression,call,builtin,textureSampleLevel:level_argument) now pass.Checklist
cargo fmt.taplo format.cargo clippy. If applicable, add:--target wasm32-unknown-unknown--target wasm32-unknown-emscriptencargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.