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

[Tapani Pälli]

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>