Godot's flavor of GLSL shaders shouldn't clash with standard GLSL, to allow better recognition by third-party software

[geekley]

Godot currently recognizes *.glsl files, since it uses GLSL (e.g. for compute shaders).

However, the syntax used is not standard GLSL. It actually uses merged stages, tagged under the Godot-specific tokens:
#[vertex], #[fragment], #[tesselation_control] (sic), #[tesselation_evaluation] (sic), #[compute]
Note that "tesselation" (with a single L) is used, which seems to be a typo. Double L doesn't work.

If you pass one such file to e.g. glslang -E it will complain about the syntax (invalid directive). This complicates compatibility with external tools such as IDE plugins. In VSCode, for example, there's a plugin that supports GLSL files, but it won't compile with this token (since it uses the official glslang tool for validation).

It seems you can currently workaround this by using #include in the joint GLSL and get tool support on included files.
But there's a few more alternatives to improve compatibility with external tools, and avoid clashing the format.

Option 1: Change Godot GLSL extension to something else, e.g. *.godot-glsl or *.joint-glsl

The advantage of this option is that you can use the same syntax. Something similar occurred in the past with #2488 where *.shader was changed to *.gdshader which is more explicit about what the format means, and helps playing nice with external tools. (Alternative possible name might be *.gdglsl, if it's not too confusing.)

You would still have both options of keeping all stages on a *.godot-glsl or having just the special tokens in it and #include each stage from a separate file with external tool support as normal.

Then external tools will be able to either ignore the unsupported Godot-GLSL format, or properly support it.

Option 2: Change the syntax to be conformant to standard GLSL

If you're going to call this file a *.glsl, then I think it should use syntax that better matches the standard.
One option would be to use #if directives instead to specify the compilation stage.

Just for example's sake, here's some alternative ways this could be implemented:

#ifdef STAGE_vertex
// vertex code
#elif defined(STAGE_fragment)
// fragment code
#endif

#if GODOT_STAGE == GODOT_STAGE_COMPUTE
// compute code
#endif

Internally, Godot would just compile the file multiple times, varying the implicit defined macro for each stage.

Then, on external tools that allow specifying macro settings per-file/glob/project, e.g. via command-line arguments to the glslang validator like --define-macro STAGE_compute=1, you may be able to configure it at least to support Godot compute shaders via some *.comp.glsl pattern or project-specific IDE settings.

Additional: Recognize extensions like *.comp.glsl too without requiring the special #[compute] token

Besides one of the previous options, you could make Godot editor recognize this extension (maybe also *.comp?) making the token optional in this case. The advantage is using external files from the web here without any sort of editing.

It also makes sense because you cannot mix #[compute] with other stages in the same file anyway.

[Calinou]

I like the idea of using #ifdef STAGE_VERTEX for this (I'd make it all-caps for consistency with other defines). It seems like the simplest solution to me.

I would not go the .godot-glsl route, as it's already hard enough for text editors to get good GLSL extensions (a lot of them are rather primitive or have significant bugs). Getting good Godot GLSL extensions will be even more difficult.

Note that in any case, we should keep support for the existing syntax as it's possible to use it in projects now that the rendering compositor was merged. The same applies to the tesselation_control and tesselation_evaluation typos, the old syntax needs to remain supported even if it prints a warning when it's used.

[geekley]

The only reason I showed an example without all-caps is because I don't know if it's desired to avoid clashing with user defines (and I hate using the ugly leading/double-underscore for this sort of thing). Likely this isn't even an issue (I'm probably just being paranoid 😆 ), so all-caps should indeed be preferred.

I agree with keeping compatibility even in the typo case (reminds me of the HTTP "Referer" header, a misspelling of "Referrer" that was kept in the spec).

Absolutely agree that we should go the simplest route of #ifdef too.
It would even allow having code outside of branching, such as includes or whatever definitions to be used by multiple stages. You could also split code for a stage in multiple parts in the file, as well as having a single place for definitions that must be in some stages but not others. I assume this sort of flexibility can compensate the extra verbosity and extra #endif line.

[danderson]

I found this discussion after filing (then closing) godotengine/godot#99840, because I hit the same problem. In the absence of builtin support for "godot style GLSL", writing compute shaders is currently a bit painful in external editors. In particular, even for compute shaders with a single section, the GLSL spec says that #version XYZ must be the first non-comment in the file, so it's currently not possible to write a single file that is compatible with Godot and standard GLSL tools.

My ideas were all worse than this, so +1 to using ifdefs!

I'm willing to write a formal proposal for this, and send the C++ patches to implement it.

Requirements:

• Existing file format must keep working with zero changes, including tesselation typos. Fortunately this is easy, it's possible to identify the 2 formats using only the first non-comment line.
• New format must be standard GLSL, so that standard-compliant GLSL tools can process them. It's OK to require extra configuration for external tools (e.g. user defines some symbols to select a shader stage).
• Shader sections are separated by #ifdef values, e.g. STAGE_VERTEX, STAGE_COMPUTE, STAGE_TESSELLATION_CONTROL. For each stage, Godot gives the entire source to the GLSL compiler, with the correct extra #defines.
• Document all of this in the manual, so that users and external tool developers know how to connect Godot shaders and GLSL tools.

Two open questions: how do we make it efficient for Godot to identify which shader stages exist in a file? And, how do we handle the #[versions] section?

My proposal for both: use #pragma statements. These are the standard way to provide compiler-specific information, and explicitly supported in the GLSL standard. For example:

#version 450

#pragma gd_shader_stages(compute) // or gd_shader_stages(vertex, fragment) for example
#pragma gd_shader_versions(standard, dithered)

#ifdef GD_STAGE_COMPUTE
...

#ifdef GD_VERSION_DITHERED
...

The above is equivalent to this in current format:

#[versions]

standard = "";
dithered = "#define GD_VERSION_DITHERED";

#[compute]
#version 450

...

#ifdef GD_VERSION_DITHERED
...

This makes the section parser trivial: scan the start of the file for gd_* pragmas, and they give Godot all the information needed for which shaders and variations it needs to compile. The GLSL compiler does the rest of the work.

Third-party tools with explicit Godot support can also parse these #pragmas and give a great experience. Tools that don't support Godot explicitly can still be used, by manually defining the desired GD_STAGE_* and GD_VERSION_* symbols just like geekley said. It's just standard GLSL :)

We can make the parser simpler by saying that the Godot pragmas must appear after the #version (GLSL says this must be first), but before any other code. This is not a big constraint for users, but it greatly simplifies the parser because we don't need full C preprocessor support, which is a surprisingly complex thing to implement!

For example, with this rule, this compute shader is valid:

#version 450

#pragma gd_shader_stages(compute)

layout(local_size_x = 2, local_size_y = 1, local_size_z = 1) in;

void main() {}

But this shader is not:

#version 450

layout(local_size_x = 2, local_size_y = 1, local_size_z = 1) in;

#pragma gd_shader_stages(compute) // ERROR: Godot pragma not at the top of the file

void main() {}

What do you think?

[danderson]

I wanted to see how hard it would be to implement this, and, uh, I accidentally implemented it 😅 My local Godot build supports the above syntax and seems to be happy with "new-style" GLSL shaders. The implementation can process both the current format and the proposed new one, so all existing shader source keeps working. The syntax I implemented with some explanations:

// GLSL spec says the version must come first, only comments and
// empty lines allowed before
#version 450

// Tell Godot what shader stages this file contains.
// Same as the existing format, compute shaders cannot be mixed
// with others, but vertex/fragment/tessellation shaders can be mixed.
#pragma gd_shader_stages(vertex, fragment)

// Optionally, complex shaders can declare multiple variants.
// If you don't specify this, you get one compiled shader per stage.
#pragma gd_shader_versions(lines, triangles)

// Now the shader code! Let's start with some stuff common to all stages, maybe an include and a shared parameter
#include "common_defs.glsl"

layout(set=0, binding=0) uniform Parameters {
    vec3 world_size;
    int grid_size;
} params;

// Vertex shader. Each stage turns into a GD_SHADER_<stagename> preprocessor symbol
#ifdef GD_STAGE_VERTEX
layout(set=0, binding=1) restrict buffer Positions {
    vec3 data[];
} positions;

void main() {
    // Pretend these helpers are in the #include, this is just a nonsense demo shader...
    do_setup_common();

    // Similar to stages, each version becomes a GD_VERSION_<name> symbol
#ifdef GD_VERSION_LINES
    process_into_lines();
#endif
#ifdef GD_VERSION_TRIANGLES
    process_into_triangles();
#endif
}
#endif // GD_STAGE_VERTEX

// And then the fragment shader.
#ifdef GD_STAGE_FRAGMENT
void main() {
    do_setup_common();
    adjust_frag();
}
#endif // GD_STAGE_FRAGMENT

One nice thing about this format: if you just need one shader stage and no variants, you don't need to use ifdefs at all. Then the shader looks like a completely generic GLSL file with one extra pragma to tell Godot the shader type:

#version 450

#pragma gd_shader_stages(compute)

void main() {
    // blah blah compute shader
}

For a more complex example, I grabbed a random shader from Godot's source tree, modules/lightmapper_rd/lm_blendseams.glsl, and ported it to the new format. Godot happily compiles the shader in either format. The source is quite long so I stashed it in https://gist.github.com/danderson/f5b518b66c1b7670016b3a817cbefbd5 to not spam this post. I also included debug logs from my editor that shows the "raw" shader source that gets compiled for each shader stage/version, it's pretty much the original file verbatim except the Godot pragmas get replaced by the right #defines for each compile.

Code still needs cleaning up and tests, but I guess it's maybe time to write a formal proposal 😅

[geekley]

That's awesome! Questions:

1. Are the pragma values are case insensitive? Are they simply upper-cased when becoming macros? So, could someone do #pragma gd_shader_versions(LINES, TRIANGLES)?
2. Is the order significant in both pragmas? Is only 1 of each allowed? Can it only have 1 stage pragma before 1 optional version pragma? Do duplicate pragmas raise an error or are they merged? I assume it's strict, for efficiency, right?
3. Would it be possible to make it so the stage pragma is optional (implied to be compute) in *.comp.glsl and/or *.comp files? So e.g. files from the Internet can just be used verbatim with no changes at all?
4. Is there already a standard in using GD_ and/or gd_ prefix in GLSL macros or identifiers, instead of GODOT_ or whatever?
   IMHO it might be prudent to use #pragma godot_stages and #pragma godot_versions to avoid confusion with the GDShader language (or whatever it's going to be called).

[danderson]

Before I get into details, just a note that obviously all this is still changeable, the answers below are just whatever I implemented based on this thread and my gut feelings :)

1. The pragma values are case insensitive, so you could say LINES, TRIANGLES if you like, yes. The pragma name and keyword must be lowercase, however. (and, you made me realize a slight bug in my parser, currently you could say #pragma gd_shader_versions(lines, LINES, LiNeS) and it won't complain about duplicates. Oops, will fix).
2. Order is not significant, you can put the pragmas and their values in any order and get the same result. Duplicate pragmas are not allowed, neither are duplicate values. To be parsed by Godot, a top-level shader file must have exactly one gd_shader_stages pragma, and 0 or 1 gd_shader_versions pragma. Not a strong opinion, I just figured it makes it easier to understand a shader if the pragmas are near the top of the file, and don't have complex merging semantics that humans have to keep track of. It also matches the "no duplicates allowed" semantics of the #[blah] shader format.
3. I haven't looked at filename hints yet. It seems feasible, but the code I've been hacking on receives just the file contents, not the file name. So I need to explore the code a bit more and figure out how to do the plumbing.
4. I don't think there's a standard (but I'm not a Godot dev so 🤷). I picked GD_ for two reasons: I wanted to have some kind of namespace so that if you drop GLSL from the internet into a project there's low risk of conflict; and GD_ is shorter to type than GODOT_ 😂 . But I don't have a strong opinion, if the semantics are acceptable I will put in whatever names people prefer :)

[danderson]

I got filename->stage mapping working. Boring demo video: https://www.youtube.com/watch?v=23HlxIXXSzQ and the (not polished yet, pls don't judge) source code is at https://github.com/danderson/godot/tree/push-xvurqvqkplyr

Magic filenames only work for the new file format for now. To start, I picked very explicit extensions: .vertex.glsl, .fragment.glsl, .tessellation_control.glsl, .tessellation_evaluation.glsl and .compute.glsl. I wanted to keep .glsl at the end for compatibility with other tools, and so that future support of HLSL/WGSL/SPIR-V doesn't need hacks like guessing the format from the source code. Like all the other names, we can change them to match group consensus :)

Currently, if you specify a stage in the filename, you cannot also set #pragma gd_shader_stage(...) in the file. Conceptually, the pragma is a "duplicate" of the implicit filename pragma, and duplicates aren't allowed. You can still set gd_shader_versions to get multiple variants.

[Calinou]

We use GODOT_ as a prefix for Godot's defined C# macros, so I think GLSL should keep doing the same.

[danderson]

Ah yeah, consistency with C# is a good reason. I'll update my prototype and the proposal to match.

I think the same reasoning applies to the pragma names: godot_shader_stages(...) instead of gd_shader_stages(...). wdyt?

[danderson]

I filed a proposal at #11274 for this, since it's been a month and there is no fundamental disagreement, even if the details might change. There is nothing new in the proposal compared to this discussion, just more details on the prototype implementation and answering the proposal template questions.