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

[Ian Romanick]

On 05/11/2015 07:57 AM, Ilia Mirkin wrote:
> 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).

I think this is a mis-feature.  Most of the ARB extensions are supersets
of the ES3.1 functionality, so we could someday have a driver that does
one but not the other.

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