[Mesa-dev] [PATCH v4 3/6] mesa/es3.1: enable GL_ARB_texture_multisample for GLES 3.1

[Tapani Pälli]

On 06/26/2015 05:38 PM, Ilia Mirkin wrote:
> On Fri, Jun 26, 2015 at 4:18 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>>
>>
>> On 06/26/2015 01:06 AM, Ilia Mirkin wrote:
>>>
>>> On Thu, Jun 25, 2015 at 4:22 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>
>>>> On Thu, Jun 25, 2015 at 5:08 AM, Marta Lofstedt
>>>> <marta.lofstedt at linux.intel.com> wrote:
>>>>>
>>>>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>>>>
>>>>> v4 : only expose GL_ARB_texture_multisample enums
>>>>> for gles 3.1 and desktop GL.
>>>>
>>>>
>>>> I was suspicious of this logic. Based on my reading of the code, what
>>>> your ARB_texture_multisample_es31 thing does is expose those enums
>>>> when *either* the driver enables ARB_texture_multisample *or* the
>>>> current context is ES3.1.
>>>>
>>>> ARB_draw_indirect_es31 has the same problem, btw.
>>
>>
>> I'm not sure I understand the issue. It should work fine with 'OR', if we
>> have either of these then we expose the enum?
>>
>>>> I could have misread the get.c extra_ext() logic, but I don't think I
>>>> have. As far as I can tell there's no way to (generically) AND these
>>>> things.
>>
>>
>> Why do we need AND? If you have 3.1 context then you *need* to support this
>> enum. There is no such condition where you would have gles 3.1 context but
>> not the enum, right?
>>
>>
>>>> What you really need to do is create a whole new [GL, GL_CORE, ES31]
>>>> section in get_hash_params and update get_hash_generator.py
>>>> accordingly.
>>>
>>>
>>> An alternative, BTW, is to just say "screw it" and expose the enums in
>>> GL ES 3.0. In that case, just move the enums into a section that
>>> includes GLES3. I'm not sure how big of a deal it is. But your current
>>> patches are definitely wrong.
>>
>>
>> We do this, but we have explicit version check against version 3.1.
>
> OK, well, at the end of the day, the things anyone says don't matter,
> it just has to work, right? So here are the scenarios:
>
> Let's take it as a given that the driver sets
> Ext.ARB_texture_multisample to true, we have the following situations:
>
> GL compat glGet(SAMPLE_MASK)
> GL core glGet(SAMPLE_MASK)
> GL ES3.0 glGet(SAMPLE_MASK)
> GL ES3.1 glGet(SAMPLE_MASK)
>
> Where the ES3.0 one should fail, and all the others should succeed.
> OK, so first off, this is in the GL/GL_CORE/GLES3 section, which means
> that the logic will get hit for all 4, right? Then you do a check for
>
> Ext.ARB_texture_multisample || ES3.1
>
> and if that doesn't get set to true, then you return INVALID_ENUM,
> right? So tell me, given that the driver *always* sets
> Ext.ARB_texture_multisample to true, how can that ever be false? The
> ES3.1 thing could be useful for drivers that don't actually support
> ARB_texture_multisample and you want to fake it, but in practice all
> the relevant drivers support it. But none of that cause a
> GL_INVALID_ENUM to get returned in ES3.0 as it was before the patch.
>
> Hope that clears things up.

Right .. now I got it. Everything seemed fine but this is just because 
we don't have negative tests for getting enums. Going this way we would 
even expose these enums to GLES 2.0 apps. We need to create a new 
section with ES31, there is no way out of it. It kind of sucks to do 
this for minor versions but that is the only way to avoid exposing enums 
for older versions.

Ilia, thanks for the patience;

// Tapani