[Mesa-dev] [PATCH] glsl: Add gl_MaxViewports to available builtin constants

[Maxence Le Doré]

Hi Matt,

To be sure to not missing something, I used a simple vertex shader I have
there and had
the following line in :

int a = gl_MaxDrawBuffers;

At runtime, everything was ok, and GLSL compiler didn't complain about
anything. So there is nothing in mesa to prevent the use of some GLSL
constants having some specifity to one or more shading stage.

But a more interesting stuff is when I took a look at GLSL 4.1 spec there :
https://www.opengl.org/registry/doc/GLSLangSpec.4.10.6.clean.pdf

See section 7.3

"Built-In Constants
The following built-in constants are provided to all shaders. The actual
values used are implementation dependent, but must be at least the value
shown. Some are deprecated, as indicated in comments."

Follows a list where gl_MaxViewports is.

2014-12-12 22:12 GMT+01:00 Maxence Le Doré <maxence.ledore at gmail.com>:
>
> I had exactly the same feeling while adding it. How to make it GS specific
> (and eventually allow access to VS, where AMD_vertex_shader_viewport_index
> is supported, even if the spec don't say a word about gl_MaxViewports). In
> that kind of situation, I always take a look at a similar case nearby :
> gl_MaxDrawBuffers definition specificity to fragment shader stage. And I
> haven't seen how glsl compiler in its current state prevent it from being
> enabled in other stage too.
>
> 2014-12-12 18:19 GMT+01:00 Matt Turner <mattst88 at gmail.com>:
>>
>> On Tue, Dec 9, 2014 at 11:09 PM, Maxence Le Doré
>> <maxence.ledore at gmail.com> wrote:
>> > It seems to have been forgotten during viewports array implementation
>> time.
>> > ---
>> >  src/glsl/builtin_variables.cpp  | 4 ++++
>> >  src/glsl/glsl_parser_extras.cpp | 3 +++
>> >  src/glsl/glsl_parser_extras.h   | 3 +++
>> >  3 files changed, 10 insertions(+)
>> >
>> > diff --git a/src/glsl/builtin_variables.cpp
>> b/src/glsl/builtin_variables.cpp
>> > index c36d198..65e32ad 100644
>> > --- a/src/glsl/builtin_variables.cpp
>> > +++ b/src/glsl/builtin_variables.cpp
>> > @@ -724,6 +724,10 @@ builtin_variable_generator::generate_constants()
>> >        add_const("gl_MaxCombinedImageUniforms",
>> >                  state->Const.MaxCombinedImageUniforms);
>> >     }
>> > +
>> > +   if (state->is_version(410, 0) ||
>> > +       state->ARB_viewport_array_enable)
>> > +      add_const("gl_MaxViewports", state->Const.MaxViewports);
>> >  }
>>
>> Nice find. The only thing that's holding back my review is that the
>> specification says
>>
>>     Add to the list of built in constants available to geometry shaders in
>>     Section 7.4:
>>
>>         const int gl_MaxViewports = 16;
>>
>> and I don't see how we're preventing gl_MaxViewports from being
>> enabled in other shader stages.
>>
>> Maybe idr can confirm.
>
>
2014-12-12 22:12 GMT+01:00 Maxence Le Doré <maxence.ledore at gmail.com>:
>
> I had exactly the same feeling while adding it. How to make it GS specific
> (and eventually allow access to VS, where AMD_vertex_shader_viewport_index
> is supported, even if the spec don't say a word about gl_MaxViewports). In
> that kind of situation, I always take a look at a similar case nearby :
> gl_MaxDrawBuffers definition specificity to fragment shader stage. And I
> haven't seen how glsl compiler in its current state prevent it from being
> enabled in other stage too.
>
> 2014-12-12 18:19 GMT+01:00 Matt Turner <mattst88 at gmail.com>:
>>
>> On Tue, Dec 9, 2014 at 11:09 PM, Maxence Le Doré
>> <maxence.ledore at gmail.com> wrote:
>> > It seems to have been forgotten during viewports array implementation
>> time.
>> > ---
>> >  src/glsl/builtin_variables.cpp  | 4 ++++
>> >  src/glsl/glsl_parser_extras.cpp | 3 +++
>> >  src/glsl/glsl_parser_extras.h   | 3 +++
>> >  3 files changed, 10 insertions(+)
>> >
>> > diff --git a/src/glsl/builtin_variables.cpp
>> b/src/glsl/builtin_variables.cpp
>> > index c36d198..65e32ad 100644
>> > --- a/src/glsl/builtin_variables.cpp
>> > +++ b/src/glsl/builtin_variables.cpp
>> > @@ -724,6 +724,10 @@ builtin_variable_generator::generate_constants()
>> >        add_const("gl_MaxCombinedImageUniforms",
>> >                  state->Const.MaxCombinedImageUniforms);
>> >     }
>> > +
>> > +   if (state->is_version(410, 0) ||
>> > +       state->ARB_viewport_array_enable)
>> > +      add_const("gl_MaxViewports", state->Const.MaxViewports);
>> >  }
>>
>> Nice find. The only thing that's holding back my review is that the
>> specification says
>>
>>     Add to the list of built in constants available to geometry shaders in
>>     Section 7.4:
>>
>>         const int gl_MaxViewports = 16;
>>
>> and I don't see how we're preventing gl_MaxViewports from being
>> enabled in other shader stages.
>>
>> Maybe idr can confirm.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141214/ba5e9da5/attachment.html>