[glsl-in] Consider supporting combined image/samplers

[magcius]

My use case for Naga is converting from GLSL ES to WGSL at runtime. GLSL ES only supports combined image/samplers, and it would be quite annoying for me to change my code to do something else. Many typical approaches to this problem are limited by the restrictions of GLSL.

For instance, my current policy is to have texture bindings and samplers take consecutive bindings.

I would be fine if the GLSL is parsed into an IR that can't be output by the WGSL backend, and I would have to convert it to the proper policy myself.

[JCapucho]

Can you post a small piece of code of what you mean?

[magcius]

We talked about this a bit in Matrix here: https://matrix.to/#/!fjjkgHFcwtkREywzfk:matrix.org/$K2DR_lrip11nvTM7r3rnk-dPRe5C56SjJ1RSTusnTv0?via=matrix.org&via=mozilla.org&via=sorunome.de

The idea is that I could write code like:

uniform sampler2D u_Texture;

And then be able to turn that into, e.g.:

[[group(0), binding(0)]] var T_u_Texture : texture_2d<f32>;
[[group(0), binding(1)]] var S_u_Texture : sampler;

[JCapucho]

Hmm how would that interact with vulkans combined image samplers? https://vulkan-tutorial.com/Texture_mapping/Combined_image_sampler

[magcius]

Similarly, or the same, actually. The issue is more that WebGPU/WGSL, and also D3D12/HLSL both don't support combined image samplers, so you need to figure out what to do with it on output.

[kvark]

Adding full support for combined stuff into IR is a biiig issue. I'm not excited about this, and I hope we'll not need it any time soon. In particular because WebGPU doesn't have combined image-samplers at all. So perhaps this can be helped on glsl-in side?

[magcius]

I don't need full support, I'm happy to write the translation code. I think it's going to be a big sticking point for those migrating from WebGL, and those interested in having engines that support both WebGL and WebGPU.

[glfmn]

While the kinks of getting combined image/samplers working, the error message that naga provides when using an unsupported combined sampler could be improved. Currently, it simply says:

error: Not implemented: variable qualifier
  ┌─ glsl:7:29
  │
7 │ layout(binding = 1) uniform sampler2D image;
  │                             ^^^^^^^^^

However, if the user looks up any documentation or tutorials online for GLSL, they will see countless examples using sampler2D as above, even for vulkan applications. Some sort of additional note or different text to indicate somehow that the proper form is:

layout(set = 1, binding = 0) uniform texture2D u_texture;
layout(set = 1, binding = 1) uniform sampler u_sampler;

and to use the function sampler2D(u_texture, u_sampler) to make the sampler would be much more helpful. At the very least, something like Not implemented: combined image samplers are unsupported.

[chyyran]

I encountered this issue and filed #2100 in error because of the confusing error message, but let me give my 2 cents while I'm here.

My use case is implementing RetroArch's shader pipeline as a dynamic library in Rust, which involves compiling shader files that I don't have control over here: https://github.com/libretro/slang-shaders.

Many of these use the uniform sampler2D u_Texture; syntax. I had originally thought that I could work around this by round-tripping from the shaderc crate to obtain SPIR-V bytecode, then parsing with spv-in, but this gives me an InvalidId error when parsing the SPIR-V. Even if there was a transformation in glsl-in, it doesn't seem like it would match up with whatever SPIR-V glslang output when dealing with combined image samplers.

For example, this shader that does consecutive bindings for combined image samplers

layout(set = 0, binding = 2) uniform sampler2D Source;
layout(set = 0, binding = 3) uniform sampler2D OriginalHistory1;
layout(set = 0, binding = 4) uniform sampler2D OriginalHistory2;
layout(set = 0, binding = 5) uniform sampler2D OriginalHistory3;
layout(set = 0, binding = 6) uniform sampler2D OriginalHistory4;
layout(set = 0, binding = 7) uniform sampler2D OriginalHistory5;
layout(set = 0, binding = 8) uniform sampler2D OriginalHistory6;
layout(set = 0, binding = 9) uniform sampler2D OriginalHistory7;

A source transformation here would need to take into account mapping bindings between the images and the samplers

[chyyran]

I don't think this is a good way of going about it, but as a proof of concept I was able to get naga to accept SPIR-V (generated by SPIRV-Cross) that contains a combined image sampler without IR changes.

https://github.com/gfx-rs/naga/compare/master...chyyran:naga:feat-combined-image-samplers

The output of this however doesn't look very correct, see particularly the wgsl example here https://gist.github.com/chyyran/14e3e17fe5d8ed56a685eee71db32947

    let _e10: vec4<f32> = textureSample(Source, Source, _e9); // (???)