[Mesa-dev] [PATCH] mesa: Fix glGetActiveAttribute for gl_VertexID when lowered.

[Ian Romanick]

On 08/09/2014 10:54 AM, Kenneth Graunke wrote:
> The lower_vertex_id pass converts uses of the gl_VertexID system value
> to the gl_BaseVertex and gl_VertexIDMESA system values.  Since
> gl_VertexID is no longer accessed, it would not be considered active.
> 
> Of course, it should be, since the shader uses gl_VertexID.
> 
> v2: Move the var->name dereference past the var != NULL check.

Right... which I didn't notice before because the var != NULL check is
hidden in is_active_attrib.  D'oh.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/main/shader_query.cpp | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> [15:31] <anujp> idr: gles3conform-v5-wip is not doing well. getactiveattribute_index_more_than_num_attribs test segfaults in src/mesa/main/shader_query.cpp:132. You can reproduce it on IVB too.
> [15:43] <idr> aw crap.
> [15:43] <idr> anujp: I guess Kayden's series needs a bit more work. :(
> 
> This fixes that test.  I haven't run other tests on v2 of this, but
> I don't really think I need to.  I literally just moved the var_name
> declaration down a bit.
> 
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 4871d09..766ad29 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -93,6 +93,7 @@ is_active_attrib(const ir_variable *var)
>         * and gl_InstanceID."
>         */
>        return var->data.location == SYSTEM_VALUE_VERTEX_ID ||
> +             var->data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE ||
>               var->data.location == SYSTEM_VALUE_INSTANCE_ID;
>  
>     default:
> @@ -133,7 +134,18 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index,
>           continue;
>  
>        if (current_index == desired_index) {
> -	 _mesa_copy_string(name, maxLength, length, var->name);
> +         const char *var_name = var->name;
> +
> +         /* Since gl_VertexID may be lowered to gl_VertexIDMESA, we need to
> +          * consider gl_VertexIDMESA as gl_VertexID for purposes of checking
> +          * active attributes.
> +          */
> +         if (var->data.mode == ir_var_system_value &&
> +             var->data.location == SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) {
> +            var_name = "gl_VertexID";
> +         }
> +
> +	 _mesa_copy_string(name, maxLength, length, var_name);
>  
>  	 if (size)
>  	    *size = (var->type->is_array()) ? var->type->length : 1;
>