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.led...@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 <matts...@gmail.com>: >> >> On Tue, Dec 9, 2014 at 11:09 PM, Maxence Le Doré >> <maxence.led...@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.led...@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 <matts...@gmail.com>: >> >> On Tue, Dec 9, 2014 at 11:09 PM, Maxence Le Doré >> <maxence.led...@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. >> >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
