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

[Ian Romanick]

On 06/26/2015 07:38 AM, 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

Yes, I think you're right, and I think I led Marta and Tapani astray.  I
think they originally had separate sections, and I told them to change
at least some of them.  The whole separate section mechanism is
confusing and leads to code churn (imagine if an ES3.0 extension comes
along that enables this functionality).

I think a good Python project for someone would be to reorganize that
file so that all the enums are in a single list.  Each enum would have a
list of minterms to describe its availability.  These would include
profile and version.  Instead of having a flat array of integers, we'd
have an array of structures:

struct requirement {
    gl_api  API;
    unsigned Version;       /**< Minimum version required. */
    unsigned extension[2];  /**< Offsets of required extensions.  Use
                             * dummy_true if none.
                             */
    enum value_extra extra; /**< Other requirements. */
};

This is equivalent to the current system (which is also a maximum of
minterms), but I think it's a lot easier to specify things.

Either way, I think that can be follow-up work.

> 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.
> 
>   -ilia
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>