[Mesa-dev] [PATCH 2/2] mesa/es3.1: Limit Framebuffer Parameter OpenGL ES 3.1 usage

[Tapani Pälli]

On 08/24/2015 12:21 PM, Tapani Pälli wrote:
>
>
> On 08/24/2015 10:10 AM, Marta Lofstedt wrote:
>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>
>> According to OpenGL ES 3.1 specification,
>> section 9.2.1 for glFramebufferParameter
>> and section 9.2.3 for glGetFramebufferParameteriv:
>>
>> "An INVALID_ENUM error is generated if pname is not
>> FRAMEBUFFER_DEFAULT_WIDTH, FRAMEBUFFER_DEFAULT_HEIGHT,
>> FRAMEBUFFER_DEFAULT_SAMPLES, or
>> FRAMEBUFFER_DEFAULT_FIXED_SAMPLE_LOCATIONS."
>>
>> Therefore exclude OpenGL ES 3.1 from using the
>> GL_FRAMEBUFFER_DEFAULT_LAYERS parameter.
>>
>> Signed-off-by: Marta Lofstedt <marta.lofstedt at intel.com>
>> ---
>>   src/mesa/main/fbobject.c | 34 ++++++++++++++++++++++++++++------
>>   1 file changed, 28 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 07db195..1bbdd30 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -1389,11 +1389,22 @@ framebuffer_parameteri(struct gl_context *ctx,
>> struct gl_framebuffer *fb,
>>            fb->DefaultGeometry.Height = param;
>>         break;
>>      case GL_FRAMEBUFFER_DEFAULT_LAYERS:
>> -      if (param < 0 || param > ctx->Const.MaxFramebufferLayers)
>> -        _mesa_error(ctx, GL_INVALID_VALUE, "%s", func);
>> -      else
>> -         fb->DefaultGeometry.Layers = param;
>> -      break;
>> +     /*
>> +      * According to the OpenGL ES 3.1 specification,
>> +      * section 9.2.1, the GL_FRAMEBUFFER_DEFAULT_LAYERS
>> +      * parameter name is not supported.
>> +      */
>> +      if (_mesa_is_gles31(ctx)) {
>> +         _mesa_error(ctx, GL_INVALID_ENUM,
>> +                     "%s(pname=0x%x)", func, pname);
>> +      }
>> +      else {
>> +         if (param < 0 || param > ctx->Const.MaxFramebufferLayers)
>> +            _mesa_error(ctx, GL_INVALID_VALUE, "%s", func);
>> +         else
>> +            fb->DefaultGeometry.Layers = param;
>> +         break;
>> +      }
>
> I'd suggest to avoid else { ... }, just insert the error case before
> regular handling and break out if error. No need to touch original code
> and looks cleaner.
>
>>      case GL_FRAMEBUFFER_DEFAULT_SAMPLES:
>>         if (param < 0 || param > ctx->Const.MaxFramebufferSamples)
>>           _mesa_error(ctx, GL_INVALID_VALUE, "%s", func);
>> @@ -1451,7 +1462,18 @@ get_framebuffer_parameteriv(struct gl_context
>> *ctx, struct gl_framebuffer *fb,
>>         *params = fb->DefaultGeometry.Height;
>>         break;
>>      case GL_FRAMEBUFFER_DEFAULT_LAYERS:
>> -      *params = fb->DefaultGeometry.Layers;
>> +      /*
>> +       * According to the OpenGL ES 3.1 specification,
>> +       * section 9.2.3, the GL_FRAMEBUFFER_LAYERS
>> +       * parameter name is not supported.
>> +       */
>> +      if (_mesa_is_gles31(ctx)) {
>> +         _mesa_error(ctx, GL_INVALID_ENUM,
>> +                     "%s(pname=0x%x)", func, pname);
>> +      }
>> +      else {
>> +         *params = fb->DefaultGeometry.Layers;
>> +      }
>
> same here
>
>>         break;
>>      case GL_FRAMEBUFFER_DEFAULT_SAMPLES:
>>         *params = fb->DefaultGeometry.NumSamples;
>>
>
>
> With these changes both of the patches are
>
> Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

One note, I just spotted that with these changes you can also enable 
ARB_framebuffer_no_attachments for 3.1 check in compute_version_es2() 
(version.c).

// Tapani