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.roman...@intel.com>
>>
>> Fixes piglit's getfragdatalocation test.
>>
>> Signed-off-by: Ian Romanick <ian.d.roman...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
