godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Shader Preprocessor Improvements #5062

Open bitsawer opened 2 years ago

bitsawer commented 2 years ago

Describe the project you are working on

Multiple, usually small ports from other sources (demos, tutorials, ShaderToy etc).

Describe the problem or limitation you are having in your project

Preprocessor doesn't always fully match the capabilities of other preprocessors or shader editors.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Now that https://github.com/godotengine/godot/pull/62513 has been merged, there have been several improvement suggestions during and after adding the shader preprocessor.

The issue template asks to make separate issue for each suggestion, but I feel like these would be good to discuss in one place, at least until we decide if any of these improvements are wanted or not as they can be pretty tightly coupled. I can create new issues for any accepted improvement if wanted. I'll tag some people in this suggestion that have been involved in making this all happen, but feel free to ignore this. And of course everyone else is free to chime in, too. Pressing the thumbs-up reaction button helps this proposal to gain support and visibility. @fire @MagdielM @TheOrangeDay @Chaosus @clayjohn @reduz

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

See above.

If this enhancement will not be used often, can it be worked around with a few lines of script?

Preprocessor is core functionality, so not easily.

Is there a reason why this should be core and not an add-on in the asset library?

It's adding features to existing core functionality.

Calinou commented 2 years ago

and used platform (WINDOWS, MACOS etc.) were automatically defined for all shaders.

I think the main distinctions we need for shaders are "desktop", "mobile" and "using MoltenVK". There are probably no shaders that work on Windows and not Linux and vice versa. This is not true for macOS and iOS due to the use of MoltenVK, which has several limitations native Vulkan doesn't have.

Shnazzy commented 2 years ago

I'd like to add to the list of suggested preprocessor features:

With #include implemented, it's not unreasonable to imagine two shaders that both reference the same shaderinclude file but with each using a different render mode or shader type. It would be helpful to be able to check through a preprocessor instruction what these settings are set to as preprocessor conditions. Example syntax:

shader1.gdshader:

shader_type spatial;
render_mode unshaded;
#include "shader_main.gdshaderinc"

shader2.gdshader:

shader_type spatial;
#include "shader_main.gdshaderinc"

shader_main.gdshaderinc:

#if rendering(unshaded)
<shader 1 would evaluate here with shader code that accounts for the shader having unshaded as a rendering mode>
#else
<shader 2 would evaluate here with shader code that accounts for the shader not having unshaded as a rendering mode>
#endif
bitsawer commented 2 years ago

@Shnazzy Unless I understood you wrong, similar functionality is already possible (although not automatically). I would handle that scenario something like this:

#define UNSHADED

shader_type spatial;
render_mode
#ifdef UNSHADED
    unshaded,
#endif
    cull_disabled;

#include "shader_main.gdshaderinc"

And in shader_main:

#ifdef UNSHADED
    //Handle unshaded variant
#else
    //Handle shaded variant
#endif

You can define things before including files and those defines will be available in those included files (this works with multiple shaders that all use the same included file, too).

Implementing this in a way you suggest would be a lot trickier, as the C/GLSL/Godot family preprocessors are basically just dumb and simple text processors which don't really understand the shader syntax or settings. Adding shader syntax parsing would make the preprocessor more complex. Especially in this case where the workaround is pretty easy to do.

Shnazzy commented 2 years ago

@bitsawer If I understand the shader stack it would go something like so (Let me know if I missed a step):

  1. Godot Shader Preprocessor
  2. Godot Shader Compiler
  3. GLSL Preprocessor
  4. GLSL Compiler/Interpreter

My original idea was to just read from the list of render modes in the Godot Shader Preprocessor, but now I realize that's not possible since getting that list is done in step 2. However, it seems that in step 2, the list of render modes is converted to a list of GLSL preprocessor #define instructions that will be further addressed in step 3. For example, the following line would be the one for the unshaded render mode:

actions.render_mode_defines["unshaded"] = "#define MODE_UNSHADED\n";

What I'm thinking is that it should be possible to, in step 1, evaluate something like:

#ifrender unshaded
 // Some conditional shader code
#endif

into:

#if defined(MODE_UNSHADED)
  // Some conditional shader code
#endif

The second block wouldn't be for the Godot Shader preprocessor to evaluate, but the GLSL shader (step 3). Does something like this sound more feasible or in the right direction?

My motive here was because I was trying to recreate in a Godot shaderinclude the default light() function that is automatically used when one isn't defined in a spatial shader for experimentation. This is a location where many GLSL preprocessor checks on the rendering mode happen, hence why I put in the request, because these checks have no equivalent if done outside of GLSL.

TokisanGames commented 1 year ago

Being able to change custom defines via ShaderMaterial is needed. I'm building a terrain editor and using includes to separate the terrain portion and editor overlays portion of the shader. As it is I will need to manually remove #define EDITOR to strip out the editor shader branches on export. Instead it should be settable in code and automatically remove itself when running in game.