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

Add FOR macro to GLSL preprocessor and J3MD #1758

Merged
merged 3 commits into from
Feb 3, 2022

Conversation

riccardobl
Copy link
Member

This can be used in shaders that need to repeat some code where loops cannot be used (eg. uniforms declaration).
See for example

#ifdef DIFFUSEMAP
uniform sampler2D m_DiffuseMap;
#endif
#ifdef DIFFUSEMAP_1
uniform sampler2D m_DiffuseMap_1;
#endif
#ifdef DIFFUSEMAP_2
uniform sampler2D m_DiffuseMap_2;
#endif
#ifdef DIFFUSEMAP_3
uniform sampler2D m_DiffuseMap_3;
#endif
#ifdef DIFFUSEMAP_4
uniform sampler2D m_DiffuseMap_4;
#endif
#ifdef DIFFUSEMAP_5
uniform sampler2D m_DiffuseMap_5;
#endif
#ifdef DIFFUSEMAP_6
uniform sampler2D m_DiffuseMap_6;
#endif
#ifdef DIFFUSEMAP_7
uniform sampler2D m_DiffuseMap_7;
#endif
#ifdef DIFFUSEMAP_8
uniform sampler2D m_DiffuseMap_8;
#endif
#ifdef DIFFUSEMAP_9
uniform sampler2D m_DiffuseMap_9;
#endif
#ifdef DIFFUSEMAP_10
uniform sampler2D m_DiffuseMap_10;
#endif
#ifdef DIFFUSEMAP_11
uniform sampler2D m_DiffuseMap_11;
#endif

This code can be rewritten like so

#for i=0..12 (#ifdef DIFFUSEMAP_$i $0 #endif)
  uniform sampler2D m_DiffuseMap_$1;
#endfor

Note: the first diffuse map (m_DiffuseMap) will be renamed in m_DiffuseMap0

As you can see, it makes code much more readable and maintainable.

@pspeed42
Copy link
Contributor

Does this come up often?

My only hesitation is that this is not compatible with "real" GLSL code. Similar to our "#import" hack... but that has unarguably been a huge boon to the engine.

For reference, even the C/C++ preprocessor does not have anything like this which is telling in that in 40 years or whatever of C development, they decided cutting and pasting code like this was the better alternative.

It's cool code, though. I also wonder if shader nodes already had an answer for something like this (I doubt it).

@riccardobl
Copy link
Member Author

riccardobl commented Jan 20, 2022

It comes up often especially if you are trying to build generic shaders.
It is true that it's weird and non standard, but a lot of engines have their own shading language, i don't think it is a bad idea to add non standard preprocessing directives if they can make things easier.

Another example, here, applying a filter to up to 6 textures at the same time + depth

#for i=0..6 ( #ifdef SCENE_$i $0 #endif )
    uniform sampler2D Input$i;
    #ifdef MRT 
        layout(location=$i) 
    #endif 
        out vec4 outScene$i;
#endfor 

#ifdef DEPTH
    uniform sampler2D InputDepth;
#endif

void main(){
    #for i=0..6 ( #ifdef SCENE_$i $0 #endif )
        outScene$i = ...;
    #endfor 

    #ifdef DEPTH
        gl_FragDepth = ...;
    #endif
}

EDIT: this is used also in the j3mds to declared the inputs for the shader

MaterialDef Effect {
    MaterialParameters {
        #for i=0..6 ( $0 )
            Texture2D Scene$i
        #endfor 

         VertexShader GLSL150:  ...
        FragmentShader GLSL150: ...

        Defines {
            #for i=0..6 ( $0 )
                SCENE_$i:Scene$i
            #endfor 
        }
    }
} 

@riccardobl riccardobl changed the title Add FOR macro to GLSL preprocessor Add FOR macro to GLSL preprocessor and J3MD Jan 20, 2022
@yaRnMcDonuts
Copy link
Contributor

This would've definitely been a helpful feature when I was making pbr terrain shaders, I recall trying to figure out if I could define textures in a for loop like this before realizing it was not possible at the time.

If it doesn't break any existent shaders and if users can still code their shaders the normal way, then I'd say it looks like a useful feature to have. And since this PR isn't actually changing any existent shaders to use the feature yet, it seems like the risk of this breaking things is pretty low.

I guess the only question is whether or not there's any significant downside to this like Paul mentioned. It doesn't seem like there would be, but I don't have enough experience to speculate that much, so I suppose the best way to know would be to write 2 versions of the same shader, one with the for loop macro and one the standard way, then compare to make sure there's no significant performance hits or other unexpected issues.

@grizeldi
Copy link
Member

grizeldi commented Feb 2, 2022

Sounds like an useful feature to have, it would've definitely come handy in my shaders a couple times by now.

@tlf30
Copy link
Contributor

tlf30 commented Feb 3, 2022

That is a very cool idea. While I have never had the need for it, it looks like it could be useful.

My only hesitation is that I have never seen anything like it before in any macro based C/C++ precompiler.

On the other hand, most glsl precompilers have custom macros. For example the engine I am currently working in has
#injdef which tells the preprocessor where the engine should inject predefined defines. It also has #alias to allow for a variable (such as a uniform, sampler, or push constant) name to be referenced by an aliased name from the engine. So I would say it would not be unexpected for JME to have custom macros as it is common in other engines.

@riccardobl
Copy link
Member Author

I think this is mergeable

@stephengold stephengold added this to the Future Release milestone Feb 3, 2022
@stephengold stephengold merged commit befa930 into jMonkeyEngine:master Feb 3, 2022
@stephengold
Copy link
Member

Thanks for your contribution, Riccardo!

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

Successfully merging this pull request may close these issues.

7 participants