On Thu, Jan 21, 2016 at 9:14 AM, Lofstedt, Marta <[email protected]> wrote: >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] On Behalf Of Ilia >> Mirkin >> Sent: Thursday, January 21, 2016 2:32 PM >> To: Lofstedt, Marta >> Cc: Marta Lofstedt; [email protected] >> Subject: Re: [PATCH v4] mesa: enable enums for OES_geometry_shader >> >> On Thu, Jan 21, 2016 at 8:28 AM, Lofstedt, Marta <[email protected]> >> wrote: >> > >> > >> >> -----Original Message----- >> >> From: [email protected] [mailto:[email protected]] On Behalf Of >> >> Ilia Mirkin >> >> Sent: Thursday, January 21, 2016 2:03 PM >> >> To: Lofstedt, Marta >> >> Cc: Marta Lofstedt; [email protected] >> >> Subject: Re: [PATCH v4] mesa: enable enums for OES_geometry_shader >> >> >> >> On Thu, Jan 21, 2016 at 7:57 AM, Lofstedt, Marta >> >> <[email protected]> >> >> wrote: >> >> >> -----Original Message----- >> >> >> From: [email protected] [mailto:[email protected]] On Behalf Of >> >> >> Ilia Mirkin >> >> >> Sent: Thursday, January 21, 2016 1:46 PM >> >> >> To: Lofstedt, Marta >> >> >> Cc: Marta Lofstedt; [email protected] >> >> >> Subject: Re: [PATCH v4] mesa: enable enums for >> OES_geometry_shader >> >> >> >> >> >> On Thu, Jan 21, 2016 at 7:41 AM, Lofstedt, Marta >> >> >> <[email protected]> >> >> >> wrote: >> >> >> >> -----Original Message----- >> >> >> >> From: [email protected] [mailto:[email protected]] On Behalf >> >> >> >> Of Ilia Mirkin >> >> >> >> Sent: Thursday, January 21, 2016 1:25 PM >> >> >> >> To: Marta Lofstedt >> >> >> >> Cc: [email protected]; Lofstedt, Marta >> >> >> >> Subject: Re: [PATCH v4] mesa: enable enums for >> >> OES_geometry_shader >> >> >> >> >> >> >> >> On Thu, Dec 10, 2015 at 11:11 AM, Marta Lofstedt >> >> >> >> <[email protected]> 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
That part looks good. I noticed you also have + ["MAX_FRAMEBUFFER_LAYERS", "CONTEXT_INT(Const.MaxFramebufferLayers), extra_ARB_framebuffer_no_attachments_and_geometry_shader"], which is missing a space after [ and before ]. And I think in one of your other patches, you were missing a space between if and (). In all, I think this would benefit from having the actual patches being resent to list, along with a note not unlike in your earlier email which is that "these have been previously reviewed but want to give people some time to provide more feedback, will push in X time unless I hear otherwise". Cheers, -ilia _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
