[Mesa-dev] [PATCH 4/4] mesa: Implement glGetFragDataLocation

[Kenneth Graunke]

On 11/04/2011 04:41 PM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Fixes piglit's getfragdatalocation test.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/mesa/main/shader_query.cpp |   56 ++++++++++++++++++++++++++++++++++++++++
>  src/mesa/main/shaderapi.c      |   20 --------------
>  2 files changed, 56 insertions(+), 20 deletions(-)
> 
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 23667a1..644916c 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -274,3 +274,59 @@ _mesa_BindFragDataLocation(GLuint program, GLuint colorNumber,
>      * glLinkProgram is called again.
>      */
>  }
> +
> +GLint GLAPIENTRY
> +_mesa_GetFragDataLocation(GLuint program, const GLchar *name)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   struct gl_shader_program *const shProg =
> +      _mesa_lookup_shader_program_err(ctx, program, "glGetFragDataLocation");
> +
> +   if (!shProg) {
> +      return -1;
> +   }
> +
> +   if (!shProg->LinkStatus) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "glGetFragDataLocation(program not linked)");
> +      return -1;
> +   }
> +
> +   if (!name)
> +      return -1;
> +
> +   if (strncmp(name, "gl_", 3) == 0) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "glGetFragDataLocation(illegal name)");
> +      return -1;
> +   }
> +
> +   /* Not having a fragment shader is not an error.
> +    */
> +   if (shProg->_LinkedShaders[MESA_SHADER_FRAGMENT] == NULL)
> +      return -1;
> +
> +   exec_list *ir = shProg->_LinkedShaders[MESA_SHADER_FRAGMENT]->ir;
> +   foreach_list(node, ir) {
> +      const ir_variable *const var = ((ir_instruction *) node)->as_variable();
> +
> +      /* The extra check against VERT_ATTRIB_GENERIC0 is because

Ditto Eric's comment...FRAG_RESULT_DATA0.

> +       * glGetFragDataLocation cannot be used on "conventional" attributes.
> +       *
> +       * From page 95 of the OpenGL 3.0 spec:
> +       *
> +       *     "If name is not an active attribute, if name is a conventional
> +       *     attribute, or if an error occurs, -1 will be returned."
> +       */

I don't think this is quite sufficient: according to the man page, it's
only legal to call glGetFragDataLocation on user-defined FS outputs.
But gl_FragData has location >= FRAG_RESULT_DATA0, doesn't it?

Maybe you need to strncmp(var->name, "gl_", 3) == 0 instead?

> +      if (var == NULL
> +	  || var->mode != ir_var_out
> +	  || var->location == -1
> +	  || var->location < FRAG_RESULT_DATA0)
> +	 continue;
> +
> +      if (strcmp(var->name, name) == 0)
> +	 return var->location - FRAG_RESULT_DATA0;
> +   }
> +
> +   return -1;
> +}
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 078f534..c766bd4 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -496,16 +496,6 @@ get_attached_shaders(struct gl_context *ctx, GLuint program, GLsizei maxCount,
>  }
>  
>  
> -static GLint
> -get_frag_data_location(struct gl_context *ctx, GLuint program,
> -                       const GLchar *name)
> -{
> -   _mesa_problem(ctx, "get_frag_data_location() not implemented yet");
> -   return -1;
> -}
> -
> -
> -
>  /**
>   * glGetHandleARB() - return ID/name of currently bound shader program.
>   */
> @@ -1167,16 +1157,6 @@ _mesa_GetAttachedShaders(GLuint program, GLsizei maxCount,
>  }
>  
>  
> -/* GL_EXT_gpu_shader4, GL3 */
> -GLint GLAPIENTRY
> -_mesa_GetFragDataLocation(GLuint program, const GLchar *name)
> -{
> -   GET_CURRENT_CONTEXT(ctx);
> -   return get_frag_data_location(ctx, program, name);
> -}
> -
> -
> -
>  void GLAPIENTRY
>  _mesa_GetInfoLogARB(GLhandleARB object, GLsizei maxLength, GLsizei * length,
>                      GLcharARB * infoLog)