[Mesa-dev] [PATCH 07/11] glsl: Add arb_cull_distance support

Tobias Klausmann tobias.johannes.klausmann at mni.thm.de
Sun May 24 15:46:32 PDT 2015


hi,
replay inline.

On 25.05.2015 00:34, Timothy Arceri wrote:
> On Sun, 2015-05-24 at 19:58 +0200, Tobias Klausmann wrote:
>> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de>
>> ---
>>   src/glsl/ast_to_hir.cpp             |  14 +++++
>>   src/glsl/builtin_variables.cpp      |  13 +++-
>>   src/glsl/glcpp/glcpp-parse.y        |   3 +
>>   src/glsl/glsl_parser_extras.cpp     |   1 +
>>   src/glsl/glsl_parser_extras.h       |   3 +
>>   src/glsl/link_varyings.cpp          |   2 +-
>>   src/glsl/linker.cpp                 | 121 +++++++++++++++++++++++++-----------
>>   src/glsl/standalone_scaffolding.cpp |   1 +
>>   src/glsl/tests/varyings_test.cpp    |  27 ++++++++
>>   9 files changed, 145 insertions(+), 40 deletions(-)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 8aebb13..4db2b2e 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -1045,6 +1045,20 @@ check_builtin_array_max_size(const char *name, unsigned size,
>>         _mesa_glsl_error(&loc, state, "`gl_ClipDistance' array size cannot "
>>                          "be larger than gl_MaxClipDistances (%u)",
>>                          state->Const.MaxClipPlanes);
>> +   } else if (strcmp("gl_CullDistance", name) == 0
>> +              && size > state->Const.MaxClipPlanes) {
>> +      /* From the ARB_cull_distance spec:
>> +       *
>> +       *   "The gl_CullDistance array is predeclared as unsized and
>> +       *    must be sized by the shader either redeclaring it with
>> +       *    a size or indexing it only with integral constant
>> +       *    expressions. The size determines the number and set of
>> +       *    enabled cull distances and can be at most
>> +       *    gl_MaxCullDistances."
>> +       */
>> +      _mesa_glsl_error(&loc, state, "`gl_CullDistance' array size cannot "
>> +                       "be larger than gl_MaxCullDistances (%u)",
>> +                       state->Const.MaxClipPlanes);
>>      }
>>   }
>>   
>> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
>> index 6806aa1..78c8db2 100644
>> --- a/src/glsl/builtin_variables.cpp
>> +++ b/src/glsl/builtin_variables.cpp
>> @@ -298,7 +298,7 @@ public:
>>      const glsl_type *construct_interface_instance() const;
>>   
>>   private:
>> -   glsl_struct_field fields[10];
>> +   glsl_struct_field fields[11];
>>      unsigned num_fields;
>>   };
>>   
>> @@ -600,6 +600,12 @@ builtin_variable_generator::generate_constants()
>>         add_const("gl_MaxVaryingComponents", state->ctx->Const.MaxVarying * 4);
>>      }
>>   
>> +   if (state->is_version(450, 0) || state->ARB_cull_distance_enable) {
>> +      add_const("gl_MaxCullDistances", state->Const.MaxClipPlanes);
>> +      add_const("gl_MaxCombinedClipAndCullDistances",
>> +                state->Const.MaxClipPlanes);
>> +   }
>> +
>>      if (state->is_version(150, 0)) {
>>         add_const("gl_MaxVertexOutputComponents",
>>                   state->Const.MaxVertexOutputComponents);
>> @@ -1029,6 +1035,11 @@ builtin_variable_generator::generate_varyings()
>>                      "gl_ClipDistance");
>>      }
>>   
>> +   if (state->is_version(450, 0) || state->ARB_cull_distance_enable) {
>> +       ADD_VARYING(VARYING_SLOT_CULL_DIST0, array(float_t, 0),
>> +                   "gl_CullDistance");
>> +   }
>> +
>>      if (compatibility) {
>>         ADD_VARYING(VARYING_SLOT_TEX0, array(vec4_t, 0), "gl_TexCoord");
>>         ADD_VARYING(VARYING_SLOT_FOGC, float_t, "gl_FogFragCoord");
>> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
>> index a11b6b2..536c17f 100644
>> --- a/src/glsl/glcpp/glcpp-parse.y
>> +++ b/src/glsl/glcpp/glcpp-parse.y
>> @@ -2483,6 +2483,9 @@ _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t versio
>>   
>>                 if (extensions->ARB_shader_precision)
>>                    add_builtin_define(parser, "GL_ARB_shader_precision", 1);
>> +
>> +          if (extensions->ARB_cull_distance)
>> +	         add_builtin_define(parser, "GL_ARB_cull_distance", 1);
>>   	   }
>>   	}
>>   
>> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
>> index 046d5d7..d1cd8ff 100644
>> --- a/src/glsl/glsl_parser_extras.cpp
>> +++ b/src/glsl/glsl_parser_extras.cpp
>> @@ -554,6 +554,7 @@ static const _mesa_glsl_extension _mesa_glsl_supported_extensions[] = {
>>      EXT(ARB_arrays_of_arrays,           true,  false,     ARB_arrays_of_arrays),
>>      EXT(ARB_compute_shader,             true,  false,     ARB_compute_shader),
>>      EXT(ARB_conservative_depth,         true,  false,     ARB_conservative_depth),
>> +   EXT(ARB_cull_distance,              true,  false,     ARB_cull_distance),
>>      EXT(ARB_derivative_control,         true,  false,     ARB_derivative_control),
>>      EXT(ARB_draw_buffers,               true,  false,     dummy_true),
>>      EXT(ARB_draw_instanced,             true,  false,     ARB_draw_instanced),
>> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
>> index 9a0c24e..8572905 100644
>> --- a/src/glsl/glsl_parser_extras.h
>> +++ b/src/glsl/glsl_parser_extras.h
>> @@ -378,6 +378,7 @@ struct _mesa_glsl_parse_state {
>>   
>>         /* ARB_viewport_array */
>>         unsigned MaxViewports;
>> +
>>      } Const;
>>   
>>      /**
>> @@ -430,6 +431,8 @@ struct _mesa_glsl_parse_state {
>>      bool ARB_compute_shader_warn;
>>      bool ARB_conservative_depth_enable;
>>      bool ARB_conservative_depth_warn;
>> +   bool ARB_cull_distance_enable;
>> +   bool ARB_cull_distance_warn;
>>      bool ARB_derivative_control_enable;
>>      bool ARB_derivative_control_warn;
>>      bool ARB_draw_buffers_enable;
>> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
>> index 46f84c6..81c0dac 100644
>> --- a/src/glsl/link_varyings.cpp
>> +++ b/src/glsl/link_varyings.cpp
>> @@ -404,7 +404,7 @@ tfeedback_decl::assign_location(struct gl_context *ctx,
>>            this->matched_candidate->type->fields.array->vector_elements;
>>         unsigned actual_array_size =
>>            (this->is_clip_distance_mesa || this->is_cull_distance_mesa) ?
>> -         prog->LastClipDistanceArraySize :
>> +         prog->LastClipCullDistanceArraySize :
>>            this->matched_candidate->type->array_size();
>>   
>>         if (this->is_subscripted) {
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index e616520..8bb1a5c 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -474,49 +474,94 @@ link_invalidate_variable_locations(exec_list *ir)
>>   }
>>   
>>
>> +
>>   /**
>> - * Set UsesClipDistance and ClipDistanceArraySize based on the given shader.
>> + * Set UsesClipCullDistance and ClipCullDistanceArraySize based on the given
>> + * shader.
>>    *
>> - * Also check for errors based on incorrect usage of gl_ClipVertex and
>> - * gl_ClipDistance.
>> + * Also check for errors based on incorrect usage of gl_ClipVertex,
>> + * gl_ClipDistance and gl_CullDistance.
>> + *
>> + * Additionally test whether the arrays gl_ClipDistance and gl_CullDistance
>> + * exceed the maximum size defined by gl_MaxCombinedClipAndCullDistances.
>>    *
>>    * Return false if an error was reported.
>>    */
>>   static void
>> -analyze_clip_usage(struct gl_shader_program *prog,
>> -                   struct gl_shader *shader, GLboolean *UsesClipDistance,
>> -                   GLuint *ClipDistanceArraySize)
>> +analyze_clip_cull_usage(struct gl_shader_program *prog,
>> +                        struct gl_shader *shader,
>> +                        struct gl_context *ctx,
>> +                        GLboolean *UsesClipCullDistance,
>> +                        GLuint *ClipCullDistanceArraySize)
>>   {
>> -   *ClipDistanceArraySize = 0;
>> +   *ClipCullDistanceArraySize = 0;
>>   
>> -   if (!prog->IsES && prog->Version >= 130) {
>> -      /* From section 7.1 (Vertex Shader Special Variables) of the
>> -       * GLSL 1.30 spec:
>> -       *
>> -       *   "It is an error for a shader to statically write both
>> -       *   gl_ClipVertex and gl_ClipDistance."
>> -       *
>> -       * This does not apply to GLSL ES shaders, since GLSL ES defines neither
>> -       * gl_ClipVertex nor gl_ClipDistance.
>> -       */
>> +   if (prog->IsES && prog->Version < 130) {
>> +      *UsesClipCullDistance = false;
>> +   } else {
>>         find_assignment_visitor clip_vertex("gl_ClipVertex");
>>         find_assignment_visitor clip_distance("gl_ClipDistance");
>> +      find_assignment_visitor cull_distance("gl_CullDistance");
>>   
>>         clip_vertex.run(shader->ir);
>>         clip_distance.run(shader->ir);
>> +      cull_distance.run(shader->ir);
>> +
>> +      /* From the ARB_cull_distance spec:
>> +       *
>> +       * It is a compile-time or link-time error for the set of shaders forming
>> +       * a program to statically read or write both gl_ClipVertex and either
>> +       * gl_ClipDistance or gl_CullDistance.
>> +       *
>> +       * This does not apply to GLSL ES shaders, since GLSL ES defines neither
>> +       * gl_ClipVertex, gl_ClipDistance or gl_CullDistance.
>> +       */
>>         if (clip_vertex.variable_found() && clip_distance.variable_found()) {
>>            linker_error(prog, "%s shader writes to both `gl_ClipVertex' "
>>                         "and `gl_ClipDistance'\n",
>>                         _mesa_shader_stage_to_string(shader->Stage));
>>            return;
>>         }
>> -      *UsesClipDistance = clip_distance.variable_found();
>> +      if (clip_vertex.variable_found() && cull_distance.variable_found()) {
>> +         linker_error(prog, "%s shader writes to both `gl_ClipVertex' "
>> +                      "and `gl_CullDistance'\n",
>> +                      _mesa_shader_stage_to_string(shader->Stage));
>> +         return;
>> +      }
>> +
>> +      *UsesClipCullDistance = clip_distance.variable_found() |
>> +                              cull_distance.variable_found();
>> +
>>         ir_variable *clip_distance_var =
>>            shader->symbols->get_variable("gl_ClipDistance");
>> +      ir_variable *cull_distance_var =
>> +         shader->symbols->get_variable("gl_CullDistance");
>> +
>>         if (clip_distance_var)
>> -         *ClipDistanceArraySize = clip_distance_var->type->length;
>> -   } else {
>> -      *UsesClipDistance = false;
>> +         *ClipCullDistanceArraySize = clip_distance_var->type->was_unsized ?
>> +               clip_distance_var->type->length  -1 :
>> +               clip_distance_var->type->length;
>> +
>> +      if (cull_distance_var)
>> +         *ClipCullDistanceArraySize+= cull_distance_var->type->was_unsized ?
>> +               cull_distance_var->type->length -1 :
>> +               cull_distance_var->type->length;
>
> How come this needs to be done for former unsized arrays?
>

We replace an unsized array with a default size of 1 ( fixup_type() ) , 
so with either cull or clip set to max (8) and the other array left 
unsized we would end up with a total of 9 reported, that would trigger 
the error below.

>
>> +
>> +      /* From the ARB_cull_distance spec:
>> +       *
>> +       * It is a compile-time or link-time error for the set of shaders forming
>> +       * a program to have the sum of the sizes of the gl_ClipDistance and
>> +       * gl_CullDistance arrays to be larger than
>> +       * gl_MaxCombinedClipAndCullDistances.
>> +       */
>> +      if (*ClipCullDistanceArraySize > ctx->Const.MaxClipPlanes) {
>> +          linker_error(prog, "%s shader: the combined size of "
>> +                       "'gl_ClipDistance' and 'gl_CullDistance' size cannot "
>> +                       "be larger than "
>> +                       "gl_MaxCombinedClipAndCullDistances (%u)",
>> +                       _mesa_shader_stage_to_string(shader->Stage),
>> +                       ctx->Const.MaxClipPlanes);
>> +      }
>>      }
>>   }
>>   
>> @@ -524,14 +569,14 @@ analyze_clip_usage(struct gl_shader_program *prog,
>>   /**
>>    * Verify that a vertex shader executable meets all semantic requirements.
>>    *
>> - * Also sets prog->Vert.UsesClipDistance and prog->Vert.ClipDistanceArraySize
>> - * as a side effect.
>> + * Also sets prog->Vert.UsesClipCullDistance and
>> + * prog->Vert.ClipCullDistanceArraySize as a side effect.
>>    *
>>    * \param shader  Vertex shader executable to be verified
>>    */
>>   void
>>   validate_vertex_shader_executable(struct gl_shader_program *prog,
>> -				  struct gl_shader *shader)
>> +				  struct gl_shader *shader, struct gl_context *ctx)
>>   {
>>      if (shader == NULL)
>>         return;
>> @@ -578,8 +623,8 @@ validate_vertex_shader_executable(struct gl_shader_program *prog,
>>         }
>>      }
>>   
>> -   analyze_clip_usage(prog, shader, &prog->Vert.UsesClipDistance,
>> -                      &prog->Vert.ClipDistanceArraySize);
>> +   analyze_clip_cull_usage(prog, shader, ctx, &prog->Vert.UsesClipCullDistance,
>> +                           &prog->Vert.ClipCullDistanceArraySize);
>>   }
>>   
>>
>> @@ -590,7 +635,7 @@ validate_vertex_shader_executable(struct gl_shader_program *prog,
>>    */
>>   void
>>   validate_fragment_shader_executable(struct gl_shader_program *prog,
>> -				    struct gl_shader *shader)
>> +				    struct gl_shader *shader, struct gl_context *ctx)
>>   {
>>      if (shader == NULL)
>>         return;
>> @@ -610,14 +655,14 @@ validate_fragment_shader_executable(struct gl_shader_program *prog,
>>   /**
>>    * Verify that a geometry shader executable meets all semantic requirements
>>    *
>> - * Also sets prog->Geom.VerticesIn, prog->Geom.UsesClipDistance, and
>> - * prog->Geom.ClipDistanceArraySize as a side effect.
>> + * Also sets prog->Geom.VerticesIn, prog->Geom.UsesClipCullDistance, and
>> + * prog->Geom.ClipCullDistanceArraySize as a side effect.
>>    *
>>    * \param shader Geometry shader executable to be verified
>>    */
>>   void
>>   validate_geometry_shader_executable(struct gl_shader_program *prog,
>> -				    struct gl_shader *shader)
>> +				    struct gl_shader *shader, struct gl_context *ctx)
>>   {
>>      if (shader == NULL)
>>         return;
>> @@ -625,8 +670,8 @@ validate_geometry_shader_executable(struct gl_shader_program *prog,
>>      unsigned num_vertices = vertices_per_prim(prog->Geom.InputType);
>>      prog->Geom.VerticesIn = num_vertices;
>>   
>> -   analyze_clip_usage(prog, shader, &prog->Geom.UsesClipDistance,
>> -                      &prog->Geom.ClipDistanceArraySize);
>> +   analyze_clip_cull_usage(prog, shader, ctx, &prog->Geom.UsesClipCullDistance,
>> +                           &prog->Geom.ClipCullDistanceArraySize);
>>   }
>>   
>>   /**
>> @@ -2834,13 +2879,13 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>>   
>>            switch (stage) {
>>            case MESA_SHADER_VERTEX:
>> -            validate_vertex_shader_executable(prog, sh);
>> +            validate_vertex_shader_executable(prog, sh, ctx);
>>               break;
>>            case MESA_SHADER_GEOMETRY:
>> -            validate_geometry_shader_executable(prog, sh);
>> +            validate_geometry_shader_executable(prog, sh, ctx);
>>               break;
>>            case MESA_SHADER_FRAGMENT:
>> -            validate_fragment_shader_executable(prog, sh);
>> +            validate_fragment_shader_executable(prog, sh, ctx);
>>               break;
>>            }
>>            if (!prog->LinkStatus)
>> @@ -2851,11 +2896,11 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>>      }
>>   
>>      if (num_shaders[MESA_SHADER_GEOMETRY] > 0)
>> -      prog->LastClipDistanceArraySize = prog->Geom.ClipDistanceArraySize;
>> +      prog->LastClipCullDistanceArraySize = prog->Geom.ClipCullDistanceArraySize;
>>      else if (num_shaders[MESA_SHADER_VERTEX] > 0)
>> -      prog->LastClipDistanceArraySize = prog->Vert.ClipDistanceArraySize;
>> +      prog->LastClipCullDistanceArraySize = prog->Vert.ClipCullDistanceArraySize;
>>      else
>> -      prog->LastClipDistanceArraySize = 0; /* Not used */
>> +      prog->LastClipCullDistanceArraySize = 0; /* Not used */
>>   
>>      /* Here begins the inter-stage linking phase.  Some initial validation is
>>       * performed, then locations are assigned for uniforms, attributes, and
>> diff --git a/src/glsl/standalone_scaffolding.cpp b/src/glsl/standalone_scaffolding.cpp
>> index a109c4e..0ac16bb 100644
>> --- a/src/glsl/standalone_scaffolding.cpp
>> +++ b/src/glsl/standalone_scaffolding.cpp
>> @@ -142,6 +142,7 @@ void initialize_context_to_defaults(struct gl_context *ctx, gl_api api)
>>      ctx->Extensions.ARB_texture_query_lod = true;
>>      ctx->Extensions.ARB_uniform_buffer_object = true;
>>      ctx->Extensions.ARB_viewport_array = true;
>> +   ctx->Extensions.ARB_cull_distance = true;
>>   
>>      ctx->Extensions.OES_EGL_image_external = true;
>>      ctx->Extensions.OES_standard_derivatives = true;
>> diff --git a/src/glsl/tests/varyings_test.cpp b/src/glsl/tests/varyings_test.cpp
>> index 4573529..7a962c5 100644
>> --- a/src/glsl/tests/varyings_test.cpp
>> +++ b/src/glsl/tests/varyings_test.cpp
>> @@ -202,6 +202,33 @@ TEST_F(link_varyings, gl_ClipDistance)
>>      EXPECT_TRUE(is_empty(consumer_interface_inputs));
>>   }
>>   
>> +TEST_F(link_varyings, gl_CullDistance)
>> +{
>> +   const glsl_type *const array_8_of_float =
>> +      glsl_type::get_array_instance(glsl_type::vec(1), 8);
>> +
>> +   ir_variable *const culldistance =
>> +      new(mem_ctx) ir_variable(array_8_of_float,
>> +                               "gl_CullDistance",
>> +                               ir_var_shader_in);
>> +
>> +   culldistance->data.explicit_location = true;
>> +   culldistance->data.location = VARYING_SLOT_CULL_DIST0;
>> +   culldistance->data.explicit_index = 0;
>> +
>> +   ir.push_tail(culldistance);
>> +
>> +   ASSERT_TRUE(linker::populate_consumer_input_sets(mem_ctx,
>> +                                                    &ir,
>> +                                                    consumer_inputs,
>> +                                                    consumer_interface_inputs,
>> +                                                    junk));
>> +
>> +   EXPECT_EQ(culldistance, junk[VARYING_SLOT_CULL_DIST0]);
>> +   EXPECT_TRUE(is_empty(consumer_inputs));
>> +   EXPECT_TRUE(is_empty(consumer_interface_inputs));
>> +}
>> +
>>   TEST_F(link_varyings, single_interface_input)
>>   {
>>      ir_variable *const v =
>



More information about the mesa-dev mailing list