On Wed, Aug 26, 2015 at 1:34 AM, Tapani Pälli <tapani.pa...@intel.com> wrote: > On 08/24/2015 05:04 PM, Ilia Mirkin wrote: >> >> On Mon, Aug 24, 2015 at 9:59 AM, Lofstedt, Marta >> <marta.lofst...@intel.com> wrote: >>>>> +static const int extra_ARB_framebuffer_no_attachments_es31[] = { >>>>> + EXT(ARB_framebuffer_no_attachments), >>>>> + EXTRA_API_ES31, >>>>> + EXTRA_END >>>>> +}; >>>> >>>> What does this add? When would you have ES31 but not >>>> ARB_framebuffer_no_attachments? >>> >>> As far as I understand, I am following what appear to be the established >>> template on how to add GLES 3.1 stuff that previously has only been exposed >>> under OpenGL. >> >> OK, but ... let's look at what this is actually doing. This is >> creating a helper which will roughly translate in the code to mean >> >> if (ARB_framebuffer_no_attachments || es31) >> >> (see get.c:check_extra). Now, this would make a ton of sense if es31 >> could be enabled independently of ARB_framebuffer_no_attachments. >> However, I don't think that's the case -- it's one of the prereqs for >> es31 (look at compute_version_es2 -- it's commented out now, but IIRC >> you uncomment it later in the series). >> >> I think that many of the existing things you're using as a template >> are wrong too, but that's no reason to continue the wrongness. > > > Technically there's nothing wrong here though, the check is absolutely > correct. Either you have ES3.1 driver or you possibly have desktop only > driver supporting this particular extension (but no ES 3.1 support). If we > now start to use multiple ways I'm not sure it will be as generic?
Sure, there's nothing *wrong* with adding || 1, but... why do it? You can't have an ES3.1 driver that doesn't set Extensions.ARB_framebuffer_no_attachments... it's (going to be) required by compute_version_es2. You could have a driver that doesn't support that ext, nor ES3.1, and a user force-enables ES3.1 on the cmdline, but I don't think that's worth worrying about here -- i965 supports the ext, and any driver that doesn't isn't going to gain much from exposing these enums in a forced es31 context. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev