[Mesa-dev] [PATCH v2 1/2] mesa: Add a gl_vert_result for gl_ClipVertex.

[Ian Romanick]

On 10/03/2011 02:17 PM, Paul Berry wrote:
> Before this patch, clip planes didn't work properly in Mesa when using
> vertex shaders, because Mesa assigned both gl_ClipVertex and
> gl_Position to the same gl_vert_result (VERT_RESULT_HPOS).  As a
> result, backends couldn't distinguish between the two variables, so
> any shader that wrote different values to them would fail to work
> properly.
>
> This patch paves the way for proper support of gl_ClipVertex by
> creating a new enumerated value in gl_vert_result for it
> (VERT_RESULT_CLIP_VERTEX).  After this patch, a back-end may add

What happens in drivers that aren't expecting / don't know about 
VERT_RESULT_CLIP_VERTEX?  In other words, does this break (more) i915 
and all Gallium drivers?  I understand that gl_ClipVertex already 
doesn't work anywhere, but it would be a shame to replace incorrect 
rendering with a crash.

I can test out (and patch up) i915 today, but I'd like some feedback 
from people that rely on TGSI.

Other than that (and the GLclipplane name change suggested by Chad), 
both gl_ClipVertex / gl_ClipDistance series look good to me.

> support for gl_ClipVertex using the following algorithm:
>
> - If using a user-supplied GLSL vertex shader:
>    - If the bit corresponding to VERT_RESULT_CLIP_VERTEX is set in
>      gl_program::OutputsWritten:
>      - Clip using the vertex shader output VERT_RESULT_CLIP_VERTEX and
>        the clip planes defined in gl_context::Transform.EyeUserPlane.
>    - Else:
>      - Clip using the vertex shader output VERT_RESULT_HPOS and the
>        clip planes defined in gl_context::Transform.EyeUserPlane.
> - Else (either using fixed function or an ARB vertex program):
>    - Clip using the vertex shader output VERT_RESULT_HPOS and the clip
>      planes defined in gl_context::Transform._ClipUserPlane (*)
>
> where (*) represents the normal Mesa behavior before this patch.
>
> An example of implementing the above algorithm can be found in the
> patch that follows this one, which implements gl_ClipVertex in i965
> Gen6.
> ---
>   src/glsl/builtin_variables.h |   38 +++++++++++++++++++-------------------
>   src/mesa/main/mtypes.h       |    7 ++++---
>   2 files changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/src/glsl/builtin_variables.h b/src/glsl/builtin_variables.h
> index 9b4f5d9..d25bbf4 100644
> --- a/src/glsl/builtin_variables.h
> +++ b/src/glsl/builtin_variables.h
> @@ -56,25 +56,25 @@ static const builtin_variable builtin_110_deprecated_fs_variables[] = {
>   };
>
>   static const builtin_variable builtin_110_deprecated_vs_variables[] = {
> -   { ir_var_in,  VERT_ATTRIB_POS,    "vec4",  "gl_Vertex" },
> -   { ir_var_in,  VERT_ATTRIB_NORMAL, "vec3",  "gl_Normal" },
> -   { ir_var_in,  VERT_ATTRIB_COLOR0, "vec4",  "gl_Color" },
> -   { ir_var_in,  VERT_ATTRIB_COLOR1, "vec4",  "gl_SecondaryColor" },
> -   { ir_var_in,  VERT_ATTRIB_TEX0,   "vec4",  "gl_MultiTexCoord0" },
> -   { ir_var_in,  VERT_ATTRIB_TEX1,   "vec4",  "gl_MultiTexCoord1" },
> -   { ir_var_in,  VERT_ATTRIB_TEX2,   "vec4",  "gl_MultiTexCoord2" },
> -   { ir_var_in,  VERT_ATTRIB_TEX3,   "vec4",  "gl_MultiTexCoord3" },
> -   { ir_var_in,  VERT_ATTRIB_TEX4,   "vec4",  "gl_MultiTexCoord4" },
> -   { ir_var_in,  VERT_ATTRIB_TEX5,   "vec4",  "gl_MultiTexCoord5" },
> -   { ir_var_in,  VERT_ATTRIB_TEX6,   "vec4",  "gl_MultiTexCoord6" },
> -   { ir_var_in,  VERT_ATTRIB_TEX7,   "vec4",  "gl_MultiTexCoord7" },
> -   { ir_var_in,  VERT_ATTRIB_FOG,    "float", "gl_FogCoord" },
> -   { ir_var_out, VERT_RESULT_HPOS,   "vec4",  "gl_ClipVertex" },
> -   { ir_var_out, VERT_RESULT_COL0,   "vec4",  "gl_FrontColor" },
> -   { ir_var_out, VERT_RESULT_BFC0,   "vec4",  "gl_BackColor" },
> -   { ir_var_out, VERT_RESULT_COL1,   "vec4",  "gl_FrontSecondaryColor" },
> -   { ir_var_out, VERT_RESULT_BFC1,   "vec4",  "gl_BackSecondaryColor" },
> -   { ir_var_out, VERT_RESULT_FOGC,   "float", "gl_FogFragCoord" },
> +   { ir_var_in,  VERT_ATTRIB_POS,         "vec4",  "gl_Vertex" },
> +   { ir_var_in,  VERT_ATTRIB_NORMAL,      "vec3",  "gl_Normal" },
> +   { ir_var_in,  VERT_ATTRIB_COLOR0,      "vec4",  "gl_Color" },
> +   { ir_var_in,  VERT_ATTRIB_COLOR1,      "vec4",  "gl_SecondaryColor" },
> +   { ir_var_in,  VERT_ATTRIB_TEX0,        "vec4",  "gl_MultiTexCoord0" },
> +   { ir_var_in,  VERT_ATTRIB_TEX1,        "vec4",  "gl_MultiTexCoord1" },
> +   { ir_var_in,  VERT_ATTRIB_TEX2,        "vec4",  "gl_MultiTexCoord2" },
> +   { ir_var_in,  VERT_ATTRIB_TEX3,        "vec4",  "gl_MultiTexCoord3" },
> +   { ir_var_in,  VERT_ATTRIB_TEX4,        "vec4",  "gl_MultiTexCoord4" },
> +   { ir_var_in,  VERT_ATTRIB_TEX5,        "vec4",  "gl_MultiTexCoord5" },
> +   { ir_var_in,  VERT_ATTRIB_TEX6,        "vec4",  "gl_MultiTexCoord6" },
> +   { ir_var_in,  VERT_ATTRIB_TEX7,        "vec4",  "gl_MultiTexCoord7" },
> +   { ir_var_in,  VERT_ATTRIB_FOG,         "float", "gl_FogCoord" },
> +   { ir_var_out, VERT_RESULT_CLIP_VERTEX, "vec4",  "gl_ClipVertex" },
> +   { ir_var_out, VERT_RESULT_COL0,        "vec4",  "gl_FrontColor" },
> +   { ir_var_out, VERT_RESULT_BFC0,        "vec4",  "gl_BackColor" },
> +   { ir_var_out, VERT_RESULT_COL1,        "vec4",  "gl_FrontSecondaryColor" },
> +   { ir_var_out, VERT_RESULT_BFC1,        "vec4",  "gl_BackSecondaryColor" },
> +   { ir_var_out, VERT_RESULT_FOGC,        "float", "gl_FogFragCoord" },
>   };
>
>   static const builtin_variable builtin_120_fs_variables[] = {
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 8c39717..ab03d9a 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -214,9 +214,10 @@ typedef enum
>      VERT_RESULT_BFC0 = 13,
>      VERT_RESULT_BFC1 = 14,
>      VERT_RESULT_EDGE = 15,
> -   VERT_RESULT_CLIP_DIST0 = 16,
> -   VERT_RESULT_CLIP_DIST1 = 17,
> -   VERT_RESULT_VAR0 = 18,  /**<  shader varying */
> +   VERT_RESULT_CLIP_VERTEX = 16,
> +   VERT_RESULT_CLIP_DIST0 = 17,
> +   VERT_RESULT_CLIP_DIST1 = 18,
> +   VERT_RESULT_VAR0 = 19,  /**<  shader varying */
>      VERT_RESULT_MAX = (VERT_RESULT_VAR0 + MAX_VARYING)
>   } gl_vert_result;
>