[Mesa-dev] [PATCH] tgsi/nouveau: Add support for tesselation ctrl and tesselation eval

[Ilia Mirkin]

On Fri, Aug 14, 2015 at 12:43 AM, Marcos Souza
<marcos.souza.org at gmail.com> wrote:
> HI Ilia
>
> 2015-08-14 1:31 GMT-03:00 Ilia Mirkin <imirkin at alum.mit.edu>:
>>
>> On Fri, Aug 14, 2015 at 12:25 AM, Marcos Souza
>> <marcos.souza.org at gmail.com> wrote:
>> > Hi Ilia,
>> >
>> > 2015-08-14 1:02 GMT-03:00 Ilia Mirkin <imirkin at alum.mit.edu>:
>> >>
>> >> On Thu, Aug 13, 2015 at 11:55 PM, Marcos Souza
>> >> <marcos.souza.org at gmail.com> wrote:
>> >> > Hi Ilia,
>> >> >
>> >> > So i found the point here it addrs that double brackets, and the
>> >> > following
>> >> > patch solves it, but this is a right solution? If someone could guide
>> >> > me
>> >> > here, I could fix it :)
>> >> >
>> >> > diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> >> > b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> >> > index 8ceb5b4..046471e 100644
>> >> > --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> >> > +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> >> > @@ -302,10 +302,14 @@ iter_declaration(
>> >> >        TXT("[]");
>> >> >     }
>> >> >
>> >> > -   if (decl->Declaration.Dimension) {
>> >>
>> >> The issue is that the declaration is getting a dimension set by the
>> >> parser, which in turn is causing it to print funny. It shouldn't be
>> >> getting a dimension in the first place for those.
>> >
>> >
>> > The following patch fix the problem, is it the right place to put it?
>>
>> I don't think so. Just glanced at the code, look at
>>
>> parse_register_dcl
>>
>>       /* for geometry shader we don't really care about
>>        * the first brackets it's always the size of the
>>        * input primitive. so we want to declare just
>>        * the index relevant to the semantics which is in
>>        * the second bracket */
>>       if (ctx->processor == TGSI_PROCESSOR_GEOMETRY && *file ==
>> TGSI_FILE_INPUT) {
>>          brackets[0] = brackets[1];
>>          *num_brackets = 1;
>>       }
>>
>> Basically you need to extend this logic to similarly exclude
>>
>> (a) tess ctrl inputs and outputs
>> (b) tess eval inputs
>>
>> Technically you need to exclude patch/tessinner/tessouter from that,
>> but in practice they won't have an extra set of brackets either.
>>
>
> Sorry for flooding the list, but I'm relaly excited about it :)
>
> So, this is the change you asked. It also solved the problem:
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c
> b/src/gallium/auxiliary/tgsi/tgsi_text.c
> index 8647e4e..95c1daf 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
> @@ -684,7 +684,12 @@ parse_register_dcl(
>         * input primitive. so we want to declare just
>         * the index relevant to the semantics which is in
>         * the second bracket */
> -      if (ctx->processor == TGSI_PROCESSOR_GEOMETRY && *file ==
> TGSI_FILE_INPUT) {
> +
> +      /* similarly from tessalation */

tessellation

> +      int exclude = (ctx->processor == TGSI_PROCESSOR_TESS_EVAL && *file ==
> TGSI_FILE_INPUT) ||
> +          (ctx->processor == TGSI_PROCESSOR_TESS_CTRL && (*file ==
> TGSI_FILE_INPUT ||
> +             *file == TGSI_FILE_OUTPUT));

Why is this separate from the geometry thing?

> +      if ((ctx->processor == TGSI_PROCESSOR_GEOMETRY && *file ==
> TGSI_FILE_INPUT) || exclude) {
>           brackets[0] = brackets[1];
>           *num_brackets = 1;
>        } else {
>
> What do you think Ilia?

Generally sounds good.

  -ilia