From d912669034eb7bf5c162358a7a574ec7a4c963c7 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 26 Sep 2011 13:43:04 -0700 Subject: [PATCH] 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 Reviewed-by: Chad Versace --- src/mesa/drivers/dri/i965/brw_context.h | 3 ++ src/mesa/drivers/dri/i965/brw_curbe.c | 10 +++--- .../drivers/dri/i965/brw_vec4_visitor.cpp | 24 +++++++++++-- src/mesa/drivers/dri/i965/brw_vs.c | 34 ++++++++++++++++++- src/mesa/drivers/dri/i965/gen6_vs_state.c | 3 +- src/mesa/main/mtypes.h | 11 ++++-- 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<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 @@ -1492,14 +1492,21 @@ 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 */ -- 2.30.2