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

[Ilia Mirkin]

On Mon, May 11, 2015 at 4:34 PM, Ian Romanick <idr at freedesktop.org> wrote:
> 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.

OK, well, I won't fight this further -- I think I've made my point,
and I believe you've understood it. Since you disagree, happy to let
it go.

I can't help but add though that in the situation where the ARB
extension is a superset in terms of actual backend driver
functionality, not just small frontend differences (e.g.
ARB_gpu_shader5), we should add more enables that allow the (in this
case) ES3.1 functionality. This enables a driver to be developed over
time and lets people use version overrides without resulting in
crashes or other driver weirdness.

  -ilia