Fix glsl backend errors regarding samplerCubeArrayShadow (#5171)

* add GL_EXT_texture_shadow_lod feature detection

* allow more cases of cube depth texture sampling in glsl

* add test for sampling a cubemap array depth texture with lod

* add test for chosing GL_EXT_texture_shadow_lod over the grad workaround if instructed

* add changelog entry for GL_EXT_texture_shadow_lod

* fix criteria for requiring and using TEXTURE_SHADOW_LOD

* require gles 320 for textureSampling over cubeArrayShadow

* prevent false positives in TEXTURE_SHADOW_LOD in checks

* make workaround_lod_with_grad usecase selection less context dependant

* move 3d array texture error into the validator

* correct ImageSample logic errors

CHANGELOG.md

@@ -114,6 +114,9 @@ Bottom level categories:
 
 - In Surface::configure and Surface::present, fix the current GL context not being unset when releasing the lock that guards access to making the context current. This was causing other threads to panic when trying to make the context current. By @Imberflur in [#5087](https://github.com/gfx-rs/wgpu/pull/5087).
 
+#### Naga
+- Make use of `GL_EXT_texture_shadow_lod` to support sampling a cube depth texture with an explicit LOD. By @cmrschwarz in #[5171](https://github.com/gfx-rs/wgpu/pull/5171).
+
 #### Tests
 
 - Fix intermittent crashes on Linux in the `multithreaded_compute` test. By @jimblandy in [#5129](https://github.com/gfx-rs/wgpu/pull/5129).

naga/src/back/glsl/features.rs

@@ -1,8 +1,8 @@
 use super::{BackendResult, Error, Version, Writer};
 use crate::{
     back::glsl::{Options, WriterFlags},
-    AddressSpace, Binding, Expression, Handle, ImageClass, ImageDimension, Interpolation, Sampling,
-    Scalar, ScalarKind, ShaderStage, StorageFormat, Type, TypeInner,
+    AddressSpace, Binding, Expression, Handle, ImageClass, ImageDimension, Interpolation,
+    SampleLevel, Sampling, Scalar, ScalarKind, ShaderStage, StorageFormat, Type, TypeInner,
 };
 use std::fmt::Write;
 
@@ -48,6 +48,8 @@ bitflags::bitflags! {
         ///
         /// We can always support this, either through the language or a polyfill
         const INSTANCE_INDEX = 1 << 22;
+        /// Sample specific LODs of cube / array shadow textures
+        const TEXTURE_SHADOW_LOD = 1 << 23;
     }
 }
 
@@ -125,6 +127,7 @@ impl FeaturesManager {
         check_feature!(TEXTURE_SAMPLES, 150);
         check_feature!(TEXTURE_LEVELS, 130);
         check_feature!(IMAGE_SIZE, 430, 310);
+        check_feature!(TEXTURE_SHADOW_LOD, 200, 300);
 
         // Return an error if there are missing features
         if missing.is_empty() {
@@ -251,6 +254,11 @@ impl FeaturesManager {
             }
         }
 
+        if self.0.contains(Features::TEXTURE_SHADOW_LOD) {
+            // https://registry.khronos.org/OpenGL/extensions/EXT/EXT_texture_shadow_lod.txt
+            writeln!(out, "#extension GL_EXT_texture_shadow_lod : require")?;
+        }
+
         Ok(())
     }
 }
@@ -469,6 +477,47 @@ impl<'a, W> Writer<'a, W> {
                         }
                     }
                 }
+                Expression::ImageSample { image, level, offset, .. } => {
+                    if let TypeInner::Image {
+                        dim,
+                        arrayed,
+                        class: ImageClass::Depth { .. },
+                    } = *info[image].ty.inner_with(&module.types) {
+                        let lod = matches!(level, SampleLevel::Zero | SampleLevel::Exact(_));
+                        let bias = matches!(level, SampleLevel::Bias(_));
+                        let auto = matches!(level, SampleLevel::Auto);
+                        let cube = dim == ImageDimension::Cube;
+                        let array2d = dim == ImageDimension::D2 && arrayed;
+                        let gles = self.options.version.is_es();
+
+                        // We have a workaround of using `textureGrad` instead of `textureLod` if the LOD is zero,
+                        // so we don't *need* this extension for those cases.
+                        // But if we're explicitly allowed to use the extension (`WriterFlags::TEXTURE_SHADOW_LOD`),
+                        // we always use it instead of the workaround.
+                        let grad_workaround_applicable = (array2d || (cube && !arrayed)) && level == SampleLevel::Zero;
+                        let prefer_grad_workaround = grad_workaround_applicable && !self.options.writer_flags.contains(WriterFlags::TEXTURE_SHADOW_LOD);
+
+                        let mut ext_used = false;
+
+                        // float texture(sampler2DArrayShadow sampler, vec4 P [, float bias])
+                        // float texture(samplerCubeArrayShadow sampler, vec4 P, float compare [, float bias])
+                        ext_used |= (array2d || cube && arrayed) && bias;
+
+                        // The non `bias` version of this was standardized in GL 4.3, but never in GLES.
+                        // float textureOffset(sampler2DArrayShadow sampler, vec4 P, ivec2 offset [, float bias])
+                        ext_used |= array2d && (bias || (gles && auto)) && offset.is_some();
+
+                        // float textureLod(sampler2DArrayShadow sampler, vec4 P, float lod)
+                        // float textureLodOffset(sampler2DArrayShadow sampler, vec4 P, float lod, ivec2 offset)
+                        // float textureLod(samplerCubeShadow sampler, vec4 P, float lod)
+                        // float textureLod(samplerCubeArrayShadow sampler, vec4 P, float compare, float lod)
+                        ext_used |= (cube || array2d) && lod && !prefer_grad_workaround;
+
+                        if ext_used {
+                            features.request(Features::TEXTURE_SHADOW_LOD);
+                        }
+                    }
+                }
                 _ => {}
             }
             }

naga/src/back/glsl/mod.rs

@@ -646,16 +646,6 @@ impl<'a, W: Write> Writer<'a, W> {
         // preprocessor not the processor ¯\_(ツ)_/¯
         self.features.write(self.options, &mut self.out)?;
 
-        // Write the additional extensions
-        if self
-            .options
-            .writer_flags
-            .contains(WriterFlags::TEXTURE_SHADOW_LOD)
-        {
-            // https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_texture_shadow_lod.txt
-            writeln!(self.out, "#extension GL_EXT_texture_shadow_lod : require")?;
-        }
-
         // glsl es requires a precision to be specified for floats and ints
         // TODO: Should this be user configurable?
         if es {
@@ -2620,51 +2610,49 @@ impl<'a, W: Write> Writer<'a, W> {
                 level,
                 depth_ref,
             } => {
-                let dim = match *ctx.resolve_type(image, &self.module.types) {
-                    TypeInner::Image { dim, .. } => dim,
+                let (dim, class, arrayed) = match *ctx.resolve_type(image, &self.module.types) {
+                    TypeInner::Image {
+                        dim,
+                        class,
+                        arrayed,
+                        ..
+                    } => (dim, class, arrayed),
                     _ => unreachable!(),
                 };
-
-                if dim == crate::ImageDimension::Cube
-                    && array_index.is_some()
-                    && depth_ref.is_some()
+                let mut err = None;
+                if dim == crate::ImageDimension::Cube {
+                    if offset.is_some() {
+                        err = Some("gsamplerCube[Array][Shadow] doesn't support texture sampling with offsets");
+                    }
+                    if arrayed
+                        && matches!(class, crate::ImageClass::Depth { .. })
+                        && matches!(level, crate::SampleLevel::Gradient { .. })
                     {
-                    match level {
-                        crate::SampleLevel::Zero
-                        | crate::SampleLevel::Exact(_)
-                        | crate::SampleLevel::Gradient { .. }
-                        | crate::SampleLevel::Bias(_) => {
-                            return Err(Error::Custom(String::from(
-                                "gsamplerCubeArrayShadow isn't supported in textureGrad, \
-                                 textureLod or texture with bias",
-                            )))
+                        err = Some("samplerCubeArrayShadow don't support textureGrad");
                     }
-                        crate::SampleLevel::Auto => {}
                 }
+                if gather.is_some() && level != crate::SampleLevel::Zero {
+                    err = Some("textureGather doesn't support LOD parameters");
+                }
+                if let Some(err) = err {
+                    return Err(Error::Custom(String::from(err)));
                 }
 
-                // textureLod on sampler2DArrayShadow and samplerCubeShadow does not exist in GLSL.
-                // To emulate this, we will have to use textureGrad with a constant gradient of 0.
-                let workaround_lod_array_shadow_as_grad = (array_index.is_some()
-                    || dim == crate::ImageDimension::Cube)
-                    && depth_ref.is_some()
-                    && gather.is_none()
-                    && !self
-                        .options
-                        .writer_flags
-                        .contains(WriterFlags::TEXTURE_SHADOW_LOD);
+                // `textureLod[Offset]` on `sampler2DArrayShadow` and `samplerCubeShadow` does not exist in GLSL,
+                // unless `GL_EXT_texture_shadow_lod` is present.
+                // But if the target LOD is zero, we can emulate that by using `textureGrad[Offset]` with a constant gradient of 0.
+                let workaround_lod_with_grad = ((dim == crate::ImageDimension::Cube && !arrayed)
+                    || (dim == crate::ImageDimension::D2 && arrayed))
+                    && level == crate::SampleLevel::Zero
+                    && matches!(class, crate::ImageClass::Depth { .. })
+                    && !self.features.contains(Features::TEXTURE_SHADOW_LOD);
 
                 // Write the function to be used depending on the sample level
                 let fun_name = match level {
                     crate::SampleLevel::Zero if gather.is_some() => "textureGather",
+                    crate::SampleLevel::Zero if workaround_lod_with_grad => "textureGrad",
                     crate::SampleLevel::Auto | crate::SampleLevel::Bias(_) => "texture",
-                    crate::SampleLevel::Zero | crate::SampleLevel::Exact(_) => {
-                        if workaround_lod_array_shadow_as_grad {
-                            "textureGrad"
-                        } else {
-                            "textureLod"
-                        }
-                    }
+                    crate::SampleLevel::Zero | crate::SampleLevel::Exact(_) => "textureLod",
                     crate::SampleLevel::Gradient { .. } => "textureGrad",
                 };
                 let offset_name = match offset {
@@ -2727,7 +2715,7 @@ impl<'a, W: Write> Writer<'a, W> {
                     crate::SampleLevel::Auto => (),
                     // Zero needs level set to 0
                     crate::SampleLevel::Zero => {
-                        if workaround_lod_array_shadow_as_grad {
+                        if workaround_lod_with_grad {
                             let vec_dim = match dim {
                                 crate::ImageDimension::Cube => 3,
                                 _ => 2,
@@ -2739,14 +2727,9 @@ impl<'a, W: Write> Writer<'a, W> {
                     }
                     // Exact and bias require another argument
                     crate::SampleLevel::Exact(expr) => {
-                        if workaround_lod_array_shadow_as_grad {
-                            log::warn!("Unable to `textureLod` a shadow array, ignoring the LOD");
-                            write!(self.out, ", vec2(0,0), vec2(0,0)")?;
-                        } else {
                         write!(self.out, ", ")?;
                         self.write_expr(expr, ctx)?;
                     }
-                    }
                     crate::SampleLevel::Bias(_) => {
                         // This needs to be done after the offset writing
                     }

naga/src/valid/type.rs

@@ -107,6 +107,12 @@ pub enum TypeError {
     MatrixElementNotFloat,
     #[error("The constant {0:?} is specialized, and cannot be used as an array size")]
     UnsupportedSpecializedArrayLength(Handle<crate::Constant>),
+    #[error("{} of dimensionality {dim:?} and class {class:?} are not supported", if *.arrayed {"Arrayed images"} else {"Images"})]
+    UnsupportedImageType {
+        dim: crate::ImageDimension,
+        arrayed: bool,
+        class: crate::ImageClass,
+    },
     #[error("Array stride {stride} does not match the expected {expected}")]
     InvalidArrayStride { stride: u32, expected: u32 },
     #[error("Field '{0}' can't be dynamically-sized, has type {1:?}")]
@@ -596,8 +602,15 @@ impl super::Validator {
             Ti::Image {
                 dim,
                 arrayed,
-                class: _,
+                class,
             } => {
+                if arrayed && matches!(dim, crate::ImageDimension::D3) {
+                    return Err(TypeError::UnsupportedImageType {
+                        dim,
+                        arrayed,
+                        class,
+                    });
+                }
                 if arrayed && matches!(dim, crate::ImageDimension::Cube) {
                     self.require_type_capability(Capabilities::CUBE_ARRAY_TEXTURES)?;
                 }

naga/tests/in/sample-cube-array-depth-lod.param.ron

@@ -0,0 +1,11 @@
+(
+	glsl: (
+        writer_flags: ("TEXTURE_SHADOW_LOD"),
+        version: Embedded(
+			version: 320,
+			is_webgl: false
+		),
+		binding_map: {},
+		zero_initialize_workgroup_memory: true,
+	),
+)

naga/tests/in/sample-cube-array-depth-lod.wgsl

@@ -0,0 +1,12 @@
+// see https://github.com/gfx-rs/wgpu/issues/4455
+
+@group(0) @binding(0) var texture: texture_depth_cube_array;
+@group(0) @binding(1) var texture_sampler: sampler_comparison;
+
+@fragment
+fn main() -> @location(0) f32  {
+    let pos = vec3<f32>(0.0);
+    let array_index: i32 = 0;
+    let depth: f32 = 0.0;
+    return textureSampleCompareLevel(texture, texture_sampler, pos, array_index, depth);
+}

naga/tests/in/use-gl-ext-over-grad-workaround-if-instructed.param.ron

@@ -0,0 +1,11 @@
+(
+	glsl: (
+        writer_flags: ("TEXTURE_SHADOW_LOD"),
+        version: Embedded(
+			version: 320,
+			is_webgl: false
+		),
+		binding_map: {},
+		zero_initialize_workgroup_memory: true,
+	),
+)

naga/tests/in/use-gl-ext-over-grad-workaround-if-instructed.wgsl

@@ -0,0 +1,12 @@
+// see https://github.com/gfx-rs/wgpu/pull/5171
+
+@group(0) @binding(0) var texture: texture_depth_2d_array;
+@group(0) @binding(1) var texture_sampler: sampler_comparison;
+
+@fragment
+fn main() -> @location(0) f32  {
+    let pos = vec2<f32>(0.0);
+    let array_index: i32 = 0;
+    let depth: f32 = 0.0;
+    return textureSampleCompareLevel(texture, texture_sampler, pos, array_index, depth);
+}

naga/tests/out/glsl/sample-cube-array-depth-lod.main.Fragment.glsl

@@ -0,0 +1,18 @@
+#version 320 es
+#extension GL_EXT_texture_cube_map_array : require
+#extension GL_EXT_texture_shadow_lod : require
+
+precision highp float;
+precision highp int;
+
+uniform highp samplerCubeArrayShadow _group_0_binding_0_fs;
+
+layout(location = 0) out float _fs2p_location0;
+
+void main() {
+    vec3 pos = vec3(0.0);
+    float _e6 = textureLod(_group_0_binding_0_fs, vec4(pos, 0), 0.0, 0.0);
+    _fs2p_location0 = _e6;
+    return;
+}
+

naga/tests/out/glsl/use-gl-ext-over-grad-workaround-if-instructed.main.Fragment.glsl

@@ -0,0 +1,17 @@
+#version 320 es
+#extension GL_EXT_texture_shadow_lod : require
+
+precision highp float;
+precision highp int;
+
+uniform highp sampler2DArrayShadow _group_0_binding_0_fs;
+
+layout(location = 0) out float _fs2p_location0;
+
+void main() {
+    vec2 pos = vec2(0.0);
+    float _e6 = textureLod(_group_0_binding_0_fs, vec4(pos, 0, 0.0), 0.0);
+    _fs2p_location0 = _e6;
+    return;
+}
+

naga/tests/snapshots.rs

@@ -736,6 +736,11 @@ fn convert_wgsl() {
             Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::HLSL | Targets::WGSL,
         ),
         ("cubeArrayShadow", Targets::GLSL),
+        ("sample-cube-array-depth-lod", Targets::GLSL),
+        (
+            "use-gl-ext-over-grad-workaround-if-instructed",
+            Targets::GLSL,
+        ),
         (
             "math-functions",
             Targets::SPIRV | Targets::METAL | Targets::GLSL | Targets::HLSL | Targets::WGSL,