On 05.04.2016 00:06, Dave Airlie wrote:
On 4 April 2016 at 23:07, Tobias Klausmann
<tobias.johannes.klausm...@mni.thm.de> wrote:

On 04.04.2016 04:48, Timothy Arceri wrote:
On Mon, 2016-04-04 at 12:15 +1000, Dave Airlie wrote:
From: Tobias Klausmann <tobias.johannes.klausm...@mni.thm.de>

Signed-off-by: Tobias Klausmann <tobias.johannes.klausm...@mni.thm.de
---
   src/compiler/glsl/ast_to_hir.cpp             |  14 +++
   src/compiler/glsl/builtin_variables.cpp      |  11 ++-
   src/compiler/glsl/glcpp/glcpp-parse.y        |   3 +
   src/compiler/glsl/glsl_parser_extras.cpp     |   1 +
   src/compiler/glsl/glsl_parser_extras.h       |   2 +
   src/compiler/glsl/link_varyings.cpp          |  10 +++
   src/compiler/glsl/link_varyings.h            |   1 +
   src/compiler/glsl/linker.cpp                 | 122
+++++++++++++++++++++------
   src/compiler/glsl/standalone_scaffolding.cpp |   1 +
   src/compiler/glsl/tests/varyings_test.cpp    |  27 ++++++
   src/compiler/shader_enums.h                  |   4 +
   11 files changed, 169 insertions(+), 27 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index 7c9be81..47db841 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -1210,6 +1210,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/compiler/glsl/builtin_variables.cpp
b/src/compiler/glsl/builtin_variables.cpp
index f31f9f6..8d049c8 100644
--- a/src/compiler/glsl/builtin_variables.cpp
+++ b/src/compiler/glsl/builtin_variables.cpp
@@ -302,7 +302,7 @@ public:
      const glsl_type *construct_interface_instance() const;
     private:
-   glsl_struct_field fields[10];
+   glsl_struct_field fields[11];
      unsigned num_fields;
   };
   @@ -675,6 +675,11 @@ builtin_variable_generator::generate_constants()
         add_const("gl_MaxClipDistances", state->Const.MaxClipPlanes);
         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->has_geometry_shader()) {
         add_const("gl_MaxVertexOutputComponents",
@@ -1246,6 +1251,10 @@
builtin_variable_generator::generate_varyings()
          add_varying(VARYING_SLOT_CLIP_DIST0, array(float_t, 0),
                      "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");
diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y
b/src/compiler/glsl/glcpp/glcpp-parse.y
index a48266c..e44f074 100644
--- a/src/compiler/glsl/glcpp/glcpp-parse.y
+++ b/src/compiler/glsl/glcpp/glcpp-parse.y
@@ -2457,6 +2457,9 @@
_glcpp_parser_handle_version_declaration(glcpp_parser_t *parser,
intmax_t versio
              if (extensions->ARB_shader_draw_parameters)
               add_builtin_define(parser,
"GL_ARB_shader_draw_parameters", 1);
+
+         if (extensions->ARB_cull_distance)
+            add_builtin_define(parser, "GL_ARB_cull_distance", 1);
         }
      }
   diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
b/src/compiler/glsl/glsl_parser_extras.cpp
index 76321aa..9b1d53f 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -569,6 +569,7 @@ static const _mesa_glsl_extension
_mesa_glsl_supported_extensions[] = {
      EXT(ARB_arrays_of_arrays,             true,  false,     ARB_array
s_of_arrays),
      EXT(ARB_compute_shader,               true,  false,     ARB_compu
te_shader),
      EXT(ARB_conservative_depth,           true,  false,     ARB_conse
rvative_depth),
+   EXT(ARB_cull_distance,                true,  false,     ARB_cull_
distance),
      EXT(ARB_derivative_control,           true,  false,     ARB_deriv
ative_control),
      EXT(ARB_draw_buffers,                 true,  false,     dummy_tru
e),
      EXT(ARB_draw_instanced,               true,  false,     ARB_draw_
instanced),
diff --git a/src/compiler/glsl/glsl_parser_extras.h
b/src/compiler/glsl/glsl_parser_extras.h
index c774fbe..85a8ebf 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -518,6 +518,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/compiler/glsl/link_varyings.cpp
b/src/compiler/glsl/link_varyings.cpp
index 8e74981..d4cc68f 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -573,6 +573,10 @@ tfeedback_decl::init(struct gl_context *ctx,
const void *mem_ctx,
          strcmp(this->var_name, "gl_ClipDistance") == 0) {
         this->lowered_builtin_array_variable = clip_distance;
      }
+   if (ctx-
Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].LowerCombinedClipCul
lDistance &&
+       strcmp(this->var_name, "gl_CullDistance") == 0) {
+      this->lowered_builtin_array_variable = cull_distance;
+   }
        if (ctx->Const.LowerTessLevel &&
          (strcmp(this->var_name, "gl_TessLevelOuter") == 0))
@@ -633,6 +637,9 @@ tfeedback_decl::assign_location(struct gl_context
*ctx,
         case clip_distance:
            actual_array_size = prog->LastClipDistanceArraySize;
            break;
+      case cull_distance:
+         actual_array_size = prog->LastCullDistanceArraySize;
+         break;
         case tess_level_outer:
            actual_array_size = 4;
            break;
@@ -853,6 +860,9 @@ tfeedback_decl::find_candidate(gl_shader_program
*prog,
      case clip_distance:
         name = "gl_ClipDistanceMESA";
         break;
+   case cull_distance:
+      name = "gl_CullDistanceMESA";
+      break;
      case tess_level_outer:
         name = "gl_TessLevelOuterMESA";
         break;
diff --git a/src/compiler/glsl/link_varyings.h
b/src/compiler/glsl/link_varyings.h
index 543b80f..0ad4f74 100644
--- a/src/compiler/glsl/link_varyings.h
+++ b/src/compiler/glsl/link_varyings.h
@@ -210,6 +210,7 @@ private:
      enum {
         none,
         clip_distance,
+      cull_distance,
         tess_level_outer,
         tess_level_inner,
      } lowered_builtin_array_variable;
diff --git a/src/compiler/glsl/linker.cpp
b/src/compiler/glsl/linker.cpp
index 9acf253..77142aa 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -641,19 +641,25 @@ link_invalidate_variable_locations(exec_list
*ir)
       /**
- * Set clip_distance_array_size based on the given shader.
+ * Set clip_distance_array_size based and cull_distance_array_size
on the given
+ * shader.
    *
    * Also check for errors based on incorrect usage of gl_ClipVertex
and
- * gl_ClipDistance.
+ * 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,
-                   GLuint *clip_distance_array_size)
+analyze_clip_cull_usage(struct gl_shader_program *prog,
+                        struct gl_shader *shader,
+                        struct gl_context *ctx,
+                        GLuint *clip_distance_array_size,
+                        GLuint *cull_distance_array_size)
   {
      *clip_distance_array_size = 0;
+   *cull_distance_array_size = 0;
        if (!prog->IsES && prog->Version >= 130) {
         /* From section 7.1 (Vertex Shader Special Variables) of the
@@ -667,22 +673,71 @@ analyze_clip_usage(struct gl_shader_program
*prog,
          */
         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;
         }
+      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;
+      }
           if (clip_distance.variable_found()) {
            ir_variable *clip_distance_var =
-               shader->symbols->get_variable("gl_ClipDistance");
-
+                shader->symbols->get_variable("gl_ClipDistance");
            assert(clip_distance_var);
-         *clip_distance_array_size = clip_distance_var->type-
length;
+         if (clip_distance_var->type->was_unsized)
+            *clip_distance_array_size =
clip_distance.variable_found() ?
+                                        clip_distance_var->type-
length :
+                                        clip_distance_var->type-
length -1;
+         else
+            *clip_distance_array_size = clip_distance_var->type-
length;
+      }
+      if (cull_distance.variable_found()) {
+         ir_variable *cull_distance_var =
+                shader->symbols->get_variable("gl_CullDistance");
+        assert(cull_distance_var);
+          if (cull_distance_var->type->was_unsized)
+             *cull_distance_array_size =
cull_distance.variable_found() ?
+                                         cull_distance_var->type-
length :
+                                         cull_distance_var->type-
length -1;
+          else
+              *cull_distance_array_size = cull_distance_var->type-
length;
I think we need a few piglit test cases for
the gl_MaxCombinedClipAndCullDistance test below. The above code makes
no sense.

We are inside an if cull_distance.variable_found()

Then we check for cull_distance_var->type->was_unsized

and if cull_distance.variable_found() we do:

*cull_distance_array_size = cull_distance_var->type->length;

So in otherwords we always just do

*cull_distance_array_size = cull_distance_var->type->length;

The was_unsized makes no difference same goes for the clip_distance
changes above. I don't think this is intended.

It was intended back then to fix an off-by-one for the combined size, but i
can't remember the actual test i fixed with that (there are 5 additional
piglit tests never got up-streamed for cull-distance, maybe it was one of
those i'll check when i got some time)
They got pushed yesterday.

Dave.

Oh, i was not aware of that, apologies!
Tobias
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to