[Mesa-dev] A few fixes for the preprocessor for GLSL 1.30

[Ian Romanick]

On 09/29/2011 11:00 PM, Carl Worth wrote:
> Eric recentl mentioned to me that when setting the following
> environment variables with current master mesa:
>
> 	MESA_GL_VERSION_OVERRIDE=3.0
> 	MESA_GLSL_VERSION_OVERRIDE=130
>
> that 8 piglit preprocessor tests start to fail.
>
> I've investigated all of these failures and am here attaching a patch
> series for review that fixes 5 of them. In the testing I've done, I
> don't see any changes from this patch series other than the desired
> piglit fixes. But, of course, I'd be happy for any review.
>
> For the remaining three failures, I believe the tests themselves are
> broken and should be fixed. These fall into two case:
>
> 1. feature-macros/gl_gragment_precision_high.{frag,vert}
>
>     In these tests, the actual preprocessor functionality being tested,
>     (that a directive of #version 130 causes the
>     GL_FRAGMENT_PRECISION_HIGH macro to be pre-defined), is working fine.
>
>     The tests are failing due to the following code:
>
> 	float f() { return 0; }
>
>     Our implementation does not currently do implicit promotion here,
>     so compilation fails with a type error:
>
> 	error: `return' with wrong type int, in function `f' returning
> 	float
>
>     The GLSL specification is not explicit on whether this promotion
>     should be happening, but other implementations appear to do it. So
>     perhaps the promotion should be added to our implementation.

It is not allowed until a much later GLSL version.

>     Meanwhile, the test could legitimately be fixed to remove the type
>     error which is a side issue from what the test is actually trying
>     to do.

The test should be fixed to return 0.0.

> 2. if/if-arg-must-be-defined-02.frag

Neither this test nor if-arg-must-be-defined-01 produce the desired 
result on NVIDIA's blob or AMD's blob.  I'd be curious to know what it 
does on Apple's implementation.

In any case, the test quotes specific language from the GLSL spec that 
explicitly forbids this behavior, so I'm somewhat reluctant to deviate 
from the spec'ed behavior.

>     This test is trying to elicit an "undefined macro" error with
>     something like the following code:
>
> 	#if 1
> 	...
> 	#elif UNDEFINED_MACRO
> 	...
> 	#endif
>
>     I propose changing the test to use "#if 0" instead, in which case
>     the current implementation would pass. It's not actually desirable
>     to generate an error in the above case. Consider a use of a
>     possibly undefined macro:
>
> 	#if PERHAPS_UNDEFINED<  5
> 	...
> 	#endif
>
>     In order to protect this use, the user might add something like the
>     following (or similar with #ifndef):
>
> 	#if ! defined PERHAPS_UNDEFINED
> 	... deal with it ...
> 	#elif PERHAPS_UNDEFINED
> 	...
> 	#endif
>
>     In this case, the user's expectation is to get no error with this
>     code. Yet this is precisly the formulation the current test case
>     uses to elicit the error.
>
> I'll be glad to push changes to piglit to fix these cases unless
> someone can argue aginst that.