[Mesa-dev] [PATCH 4/8] glsl: Add new builtins required by GL_ARB_sample_shading

[Kenneth Graunke]

On 10/14/2013 10:12 AM, Anuj Phogat wrote:
> New builtins added by GL_ARB_sample_shading:
> in vec2 gl_SamplePosition
> in int gl_SampleID
> in int gl_NumSamples
> out int gl_SampleMask[]
> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/glsl/builtin_variables.cpp | 11 +++++++++++
>  src/glsl/link_varyings.cpp     |  2 ++
>  src/mesa/main/mtypes.h         |  7 ++++++-
>  src/mesa/program/prog_print.c  |  5 +++++
>  4 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
> index ae0a03f..c886840 100644
> --- a/src/glsl/builtin_variables.cpp
> +++ b/src/glsl/builtin_variables.cpp
> @@ -30,6 +30,9 @@
>  #include "program/prog_statevars.h"
>  #include "program/prog_instruction.h"
>  
> +static struct gl_builtin_uniform_element gl_NumSamples_elements[] = {
> +   {NULL, {STATE_NUM_SAMPLES, 0, 0}, SWIZZLE_XYZW}

I believe this should be SWIZZLE_XXXX, since it's a scalar value.  (See
the similar code for gl_DepthRange immediately below.)

> +};
>  
>  static struct gl_builtin_uniform_element gl_DepthRange_elements[] = {
>     {"near", {STATE_DEPTH_RANGE, 0, 0}, SWIZZLE_XXXX},
> @@ -236,6 +239,7 @@ static struct gl_builtin_uniform_element gl_NormalMatrix_elements[] = {
>  #define STATEVAR(name) {#name, name ## _elements, Elements(name ## _elements)}
>  
>  static const struct gl_builtin_uniform_desc _mesa_builtin_uniform_desc[] = {
> +   STATEVAR(gl_NumSamples),
>     STATEVAR(gl_DepthRange),
>     STATEVAR(gl_ClipPlane),
>     STATEVAR(gl_Point),
> @@ -613,6 +617,7 @@ builtin_variable_generator::generate_constants()
>  void
>  builtin_variable_generator::generate_uniforms()
>  {
> +   add_uniform(int_t, "gl_NumSamples");
>     add_uniform(type("gl_DepthRangeParameters"), "gl_DepthRange");
>     add_uniform(array(vec4_t, VERT_ATTRIB_MAX), "gl_CurrentAttribVertMESA");
>     add_uniform(array(vec4_t, VARYING_SLOT_MAX), "gl_CurrentAttribFragMESA");
> @@ -789,6 +794,12 @@ builtin_variable_generator::generate_fs_special_vars()
>        if (state->AMD_shader_stencil_export_warn)
>           var->warn_extension = "GL_AMD_shader_stencil_export";
>     }
> +
> +   if (state->ARB_sample_shading_enable) {
> +      add_input(VARYING_SLOT_SAMPLE_ID, int_t, "gl_SampleID");
> +      add_input(VARYING_SLOT_SAMPLE_POS, vec2_t, "gl_SamplePosition");

      /* From the ARB_sample_shading specification:
       * "The number of elements in the array is ceil(<s>/32), where <s>
       *  is the maximum number of color samples supported by the
       *  implementation."
       *
       * Since no drivers expose more than 32x MSAA, we can simply set
       * the size to 1 rather than computing it.
       */

I think it's reasonable that you did this - I doubt anybody is going to
support >32x MSAA any time soon.  To do it properly, we'd have to use
ctx->Const.MaxColorTextureSamples (or MaxSamples?).  I believe they
should be initialized before this code gets called.  If not, that would
need to get fixed.

Adding a comment is definitely worthwhile, though.

> +      add_output(FRAG_RESULT_SAMPLE_MASK, array(int_t, 1), "gl_SampleMask");
> +   }
>  }

With those changes, this patch is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>