i965 Gen6: Implement gl_ClipVertex.

This patch implements proper support for gl_ClipVertex by causing the
new VS backend to populate the clip distance VUE slots using
VERT_RESULT_CLIP_VERTEX when appropriate, and by using the
untransformed clip planes in ctx->Transform.EyeUserPlane rather than
the transformed clip planes in ctx->Transform._ClipUserPlane when a
GLSL-based vertex shader is in use.

When not using a GLSL-based vertex shader, we use
ctx->Transform._ClipUserPlane (which is what we used prior to this
patch).  This ensures that clipping is still performed correctly for
fixed function and ARB vertex programs.  A new function,
brw_select_clip_planes() is used to determine whether to use
_ClipUserPlane or EyeUserPlane, so that the logic for making this
decision is shared between the new and old vertex shaders.

Fixes the following Piglit tests on i965 Gen6:
- vs-clip-vertex-const-accept
- vs-clip-vertex-const-reject
- vs-clip-vertex-different-from-position
- vs-clip-vertex-equal-to-position
- vs-clip-vertex-homogeneity
- vs-clip-based-on-position
- vs-clip-based-on-position-homogeneity
- clip-plane-transformation clipvert_pos
- clip-plane-transformation pos_clipvert
- clip-plane-transformation pos

Reviewed-by: Kenneth Graunke <[email protected]>
Reviewed-by: Chad Versace <[email protected]>

6 files changed, 75 insertions, 10 deletions

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index d32eded90d1..e26437fa448 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -899,6 +899,7 @@ struct brw_context
 };
 
 
+
 #define BRW_PACKCOLOR8888(r,g,b,a)  ((r<<24) | (g<<16) | (b<<8) | a)
 
 struct brw_instruction_info {
@@ -966,6 +967,8 @@ int brw_disasm (FILE *file, struct brw_instruction *inst, int gen);
 void brw_compute_vue_map(struct brw_vue_map *vue_map,
                          const struct intel_context *intel, int nr_userclip,
                          GLbitfield64 outputs_written);
+gl_clip_plane *brw_select_clip_planes(struct gl_context *ctx);
+
 
 /*======================================================================
  * Inline conversion functions.  These are better-typed than the
diff --git a/src/mesa/drivers/dri/i965/brw_curbe.c b/src/mesa/drivers/dri/i965/brw_curbe.c
index e1676de1e02..25c7e0908fd 100644
--- a/src/mesa/drivers/dri/i965/brw_curbe.c
+++ b/src/mesa/drivers/dri/i965/brw_curbe.c
@@ -188,6 +188,7 @@ static void prepare_constant_buffer(struct brw_context *brw)
    const GLuint bufsz = sz * 16 * sizeof(GLfloat);
    GLfloat *buf;
    GLuint i;
+   gl_clip_plane *clip_planes;
 
    if (sz == 0) {
       brw->curbe.last_bufsz  = 0;
@@ -232,12 +233,13 @@ static void prepare_constant_buffer(struct brw_context *brw)
       /* Clip planes: _NEW_TRANSFORM plus _NEW_PROJECTION to get to
        * clip-space:
        */
+      clip_planes = brw_select_clip_planes(ctx);
       for (j = 0; j < MAX_CLIP_PLANES; j++) {
 	 if (ctx->Transform.ClipPlanesEnabled & (1<<j)) {
-	    buf[offset + i * 4 + 0] = ctx->Transform._ClipUserPlane[j][0];
-	    buf[offset + i * 4 + 1] = ctx->Transform._ClipUserPlane[j][1];
-	    buf[offset + i * 4 + 2] = ctx->Transform._ClipUserPlane[j][2];
-	    buf[offset + i * 4 + 3] = ctx->Transform._ClipUserPlane[j][3];
+	    buf[offset + i * 4 + 0] = clip_planes[j][0];
+	    buf[offset + i * 4 + 1] = clip_planes[j][1];
+	    buf[offset + i * 4 + 2] = clip_planes[j][2];
+	    buf[offset + i * 4 + 3] = clip_planes[j][3];
 	    i++;
 	 }
       }
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index ad8b43365ad..17bde91bb2c 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -557,6 +557,8 @@ vec4_visitor::setup_uniform_values(int loc, const glsl_type *type)
 void
 vec4_visitor::setup_uniform_clipplane_values()
 {
+   gl_clip_plane *clip_planes = brw_select_clip_planes(ctx);
+
    int compacted_clipplane_index = 0;
    for (int i = 0; i < MAX_CLIP_PLANES; ++i) {
       if (ctx->Transform.ClipPlanesEnabled & (1 << i)) {
@@ -564,7 +566,7 @@ vec4_visitor::setup_uniform_clipplane_values()
          this->userplane[compacted_clipplane_index] = dst_reg(UNIFORM, this->uniforms);
          this->userplane[compacted_clipplane_index].type = BRW_REGISTER_TYPE_F;
          for (int j = 0; j < 4; ++j) {
-            c->prog_data.param[this->uniforms * 4 + j] = &ctx->Transform._ClipUserPlane[i][j];
+            c->prog_data.param[this->uniforms * 4 + j] = &clip_planes[i][j];
          }
          ++compacted_clipplane_index;
          ++this->uniforms;
@@ -1863,9 +1865,27 @@ vec4_visitor::emit_clip_distances(struct brw_reg reg, int offset)
       return;
    }
 
+   /* From the GLSL 1.30 spec, section 7.1 (Vertex Shader Special Variables):
+    *
+    *     "If a linked set of shaders forming the vertex stage contains no
+    *     static write to gl_ClipVertex or gl_ClipDistance, but the
+    *     application has requested clipping against user clip planes through
+    *     the API, then the coordinate written to gl_Position is used for
+    *     comparison against the user clip planes."
+    *
+    * This function is only called if the shader didn't write to
+    * gl_ClipDistance.  Accordingly, we use gl_ClipVertex to perform clipping
+    * if the user wrote to it; otherwise we use gl_Position.
+    */
+   gl_vert_result clip_vertex = VERT_RESULT_CLIP_VERTEX;
+   if (!(c->prog_data.outputs_written
+         & BITFIELD64_BIT(VERT_RESULT_CLIP_VERTEX))) {
+      clip_vertex = VERT_RESULT_HPOS;
+   }
+
    for (int i = 0; i + offset < c->key.nr_userclip && i < 4; ++i) {
       emit(DP4(dst_reg(brw_writemask(reg, 1 << i)),
-               src_reg(output_reg[VERT_RESULT_HPOS]),
+               src_reg(output_reg[clip_vertex]),
                src_reg(this->userplane[i + offset])));
    }
 }
diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
index 93c68381380..3a9b780382a 100644
--- a/src/mesa/drivers/dri/i965/brw_vs.c
+++ b/src/mesa/drivers/dri/i965/brw_vs.c
@@ -137,15 +137,47 @@ brw_compute_vue_map(struct brw_vue_map *vue_map,
    /* The hardware doesn't care about the rest of the vertex outputs, so just
     * assign them contiguously.  Don't reassign outputs that already have a
     * slot.
+    *
+    * Also, don't assign a slot for VERT_RESULT_CLIP_VERTEX, since it is
+    * unsupported in pre-GEN6, and in GEN6+ the vertex shader converts it into
+    * clip distances.
     */
    for (int i = 0; i < VERT_RESULT_MAX; ++i) {
       if ((outputs_written & BITFIELD64_BIT(i)) &&
-          vue_map->vert_result_to_slot[i] == -1) {
+          vue_map->vert_result_to_slot[i] == -1 &&
+          i != VERT_RESULT_CLIP_VERTEX) {
          assign_vue_slot(vue_map, i);
       }
    }
 }
 
+
+/**
+ * Decide which set of clip planes should be used when clipping via
+ * gl_Position or gl_ClipVertex.
+ */
+gl_clip_plane *brw_select_clip_planes(struct gl_context *ctx)
+{
+   if (ctx->Shader.CurrentVertexProgram) {
+      /* There is currently a GLSL vertex shader, so clip according to GLSL
+       * rules, which means compare gl_ClipVertex (or gl_Position, if
+       * gl_ClipVertex wasn't assigned) against the eye-coordinate clip planes
+       * that were stored in EyeUserPlane at the time the clip planes were
+       * specified.
+       */
+      return ctx->Transform.EyeUserPlane;
+   } else {
+      /* Either we are using fixed function or an ARB vertex program.  In
+       * either case the clip planes are going to be compared against
+       * gl_Position (which is in clip coordinates) so we have to clip using
+       * _ClipUserPlane, which was transformed into clip coordinates by Mesa
+       * core.
+       */
+      return ctx->Transform._ClipUserPlane;
+   }
+}
+
+
 static bool
 do_vs_prog(struct brw_context *brw,
 	   struct gl_shader_program *prog,
diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 0f6f6a7e062..14a96fca6b1 100644
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -77,9 +77,10 @@ gen6_prepare_vs_push_constants(struct brw_context *brw)
           * until we redo the VS backend.
           */
          if (!uses_clip_distance) {
+            gl_clip_plane *clip_planes = brw_select_clip_planes(ctx);
             for (i = 0; i < MAX_CLIP_PLANES; i++) {
                if (ctx->Transform.ClipPlanesEnabled & (1 << i)) {
-                  memcpy(param, ctx->Transform._ClipUserPlane[i], 4 * sizeof(float));
+                  memcpy(param, clip_planes[i], 4 * sizeof(float));
                   param += 4;
                   params_uploaded++;
                }
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 71cbe0443d9..4fe442fa408 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1493,13 +1493,20 @@ struct gl_texture_attrib
 
 
 /**
+ * Data structure representing a single clip plane (e.g. one of the elements
+ * of the ctx->Transform.EyeUserPlane or ctx->Transform._ClipUserPlane array).
+ */
+typedef GLfloat gl_clip_plane[4];
+
+
+/**
  * Transformation attribute group (GL_TRANSFORM_BIT).
  */
 struct gl_transform_attrib
 {
    GLenum MatrixMode;				/**< Matrix mode */
-   GLfloat EyeUserPlane[MAX_CLIP_PLANES][4];	/**< User clip planes */
-   GLfloat _ClipUserPlane[MAX_CLIP_PLANES][4];	/**< derived */
+   gl_clip_plane EyeUserPlane[MAX_CLIP_PLANES];	/**< User clip planes */
+   gl_clip_plane _ClipUserPlane[MAX_CLIP_PLANES]; /**< derived */
    GLbitfield ClipPlanesEnabled;                /**< on/off bitmask */
    GLboolean Normalize;				/**< Normalize all normals? */
    GLboolean RescaleNormals;			/**< GL_EXT_rescale_normal */