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

[Tapani Pälli]

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:
>>> -----Original Message-----
>>> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
>>> Behalf Of Ilia Mirkin
>>> Sent: Monday, August 24, 2015 3:02 PM
>>> To: Marta Lofstedt
>>> Cc: mesa-dev at lists.freedesktop.org
>>> Subject: Re: [Mesa-dev] [PATCH v2 1/2] mesa/es3.1: Expose
>>> GL_ARB_framebuffer_no_attachments to GLES 3.1
>>>
>>> On Mon, Aug 24, 2015 at 7:01 AM, Marta Lofstedt
>>> <marta.lofstedt at linux.intel.com> wrote:
>>>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>>>
>>>> Signed-off-by: Marta Lofstedt <marta.lofstedt at intel.com>
>>>> ---
>>>>   src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml | 4 ++--
>>>>   src/mapi/glapi/gen/apiexec.py                         | 4 ++--
>>>>   src/mesa/main/get.c                                   | 6 ++++++
>>>>   src/mesa/main/get_hash_params.py                      | 8 +++++---
>>>>   src/mesa/main/tests/dispatch_sanity.cpp               | 6 ++----
>>>>   src/mesa/main/version.c                               | 2 +-
>>>>   6 files changed, 18 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
>>>> b/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
>>>> index 59839a0..55ad764 100644
>>>> --- a/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
>>>> +++ b/src/mapi/glapi/gen/ARB_framebuffer_no_attachments.xml
>>>> @@ -15,13 +15,13 @@
>>>>      <enum name="MAX_FRAMEBUFFER_LAYERS"                     value="0x9317"
>>> />
>>>>      <enum name="MAX_FRAMEBUFFER_SAMPLES"
>>> value="0x9318" />
>>>> -    <function name="FramebufferParameteri">
>>>> +    <function name="FramebufferParameteri" es2="3.1">
>>>>          <param name="target" type="GLenum"/>
>>>>          <param name="pname"  type="GLenum"/>
>>>>          <param name="param"  type="GLint" />
>>>>       </function>
>>>>
>>>> -    <function name="GetFramebufferParameteriv">
>>>> +    <function name="GetFramebufferParameteriv" es2="3.1">
>>>>          <param name="target" type="GLenum" />
>>>>          <param name="pname"  type="GLenum" />
>>>>          <param name="params" type="GLint *" output="true" /> diff
>>>> --git a/src/mapi/glapi/gen/apiexec.py b/src/mapi/glapi/gen/apiexec.py
>>>> index 3a0eb18..58ec08b 100644
>>>> --- a/src/mapi/glapi/gen/apiexec.py
>>>> +++ b/src/mapi/glapi/gen/apiexec.py
>>>> @@ -151,8 +151,8 @@ functions = {
>>>>
>>>>       # OpenGL 4.3 / GL_ARB_framebuffer_no_attachments.  Mesa can
>>> expose the
>>>>       # extension with OpenGL 3.0.
>>>> -    "FramebufferParameteri": exec_info(compatibility=30, core=31),
>>>> -    "GetFramebufferParameteri": exec_info(compatibility=30, core=31),
>>>> +    "FramebufferParameteri": exec_info(compatibility=30, core=31,
>>> es2=31),
>>>> +    "GetFramebufferParameteri": exec_info(compatibility=30, core=31,
>>>> + es2=31),
>>>>
>>>>       # OpenGL 4.5 / GL_ARB_direct_state_access.   Mesa can expose the
>>> extension
>>>>       # with core profile.
>>>> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index
>>>> 307a5ff..8542ddb 100644
>>>> --- a/src/mesa/main/get.c
>>>> +++ b/src/mesa/main/get.c
>>>> @@ -403,6 +403,12 @@ static const int
>>> extra_ARB_explicit_uniform_location_es31[] = {
>>>>      EXTRA_END
>>>>   };
>>>>
>>>> +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?

// Tapani