Hi Paul,
I won't comment on the patch 1, because I didn't write it, I was just
preserving history.
On Fri, Nov 4, 2011 at 10:00 PM, Paul Berry <stereotype...@gmail.com> wrote:
[snip]
>> +bool parse_tfeedback_decl(const void *mem_ctx, const char *input,
>> + struct tfeedback_decl *decl)
>> +{
>> + /* We don't have to be pedantic about what is a valid GLSL variable
>> name,
>> + * because any variable with an invalid name can't exist in the IR
>> anyway.
>> + */
>> +
>> + const char *bracket = strrchr(input, '[');
>> +
>> + if (bracket) {
>> + decl->name = ralloc_strndup(mem_ctx, input, bracket - input);
>> + if (sscanf(bracket, "[%u]", &decl->array_index) == 1) {
>> + decl->is_array = true;
>> + return true; /* Found: "var[i]" */
>> + }
>> + } else {
>> + decl->name = ralloc_strdup(mem_ctx, input);
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>
> For non-arrays, this function doesn't assign to decl->is_array or
> decl->array_index. That's a problem, because its caller
> (ir_set_transform_feedback_outs::ir_set_transform_feedback_outs()) passes
> uninitialized memory for decl. We could fix the problem by having
> ir_set_transform_feedback_outs::ir_set_transform_feedback_outs() initialize
> the memory first, but I would prefer if we fixed the problem by always
> assigning to all fields of decl, since we're effectively treating it as an
> output of this function.
Yeah, that was unintentional.
Regarding the other comments, I don't know the GLSL compiler so well.
I had forked ir_set_program_inouts and was basing upon that. Now that
I think about it, it makes more sense to do it like you say. :)
Concerning this:
varying mat4 r;
glTransformFeedbackVaryingsEXT(.. "r[1]" ..);
The EXT spec doesn't state whether this is legal. It says nothing
about matrices, actually.
[snip]
>
> The patch series doesn't add any code that calls this function. Is that
> planned for a future patch?
>
The function is already used here:
http://cgit.freedesktop.org/~mareko/mesa/log/?h=transform-feedback
It's a work in progress, but EXT_transform_feedback already works
quite well and ARB_transform_feedback2 too except
glDrawTransformFeedback, which isn't fully implemented yet. Despite
your remarks, the GLSL linker works pretty good for me, so I am going
to take a break from it for a while.
Feel free to take over all 3 patches and implement it properly, as I
might not have time to get to it again soon.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
