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

[Lofstedt, Marta]

> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Tapani Pälli
> Sent: Monday, August 24, 2015 11:22 AM
> To: Marta Lofstedt; mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH 2/2] mesa/es3.1: Limit Framebuffer
> Parameter OpenGL ES 3.1 usage
> 
> 
> 
> 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.
OKI, done in V2.

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