[GL] Panic when a uniform block is not accessed in a shader stage.

[agausmann]

Description

I have a WGSL shader that accepts a uniform buffer binding, which is used by the vertex stage (vertex transformation matrix), and is unused by the fragment stage.
On the Vullkan backend, this runs without errors. On the GL backend, the shader compiler panics.

It will panic regardless of whether the binding's visibility flags include the fragment stage.

It will not panic if the uniform buffer binding is made visible to the fragment stage AND the binding is accessed in the fragment shader code (even if the value is ultimately not used to compute any output values).

Repro steps

• MCVE at https://github.com/agausmann/wgpu-gl-unused-uniform-repro
• cargo r -> panics
• Uncomment this line, so the fragment shader accesses the uniform, and cargo r -> does not panic

Extra materials

Full backtrace:

thread 'main' panicked at 'no entry found for key', /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu-hal/src/gles/device.rs:38:24
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:515:5
   1: core::panicking::panic_fmt
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/panicking.rs:92:14
   2: core::option::expect_failed
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/option.rs:1243:5
   3: core::option::Option<T>::expect
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/option.rs:351:21
   4: <std::collections::hash::map::HashMap<K,V,S> as core::ops::index::Index<&Q>>::index
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/collections/hash/map.rs:1148:9
   5: wgpu_hal::gles::device::CompilationContext::consume_reflection
             at /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu-hal/src/gles/device.rs:38:24
   6: wgpu_hal::gles::device::<impl wgpu_hal::gles::Device>::create_shader
             at /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu-hal/src/gles/device.rs:154:9
   7: wgpu_hal::gles::device::<impl wgpu_hal::gles::Device>::create_pipeline
             at /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu-hal/src/gles/device.rs:190:26
   8: wgpu_hal::gles::device::<impl wgpu_hal::Device<wgpu_hal::gles::Api> for wgpu_hal::gles::Device>::create_render_pipeline
             at /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu-hal/src/gles/device.rs:872:21
   9: wgpu_core::device::Device<A>::create_render_pipeline
             at /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu-core/src/device/mod.rs:2299:22
  10: wgpu_core::device::<impl wgpu_core::hub::Global<G>>::device_create_render_pipeline
             at /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu-core/src/device/mod.rs:4058:34
  11: <wgpu::backend::direct::Context as wgpu::Context>::device_create_render_pipeline
             at /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu/src/backend/direct.rs:1229:27
  12: wgpu::Device::create_render_pipeline
             at /home/adam/.cargo/git/checkouts/wgpu-53e70f8674b08dd4/a222e6e/wgpu/src/lib.rs:1709:17
  13: wgpu_gl_unused_uniform_repro::main
             at ./src/main.rs:37:28
  14: core::ops::function::FnOnce::call_once
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/ops/function.rs:227:5

Platform

• OS: Linux/X11, and also when targeting WebGL on the browser with Implement WebGL2 Backend #1686
• wgpu version: latest commit on master (a222e6e)

[agausmann]

I did some tracing - here's some additional context from when the panic happens:

(Panic location in broader context)

• The GL backend is in the process of reflecting upon the emitted GLSL.
• It is enumerating through the WGSL shader module's list of global variables.
• It is trying to get the name of the uniform block's global variable.
• The ReflectionInfo's uniform list does not contain any reference to that global, and so it panics.

If I replace the panicking line with an explicit check that ignores the global variable if it isn't present in the ReflectionInfo, then the panic goes away and the program runs without error.

I also tested this patch against the original program where I first encountered this panic. Unlike the MCVE, it fully executes the render pipeline. In that case, there were still no problems.

diff --git a/wgpu-hal/src/gles/device.rs b/wgpu-hal/src/gles/device.rs
index ec858bf0..6a503a0f 100644
--- a/wgpu-hal/src/gles/device.rs
+++ b/wgpu-hal/src/gles/device.rs
@@ -35,7 +35,10 @@ impl CompilationContext<'_> {
             let br = var.binding.as_ref().unwrap();
             let slot = self.layout.get_slot(br);

-            let name = reflection_info.uniforms[&handle].clone();
+            let name = match reflection_info.uniforms.get(&handle) {
+                Some(name) => name.clone(),
+                None => continue,
+            };
             log::debug!(
                 "Rebind buffer: {:?} -> {}, register={:?}, slot={}",
                 var.name.as_ref(),

However, I'm not 100% sure this is the correct solution. If it is panicking because there is no reference to the uniform in the emitted GLSL, then why don't the official examples also panic when being run on the GL backend? For example, the cube example is another case where the uniform block is only used in the vertex stage, but in that case, there is no panic when compiling the fragment stage, it still runs just fine on the GL backend.

[kvark]

Thank you for filing and investigation!
That seems like a right fix to me. Could you make this a PR?