[Mesa-dev] [PATCH] mesa: fix error handling in get_framebuffer_parameteriv

[Ian Romanick]

On 05/04/2018 03:27 PM, Rhys Perry wrote:
> CC: <mesa-stable at lists.freedesktop.org>
> Signed-off-by: Rhys Perry <pendingchaos02 at gmail.com>
> ---
>  src/mesa/main/fbobject.c | 72 +++++++++++++++++++++++++++---------------------
>  1 file changed, 41 insertions(+), 31 deletions(-)
> 
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index c72204e11a..0fc3319fe9 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -1488,45 +1488,66 @@ _mesa_FramebufferParameteri(GLenum target, GLenum pname, GLint param)
>  }
>  
>  static bool
> -_pname_valid_for_default_framebuffer(struct gl_context *ctx,
> -                                     GLenum pname)
> +validate_get_framebuffer_parameteriv_pname(struct gl_context *ctx,
> +                                           struct gl_framebuffer *fb,
> +                                           GLuint pname, const char *func)
>  {
> -   if (!_mesa_is_desktop_gl(ctx))
> -      return false;
> +   bool cannotBeWinsysFbo = true;

The common usage in Mesa is to separate words with _ instead of using
camel case.

With that changed, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>  
>     switch (pname) {
> +   case GL_FRAMEBUFFER_DEFAULT_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) && !ctx->Extensions.OES_geometry_shader) {
> +         _mesa_error(ctx, GL_INVALID_ENUM, "%s(pname=0x%x)", func, pname);
> +         return false;
> +      }
> +      break;
> +   case GL_FRAMEBUFFER_DEFAULT_WIDTH:
> +   case GL_FRAMEBUFFER_DEFAULT_HEIGHT:
> +   case GL_FRAMEBUFFER_DEFAULT_SAMPLES:
> +   case GL_FRAMEBUFFER_DEFAULT_FIXED_SAMPLE_LOCATIONS:
> +      break;
>     case GL_DOUBLEBUFFER:
>     case GL_IMPLEMENTATION_COLOR_READ_FORMAT:
>     case GL_IMPLEMENTATION_COLOR_READ_TYPE:
>     case GL_SAMPLES:
>     case GL_SAMPLE_BUFFERS:
>     case GL_STEREO:
> -      return true;
> +      /* From OpenGL 4.5 spec, section 9.2.3 "Framebuffer Object Queries:
> +       *
> +       *    "An INVALID_OPERATION error is generated by GetFramebufferParameteriv
> +       *     if the default framebuffer is bound to target and pname is not one
> +       *     of the accepted values from table 23.73, other than
> +       *     SAMPLE_POSITION."
> +       *
> +       * For OpenGL ES, using default framebuffer raises INVALID_OPERATION
> +       * for any pname.
> +       */
> +      cannotBeWinsysFbo = !_mesa_is_desktop_gl(ctx);
> +      break;
>     default:
> +      _mesa_error(ctx, GL_INVALID_ENUM, "%s(pname=0x%x)", func, pname);
>        return false;
>     }
> +
> +   if (cannotBeWinsysFbo && _mesa_is_winsys_fbo(fb)) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "%s(invalid pname=0x%x for default framebuffer)", func, pname);
> +      return false;
> +   }
> +
> +   return true;
>  }
>  
>  static void
>  get_framebuffer_parameteriv(struct gl_context *ctx, struct gl_framebuffer *fb,
>                              GLenum pname, GLint *params, const char *func)
>  {
> -   /* From OpenGL 4.5 spec, section 9.2.3 "Framebuffer Object Queries:
> -    *
> -    *    "An INVALID_OPERATION error is generated by GetFramebufferParameteriv
> -    *     if the default framebuffer is bound to target and pname is not one
> -    *     of the accepted values from table 23.73, other than
> -    *     SAMPLE_POSITION."
> -    *
> -    * For OpenGL ES, using default framebuffer still raises INVALID_OPERATION
> -    * for any pname.
> -    */
> -   if (_mesa_is_winsys_fbo(fb) &&
> -       !_pname_valid_for_default_framebuffer(ctx, pname)) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION,
> -                  "%s(invalid pname=0x%x for default framebuffer)", func, pname);
> +   if (!validate_get_framebuffer_parameteriv_pname(ctx, fb, pname, func))
>        return;
> -   }
>  
>     switch (pname) {
>     case GL_FRAMEBUFFER_DEFAULT_WIDTH:
> @@ -1536,14 +1557,6 @@ get_framebuffer_parameteriv(struct gl_context *ctx, struct gl_framebuffer *fb,
>        *params = fb->DefaultGeometry.Height;
>        break;
>     case GL_FRAMEBUFFER_DEFAULT_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) && !ctx->Extensions.OES_geometry_shader) {
> -         _mesa_error(ctx, GL_INVALID_ENUM, "%s(pname=0x%x)", func, pname);
> -         break;
> -      }
>        *params = fb->DefaultGeometry.Layers;
>        break;
>     case GL_FRAMEBUFFER_DEFAULT_SAMPLES:
> @@ -1570,9 +1583,6 @@ get_framebuffer_parameteriv(struct gl_context *ctx, struct gl_framebuffer *fb,
>     case GL_STEREO:
>        *params = fb->Visual.stereoMode;
>        break;
> -   default:
> -      _mesa_error(ctx, GL_INVALID_ENUM,
> -                  "%s(pname=0x%x)", func, pname);
>     }
>  }
>  
>