[Mesa-dev] [PATCH] mesa: allow multisampled format info to be returned on GLES 3.1

[Ilia Mirkin]

On Sun, Feb 14, 2016 at 1:28 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Sun, Feb 14, 2016 at 5:36 AM, Timothy Arceri
> <timothy.arceri at collabora.com> wrote:
>> On Sat, 2016-02-13 at 23:50 -0500, Ilia Mirkin wrote:
>>> Marta, perhaps you can have a look? It's pretty similar to your
>>> commit
>>> b6d014f0b (mesa/es3.1: Pass sample count check for multisampled
>>> textures).
>>>
>>> On Thu, Feb 11, 2016 at 3:04 PM, Ilia Mirkin <imirkin at alum.mit.edu>
>>> wrote:
>>> > The restriction on multisampled integer texture formats only
>>> > applies to
>>> > GLES 3.0, so don't apply it to GLES 3.1 contexts. This fixes a slew
>>> > of
>>> > mesa: allow multisampled
>>> > dEQP-GLES31.functional.state_query.internal_format.*
>>> >
>>> > tests, which now all pass.
>>> >
>>> > Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> > ---
>>> >  src/mesa/main/formatquery.c | 3 ++-
>>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/src/mesa/main/formatquery.c
>>> > b/src/mesa/main/formatquery.c
>>> > index 85f7b6b..cfadffd 100644
>>> > --- a/src/mesa/main/formatquery.c
>>> > +++ b/src/mesa/main/formatquery.c
>>> > @@ -131,7 +131,8 @@ _mesa_GetInternalformativ(GLenum target, GLenum
>>> > internalformat, GLenum pname,
>>> >              internalformat, buffer);ctx->API == API_OPENGLES2 &&
>>> > ctx->Version >= 30
>>> >        break;
>>> >     case GL_NUM_SAMPLE_COUNTS: {
>>> > -      if (_mesa_is_gles3(ctx) &&
>>> > _mesa_is_enum_format_integer(internalformat)) {
>>> > +      if (_mesa_is_gles3(ctx) && !_mesa_is_gles31(ctx) &&
>>> > +          _mesa_is_enum_format_integer(internalformat)) {
>>
>> IMO it would be much nicer to just have:
>>
>> ctx->API == API_OPENGLES2 && ctx->Version == 30
>
> Sure thing. That's also the style that Marta went with in her change
> (although I hadn't seen that back when I composed this commit).
>
>>
>> It would also be nice to update the comment to say that multisampling
>> was added in 3.1 so the rule no longer applies.
>
> Actually MS existed in 3.0 as well, just not for integer formats.
>
>>
>> Looking at the GLES 3.1 spec there is more to it.
>>
>> An INVALID_ENUM error is generated if internalformat is not color-,
>> depth- or stencil-renderable.
>>
>> Where for GL zero is returned.
>
> Interesting. I'll have a closer look, thanks for digging through the
> specs! I wonder if we already have such a function [to determine the
> renderability of a format]. If not, I think that would go beyond the
> scope of this change...

I believe this is handled above:

   /* The ARB_internalformat_query spec says:
    *
    *     "If the <internalformat> parameter to GetInternalformativ is not
    *     color-, depth- or stencil-renderable, then an INVALID_ENUM error is
    *     generated."
    *
    * Page 243 of the GLES 3.0.4 spec says this for GetInternalformativ:
    *
    *     "internalformat must be color-renderable, depth-renderable or
    *     stencilrenderable (as defined in section 4.4.4)."
    *
    * Section 4.4.4 on page 212 of the same spec says:
    *
    *     "An internal format is color-renderable if it is one of the
    *     formats from table 3.13 noted as color-renderable or if it
    *     is unsized format RGBA or RGB."
    *
    * Therefore, we must accept GL_RGB and GL_RGBA here.
    */
   if (internalformat != GL_RGB && internalformat != GL_RGBA &&
       _mesa_base_fbo_format(ctx, internalformat) == 0) {
      _mesa_error(ctx, GL_INVALID_ENUM,
                  "glGetInternalformativ(internalformat=%s)",
                  _mesa_enum_to_string(internalformat));
      return;
   }

I'll resend this with the adjusted version check.