[Mesa-dev] [PATCH 6/9] mesa/es3.1: Allow Multisampled FrameBufferTextures

[Ilia Mirkin]

On Mon, May 11, 2015 at 10:08 AM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
> On Mon, May 11, 2015 at 3:03 PM, Marta Lofstedt
> <marta.lofstedt at linux.intel.com> wrote:
>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>
>> GLES 3.1 must be allowed to use multisampled
>> frambuffer textures.
>>
>> Signed-off-by: Marta Lofstedt <marta.lofstedt at intel.com>
>> ---
>>  src/mesa/main/fbobject.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 27cf97f..14a015e 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -2756,8 +2756,9 @@ _mesa_FramebufferTexture2D(GLenum target, GLenum attachment,
>>           break;
>>        case GL_TEXTURE_2D_MULTISAMPLE:
>>        case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>> -         error = _mesa_is_gles(ctx)
>> -            || !ctx->Extensions.ARB_texture_multisample;
>> +         error = (_mesa_is_gles(ctx)
>> +            || !ctx->Extensions.ARB_texture_multisample) &&
>> +            !_mesa_is_gles31(ctx);

This seems correct. error = true when old condition, but not when
es3.1 even if the old condition holds true. If the old condition is
false, then the new addition doesn't matter.

Personally I would have written this as

error = _mesa_is_gles(ctx) && !mesa_is_gles31(ctx) ||
!ctx->Extensions.ARB_texture_multisample;

The nice thing about this is that it will force an error even in the
very hypothetical situation where a driver doesn't expose
ARB_texture_multisample, but a GLES3.1 context was created (e.g. via
an override flag).

>>           break;
>>        default:
>>           error = GL_TRUE;
>
> Shouldn't this be like this instead (and make sure
> ARB_texture_multisample is enabled for ES3.1)?
>
> @@ -2756,8 +2756,9 @@ _mesa_FramebufferTexture2D(GLenum target, GLenum
> attachment,
>           break;
>        case GL_TEXTURE_2D_MULTISAMPLE:
>        case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
> -         error = _mesa_is_gles(ctx)
> -            || !ctx->Extensions.ARB_texture_multisample;
> +         error = !ctx->Extensions.ARB_texture_multisample;

error = false when you have a driver that supports texture_ms, but you
have a gles1/2/3 context, whereas you wanted error = true there...

  -ilia