[Mesa-dev] [PATCH v4] mesa: enable enums for OES_geometry_shader

[Lofstedt, Marta]

> -----Original Message-----
> From: ibmirkin at gmail.com [mailto:ibmirkin at gmail.com] On Behalf Of Ilia
> Mirkin
> Sent: Thursday, January 21, 2016 2:32 PM
> To: Lofstedt, Marta
> Cc: Marta Lofstedt; mesa-dev at lists.freedesktop.org
> Subject: Re: [PATCH v4] mesa: enable enums for OES_geometry_shader
> 
> On Thu, Jan 21, 2016 at 8:28 AM, Lofstedt, Marta <marta.lofstedt at intel.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: ibmirkin at gmail.com [mailto:ibmirkin at gmail.com] On Behalf Of
> >> Ilia Mirkin
> >> Sent: Thursday, January 21, 2016 2:03 PM
> >> To: Lofstedt, Marta
> >> Cc: Marta Lofstedt; mesa-dev at lists.freedesktop.org
> >> Subject: Re: [PATCH v4] mesa: enable enums for OES_geometry_shader
> >>
> >> On Thu, Jan 21, 2016 at 7:57 AM, Lofstedt, Marta
> >> <marta.lofstedt at intel.com>
> >> wrote:
> >> >> -----Original Message-----
> >> >> From: ibmirkin at gmail.com [mailto:ibmirkin at gmail.com] On Behalf Of
> >> >> Ilia Mirkin
> >> >> Sent: Thursday, January 21, 2016 1:46 PM
> >> >> To: Lofstedt, Marta
> >> >> Cc: Marta Lofstedt; mesa-dev at lists.freedesktop.org
> >> >> Subject: Re: [PATCH v4] mesa: enable enums for
> OES_geometry_shader
> >> >>
> >> >> On Thu, Jan 21, 2016 at 7:41 AM, Lofstedt, Marta
> >> >> <marta.lofstedt at intel.com>
> >> >> wrote:
> >> >> >> -----Original Message-----
> >> >> >> From: ibmirkin at gmail.com [mailto:ibmirkin at gmail.com] On Behalf
> >> >> >> Of Ilia Mirkin
> >> >> >> Sent: Thursday, January 21, 2016 1:25 PM
> >> >> >> To: Marta Lofstedt
> >> >> >> Cc: mesa-dev at lists.freedesktop.org; Lofstedt, Marta
> >> >> >> Subject: Re: [PATCH v4] mesa: enable enums for
> >> OES_geometry_shader
> >> >> >>
> >> >> >> On Thu, Dec 10, 2015 at 11:11 AM, Marta Lofstedt
> >> >> >> <marta.lofstedt at linux.intel.com> wrote:
> >> >> >> > +      case EXTRA_EXT_GPU5_GS:
> >> >> >> > +         api_check = GL_TRUE;
> >> >> >> > +         api_found = (ctx->Extensions.ARB_gpu_shader5 ||
> >> >> >> > +                      _mesa_has_OES_geometry_shader(ctx));
> >> >> >> > +         break;
> >> >> >> > +      case EXTRA_EXT_VIEWPORT_GS:
> >> >> >> > +         api_check = GL_TRUE;
> >> >> >> > +         api_found = (ctx->Extensions.ARB_viewport_array ||
> >> >> >> > +                      _mesa_has_OES_geometry_shader(ctx));
> >> >> >> > +         break;
> >> >> >>
> >> >> >> You can do these without the special tokens. Or did you mean &&
> >> here?
> >> >> >
> >> >> > I am pretty sure that our previous discussions on this topic
> >> >> > ended up with
> >> >> || to be preferable in these cases, but if you want && I will change.
> >> >>
> >> >> I actually don't want either. What I'm saying is that if you want
> >> >> ||, then you don't have to add these EXTRA_EXT_GPU5_GS things --
> >> >> using the regular mechanism for composing tokens will get you ||.
> >> >> You only need to use these special tokens if you want &&.
> >> >>
> >> > If by the "regular" mechanism mean:
> >> > +static const int extra_ARB_viewport_array_or_geometry_shader[] = {
> >> > +   EXT(ARB_viewport_array),
> >> > +   EXT(OES_geometry_shader),
> >> > +   EXTRA_END
> >> > +};
> >> > I had that in an earlier patch, where I interpreted your comment as
> >> > a
> >> rejection.
> >>
> >> Nope, that's precisely what I mean. The only distinction between the
> >> two is the extra check for the ES context. If you want to preserve
> >> that, just create a single EXTRA_EXT_ES_GS token which just does
> >>
> >> api_found = _mesa_has_OES_geometry_shader(ctx)
> >>
> >> and then you can use that instead of EXT(OES_geometry_shader). The
> >> distinction is pretty minor, of course. Exposing one or two enums
> >> that you're supposed to error on is probably not the end of the world.
> >> Either way is good with me though.
> >>
> > So, to check if we understand each other. Would it be OK to you, if I
> replace:
> > EXTRA_EXT_VIEWPORT_GS and EXTRA_EXT_GPU5_GS, both in
> value_extra and
> > the check_extra(...) switch,
> >
> > with: EXTRA_EXT_ES_GS  in value_extra and according to your suggestion
> > above in check_extra(...)
> >
> > And use EXTRA_EXT_ES_GS instead of EXT(OES_geometry_shader) in:
> > extra_ARB_viewport_array_or_geometry_shader[] and similar to:
> > extra_ARB_gpu_shader5_and_geometry_shader []
> 
> Yep, that's precisely my suggestion. I think we understand each other
> perfectly.
>
That's good. I now have this on my github:
https://github.com/MartaLo/mesa/tree/oes_geometry_shader_Ilia_enums

> 
> I hope you're not seeing this as random busy-work -- if you are, please speak
> up -- but I think it's best to minimize the number of the custom tokens to the
> ones that we *actually* need. Otherwise there's too many ways to do the
> same thing, and the getter logic doesn't need any more of those!
>
>
> Cheers,
> 
>   -ilia