On 25.05.2015 01:19, Timothy Arceri wrote:
On Mon, 2015-05-25 at 00:46 +0200, Tobias Klausmann wrote:
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.klausm...@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.
I see. Is there a piglit test for this? I couldn't find it but I may be
blind. I take it the Nvidia driver passes in this case?

Wouldn't you only want the test below to pass if the unsized cull or
clip is unused? If it defaults to 1 (or is resized to 1) and its used
then isn't it valid for this to fail?


You are right, if the array is implicitly sized to one and used, this is plain wrong, thanks for pointing this out

+
+      /* 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 =


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to