[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: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 []


>
> I doubt I flat-out rejected the quoted approach earlier, but I may have
> commented about how it's not the perfect thing :)
> 
>   -ilia