Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
chyyran opened this issue Nov 30, 2023 · 5 comments
Open

Comments

@chyyran
Copy link
Contributor

chyyran commented Nov 30, 2023

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?

@chyyran chyyran changed the title Add option to separate combined image samplers when emitting GLSL GLSL: Add option to separate combined image samplers when emitting GLSL Nov 30, 2023
@HansKristian-Work
Copy link
Contributor

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
Copy link
Contributor Author

chyyran commented Nov 30, 2023

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
Copy link
Contributor Author

chyyran commented Nov 30, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants