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

[Lofstedt, Marta]

> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Ian Romanick
> Sent: Wednesday, July 22, 2015 8:55 PM
> To: Ilia Mirkin; Palli, Tapani
> Cc: mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH v4 3/6] mesa/es3.1: enable
> GL_ARB_texture_multisample for GLES 3.1
> 
> 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).
> 

Thanks, for clearing that up Ian...

A V5 has been sent. 
I needed to add functionality for the GLES31 label in get_hash_generator.py.
Unfortunately I couldn't come up with a better solution than to take all GLES2
and then GLES3 stuff to put in the GLES31 hashtable. If GLES version will continue
to be released, it will soon start to look real bad.

> 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
> >
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
----------------------------------------------------------------------
Intel Sweden AB
Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden
Registration Number: 556189-6027

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.