[Mesa-dev] [PATCH v2 1/2] mesa/es3.1: Expose GL_ARB_framebuffer_no_attachments to GLES 3.1

[Ilia Mirkin]

On Wed, Aug 26, 2015 at 1:34 AM, Tapani Pälli <tapani.palli at 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.lofstedt at 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