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

[Kenneth Graunke]

On 11/08/2011 08:10 PM, Kenneth Graunke wrote:
> 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?

Durr...I can read.  You already checked that above.

I assert that all FS outputs with var->location < FRAG_RESULT_DATA0 have
names that begin with "gl_".  So, this check is just redundant, and you
can drop it.  Right?

After all, the only "conventional" attributes are gl_FragColor,
gl_FragData, and gl_FragDepth.

>> +      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)
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev