KhronosGroup / SPIRV-Cross

SPIRV-Cross is a practical tool and library for performing reflection on SPIR-V and disassembling SPIR-V back to high level languages.
Apache License 2.0
1.97k stars 549 forks source link

GLSL: Add option to separate combined image samplers when emitting GLSL #2236

Open chyyran opened 7 months ago

chyyran commented 7 months ago

My use case involves converting a ton of "legacy" GLSL shaders into GLSL that naga can consume and transpile to WGSL. Since there is already logic to split combined image samplers to a sampler and texture in the HLSL and MSL backends, would it be possible to add an optional flag to the GLSL backend as well to enable this, for example converting a sampler2D to a split texture2D and sampler?

HansKristian-Work commented 7 months ago

Why can't Naga do this? Splitting image and samplers gets very awkward since they have to be combined again, and HLSL and MSL don't have that issue. I'd rather not add yet another hack like this to SPIRV-Cross when it can be solved on the outside through SPIR-V transformations.

chyyran commented 7 months ago

There is an issue in naga for supporting combined image samplers but it's been open for two years and I don't believe that they would like to support the necessary IR changes regardless since combined image samplers are unrepresentable in WGSL.

Forgive my misunderstanding here but I'm not entirely sure what the difference is between HLSL and splitting the samplers in the output GLSL; for a basic shader like

#version 450 

layout(location = 0) out vec4 color;
layout(binding = 0) uniform sampler2D tex;

void main() {
    color = texture(tex, vec2(0.0));
}

the HLSL/MSL backends already split the samplers up,

Texture2D<float4> tex : register(t0);
SamplerState _tex_sampler : register(s0);
// omitted ...
color = tex.Sample(_tex_sampler, 0.0f.xx);

would it be more awkward than outputting an equivalent

layout(set = 0, binding = 0) uniform texture2D tex;
layout(set = 1, binding = 0) uniform sampler _tex_sampler;
// omitted ...
color = texture(sampler2D(tex, _tex_sampler), vec2(0.0));

Besides the Naga issue, I think a transform to this GLSL dialect is still useful outside of anything WGSL related.

chyyran commented 7 months ago

Also you mention SPIR-V transformations; if not in SPIRV-Cross would SPIRV-Tools have the existing tools to do a transform like this?

HansKristian-Work commented 7 months ago

layout(set = 1, binding = 0) uniform sampler _tex_sampler;

Here, you're inventing a new binding out of thin air, and that means we now have to add a bunch of infra to support how to remap those types. I really don't want to do that just to deal with this feature.

HansKristian-Work commented 7 months ago

would SPIRV-Tools have the existing tools to do a transform like this?

This sounds like the thing that SPIRV-Tools would have, if normal SPIR-V cannot be translated to WGSL as-is. I'm still unconvinced this isn't Naga's problem. If they accept GLSL as input, they have to consider GLSL shaders people actually write, which includes legacy-style combined image samplers.