Page 1 of 2

glsl preprocessor

Posted: Fri Jul 02, 2010 2:26 am
by cyrfer
I'm just wondering if I should be able to define a constant with the preprocessor feature in .program files, like this:

Code: Select all

fragment_program phong_fp glsl
{
    source phong.frag
    preprocessor_defines NUM_LIGHTS=2
}

Code: Select all

// shader implementation
#ifndef NUM_LIGHTS
#define NUM_LIGHTS 1
#endif // NUM_LIGHTS
The compiled shader always ends up with NUM_LIGHTS = 1. Why?

Re: glsl preprocessor

Posted: Fri Jul 02, 2010 2:30 am
by cyrfer
Removing the use of #ifndef in the implementation allows the correct constant value to get compiled. I guess glsl does not support "ifndef".

Re: glsl preprocessor

Posted: Fri Jul 02, 2010 2:35 am
by masterfalcon
It does support #ifndef. But as far as I can tell, the GLSL Preprocessor in Ogre skips over and removes them prior to the code being compiled, which would explain why your statement doesn't have any effect.

Re: glsl preprocessor

Posted: Sat Jul 03, 2010 12:08 am
by cyrfer
That's good to know. Too bad about it. Is this a "feature"?

Re: glsl preprocessor

Posted: Wed Jul 07, 2010 2:41 am
by cyrfer
Is it possible to disable this feature? The shader is not valid without the preprocessor definition, unless it being compiled by OGRE. So, I have to run my scene through OGRE now just to see if a shader compiles, rather than just verify it with a compiler tool, which slows me down quite a bit.

Re: glsl preprocessor

Posted: Wed Jul 07, 2010 4:02 am
by masterfalcon
I don't believe there's any way to disable it. I'm not sure why they are ignored anyways but the preprocessor code has been around since before my time.

Re: glsl preprocessor

Posted: Wed Jul 07, 2010 6:47 pm
by masterfalcon
Just a quick followup, is #ifndef the only preprocessor directive that is not working properly for you?

Re: glsl preprocessor

Posted: Wed Jul 07, 2010 8:20 pm
by cyrfer
So far that is the only one I've found that has a problem.

Re: glsl preprocessor

Posted: Wed Jul 07, 2010 8:23 pm
by masterfalcon
Ok, I'll take a look tonight. It may be a bug that's never shown up before. But it's certainly not a "feature".

Re: glsl preprocessor

Posted: Wed Jul 07, 2010 9:28 pm
by cyrfer
Your help is very much appreciated. Could this be a change slated for another 1.7 release?

Re: glsl preprocessor

Posted: Wed Jul 07, 2010 9:57 pm
by masterfalcon
Depends on what the problem is, we'll see though. My gut tells me that it's something relatively simple that just affects #ifndef's. I'll have a better idea when I have some time later tonight.

Re: glsl preprocessor

Posted: Thu Jul 08, 2010 3:53 am
by masterfalcon
Alright, so here's what's going on:

Let's say you define MYCOLOR in a material script:

Code: Select all

preprocessor_defines MYCOLOR=2
Then in your shader you do something like this:

Code: Select all

uniform vec4 ambient;

/*
  Basic ambient lighting vertex program
*/
#ifndef MYCOLOR
#define MYCOLOR 0
#endif

void main()
{
	gl_Position = ftransform();
	gl_TexCoord[0] = gl_MultiTexCoord0;
	gl_FrontColor = ambient;
	#if MYCOLOR==0
	gl_FrontColor = vec4(0.0, 0.0, 0.0, ambient.a);
	#endif

	#if MYCOLOR==1
	gl_FrontColor = vec4(1.0, 0.0, 0.0, ambient.a);
	#endif

	#if MYCOLOR==2
	gl_FrontColor = vec4(0.0, 1.0, 0.0, ambient.a);
	#endif

	#if MYCOLOR==3
	gl_FrontColor = vec4(0.0, 0.0, 1.0, ambient.a);
	#endif
}
Even though it is already defined, MYCOLOR gets redefined with the value in the shader without a warning. The preprocessor really should skip over that whole ifndef->endif section if the value is not defined, so this appears to be a bug. I'll do a little more investigation, but I'm not that familiar with this code(which is quite complicated).

Also, it looks like someone else found this before, but it is a very obscure issue. http://www.ogre3d.org/forums/viewtopic. ... 37&start=0

Re: glsl preprocessor

Posted: Thu Jul 08, 2010 10:28 am
by Olemars
Out of curiosity I ran it through some debugging, and it seems like glsl preprocessor conditionals (not just #ifndef, but #ifdef and #if/#else too) are quite broken and have been for a long time. #define directives are always processed, even when inside a conditional blocks, and stored in the internal macro list of the preprocessor.

From what I can see, the conditionals do suppress output correctly so if you have code like

Code: Select all

#define DEF 1

int value = 0;
#ifndef DEF
   value = 1;
#endif
the value = 1 line will be ditched.

if you have code like this however:

Code: Select all

#define DEF 0
#ifdef NOTDEF
    #define DEF 1
#endif

#define DEF2 0
#ifndef DEF
    #define DEF2 1
#endif
int value1 = DEF; //value1 will be 1, should be 0
int value2 = DEF2;  //value2 will also be 1, should be 0
The results will be incorrect since the defines are processed even if the conditionals say they shouldn't. Basically, they're processed and then their lines are ditched, which is pretty senseless. From what I can gather from the ogreglslpreprocessor.cpp code, the fix would be something like not doing handleDirective in CPreprocessor::Parse if output is disabled. Code definitely not the most readable, fully agreed on that. There's even a few goto's in there.

Bit odd that this hasn't floated up more than once or twice over the years.

Re: glsl preprocessor

Posted: Sun Jul 11, 2010 11:23 pm
by Matheus Martino
Is not possible that the preprocessor definetion suport in the ogre preprocessor was in fact never added at all?

Re: glsl preprocessor

Posted: Mon Jul 12, 2010 8:53 am
by Olemars
No, the support for #define is fully in place, it's just broken when combined with #if's.

It's more of a pre-preprocessor though, as it just parses the source for certain specific directives (#define, #if, #if defined(), #else, #ifdef, #ifndef, #endif) and handles those. All other directives are just passed through to the real preprocessor/compiler. That's another thing that should be checked maybe, whether unhandled directives are passed through when inside conditional blocks that have evaluated to false.

EDIT: Also #undef, which isn't properly handled either.

Re: glsl preprocessor

Posted: Fri Aug 13, 2010 2:10 am
by cyrfer
What was the resolution on this? Was there an official bug logged? Can the fix be added to 1.7.1 instead of waiting for 1.8? Thanks.

Re: glsl preprocessor

Posted: Wed Oct 20, 2010 11:36 am
by racoon
Hi,

I believe I also found a Bug in the Preprocessor. When the Preprocessor parses the ShaderCode to clean it from
Commtent and so on, it is a little bit too strict. It can't handle the function parameter statement "in out".

Code: Select all

bool testFunction(in out vec3 v0)
Is changed to

Code: Select all

bool testFunction(in vec3 v0)
I believe the Preprocessor should leave the function declarations asis.
I am also wundering why the Preprocessor does so much cleanup on the code. Shouldn't the compiler do the
code optimization?
I understand that the preprocessor is needed to add code to the shaders but imo it shouldn't change the actual code,
because this makes shader debugging much harder :)

Cheers

Sebastian

Re: glsl preprocessor

Posted: Thu Oct 28, 2010 12:10 am
by cyrfer
Sebastian,
I like that vote. I asked for this in a feature request but the dev gods didn't like it.

Re: glsl preprocessor

Posted: Thu Dec 09, 2010 3:18 am
by cyrfer
I'm able to do some things with conditional preprocessor statements. I wonder if the 1.7.2 release fixed some cases. However, I have confirmed that #if/ #elif/ #else/ #endif is not working. I did a simple test by making the output color different depending on the preprocessor definitions. It won't even work with a hard coded definition.

Code: Select all

#define TEST_2

#if defined(TEST_1)
  outcolor = vec4(1,0,0,1);
#elif defined(TEST_2)
  outcolor = vec4(0,1,0,1);
#else
  outcolor = vec4(0,0,1,1);
#endif
The output color is always the 'else' part of the conditional. I tried this in HLSL as well and I get the right (different than GLSL) results.

Re: glsl preprocessor

Posted: Fri Apr 29, 2011 11:02 pm
by cyrfer
I've found another issue with the GLSL preprocessor. When 'GLSLLinkProgramManager::extractConstantDefs' is called, the preprocessor defines have not been replaced. This function is supposed to process the shader source AFTER the preprocessor has transformed the code. See the comments requiring the preprocessed source, in OgreGLSLLinkProgramManager.cpp, lines 397 to 399 (I'm using the Sinbad Hg clone, before any 1.8.0 release).

My symptom is that OGRE is 'asserting' (in OgreGLSLLinkProgramManager::extractUniforms) because it thinks that one of my array uniforms has an array size of '0' and OpenGL is telling Ogre that the array has a different size upon linking (4 in my case). The Ogre code decided to assert instead of logging, or even trusting GL. I'm not sure but I think that OGRE uses the post-preprocessed shader source to count parameter offsets. I guess that is why the author decided to assert here instead of just using what GL reports for the uniform's array size.

[edit] I fixed it. See patch. [/edit]

Re: glsl preprocessor

Posted: Wed Jun 08, 2011 10:35 am
by Kaylx
I just came across a glsl preprocessor bug myself in Ogre 1.7.2.
Would your patch fix the following issue?

In my glsl fragment shader i have this:

Code: Select all

#ifdef ABC
uniform float bar;
#endif
and in my .program file i have this:

Code: Select all

fragment_program Foo_ABC_FP glsl
{
    source Foo.fs
    preprocessor_defines ABC
    default_params
    {
        param_named bar float  1.75
    }
}
and in my material i have this:

Code: Select all

fragment_program_ref Foo_ABC_FP
{
    param_named bar float  1.25
}
and Ogre spits out this:

Code: Select all

10:27:22: OGRE EXCEPTION(2:InvalidParametersException): Parameter called bar does not exist.  in GpuProgramParameters::_findNamedConstantDefinition at ..\..\ogre_src_v1-7-2\OgreMain\src\OgreGpuProgramParams.cpp (line 1433)
10:27:22: Compiler error: invalid parameters in Foo.material(549): setting of constant failed
So it looks like Ogre is trying to find/set the uniform parameter before the #define is applied?

Re: glsl preprocessor

Posted: Mon Jun 20, 2011 9:24 pm
by cyrfer
I think you just need to define your preprocessor differently, but I could be wrong. Make sure your code is actually using the variable also, then I think you should get the benefit of a compiler error if the variable is not declared. Try:
preprocessor_defines ABC=1

If I'm wrong about the way you defined your macro, then the patch might help. I made the patch against SVN 1.8, but it still may merge fine with 1.7.2 source.

Re: glsl preprocessor

Posted: Mon Jul 25, 2011 12:58 am
by Xavyiy
Well... another one with the glsl preprocessor bug problem here!

I'm writting some shaders for SkyX in GLSL, and glsl preprocessor conditional statements really don't work! I can't do even the most simple condition :(
While in HLSL all is working as expected!

My code:
.material:

Code: Select all

... fragment_program SkyX_Skydome_STARFIELD_LDR_FP_GLSL glsl
{
    source SkyX_Skydome.fragment
	preprocessor_defines LDR,STARFIELD

    default_params
    {
    }
} ...
in the fragment program:

Code: Select all

...
#ifdef LDR
	gl_FragColor = vec4((1 - exp(-uExposure * (rayleighPhase * vRayleighColor + miePhase * vMieColor))), vOpacity);
#else // HDR
    gl_FragColor = vec4(rayleighPhase * vRayleighColor + miePhase * vMieColor, vOpacity);
#endif // LDR
	...
#ifdef STARFIELD
	gl_FragColor.xyz += nightmult *(vec3(0.05, 0.05, 0.1)*(2-0.75*clamp(-uLightDir.y, 0.0, 1.0))*pow(vHeight,3) + texture2D(uStarfield, vUV+uTime)*(0.35f + clamp(-uLightDir.y*0.45f, 0.0, 1.0))); 
#else // NO STARFIELD
	gl_FragColor.xyz += nightmult *(vec3(0.05, 0.05, 0.1)*(2-0.75*clamp(-uLightDir.y, 0.0, 1.0))*pow(vHeight,3)); 
#endif // STARFIELD	
...
The output is always the "else" statement part!

Hope it's going to be fixed before the next maintenance release!

Edit:
The first confitional statement seems to work, but the second one doesn't...
This work:

Code: Select all

#ifdef LDR
	gl_FragColor = vec4((1 - exp(-uExposure * (rayleighPhase * vRayleighColor + miePhase * vMieColor))), vOpacity);
#else // HDR
    gl_FragColor = vec4(rayleighPhase * vRayleighColor + miePhase * vMieColor, vOpacity);
#endif // LDR
Odd bug.

Xavi

Re: glsl preprocessor

Posted: Wed Aug 17, 2011 11:02 am
by Giggzee
is this bug fixed or not?



----
How To Use Your Spy Earpiece For Maximum Benefit
Useful Features Of Spy Earpiece

Re: glsl preprocessor

Posted: Wed Aug 17, 2011 9:58 pm
by cyrfer
I'm not sure if everything was fixed. I AM able to use the "ifdef", "ifndef", and "else" statements. The 1.8 code has a patch now applied that fixes some problems related to #defines providing the array size. I do not know if this was applied to the 1.7 code.

Here are some code fragments that are working well for me:
preprocessor_defines NORMAL_TEXTURE=1

Code: Select all

#ifdef NORMAL_TEXTURE
        // transform view space to tangent space
        vec3 L_ts = vec3( dot(L,T), dot(L,B), dot(L,N) );
        vec3 H_ts = vec3( dot(H,T), dot(H,B), dot(H,N) );
        // calculate the coefficients
        float I_diffuse = clamp( dot(bump,L_ts), 0.0, 1.0 );
        float I_specular = clamp( dot(bump,H_ts), 0.0, 1.0 );
#else // NORMAL_TEXTURE
        // calculate the coefficients
        float I_specular = max( dot(N,H), 0.0 );
        float I_diffuse = max( dot(N,L), 0.0 );
#endif // NORMAL_TEXTURE