[Mesa-dev] [PATCH 10/13] mesa: add KHR_no_error support for FramebufferTextureLayer

[Eric Anholt]

Timothy Arceri <tarceri at itsqueeze.com> writes:

> ---
>  src/mapi/glapi/gen/ARB_framebuffer_object.xml |  2 +-
>  src/mesa/main/fbobject.c                      | 31 +++++++++++++++++++++++++++
>  src/mesa/main/fbobject.h                      |  4 ++++
>  3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/src/mapi/glapi/gen/ARB_framebuffer_object.xml b/src/mapi/glapi/gen/ARB_framebuffer_object.xml
> index ce5e45d..76114eb 100644
> --- a/src/mapi/glapi/gen/ARB_framebuffer_object.xml
> +++ b/src/mapi/glapi/gen/ARB_framebuffer_object.xml
> @@ -239,21 +239,21 @@
>      <function name="FramebufferTexture3D" no_error="true">
>          <param name="target" type="GLenum"/>
>          <param name="attachment" type="GLenum"/>
>          <param name="textarget" type="GLenum"/>
>          <param name="texture" type="GLuint"/>
>          <param name="level" type="GLint"/>
>          <param name="layer" type="GLint"/>
>  	<glx rop="4323"/>
>      </function>
>  
> -    <function name="FramebufferTextureLayer" es2="3.0">
> +    <function name="FramebufferTextureLayer" es2="3.0" no_error="true">
>          <param name="target" type="GLenum"/>
>          <param name="attachment" type="GLenum"/>
>          <param name="texture" type="GLuint"/>
>          <param name="level" type="GLint"/>
>          <param name="layer" type="GLint"/>
>  	<glx rop="237"/>
>      </function>
>  
>      <function name="FramebufferRenderbuffer" es2="2.0">
>          <param name="target" type="GLenum"/>
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index ba01d0c..08e7347 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -3424,20 +3424,51 @@ void GLAPIENTRY
>  _mesa_FramebufferTexture3D(GLenum target, GLenum attachment,
>                             GLenum textarget, GLuint texture,
>                             GLint level, GLint layer)
>  {
>     framebuffer_texture_with_dims(3, target, attachment, textarget, texture,
>                                   level, layer, "glFramebufferTexture3D");
>  }
>  
>  
>  void GLAPIENTRY
> +_mesa_FramebufferTextureLayer_no_error(GLenum target, GLenum attachment,
> +                                       GLuint texture, GLint level,
> +                                       GLint layer)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   /* Get the framebuffer object */
> +   struct gl_framebuffer *fb = get_framebuffer_target(ctx, target);
> +
> +   /* Get the texture object */
> +   struct gl_texture_object *texObj =
> +      get_texture_for_framebuffer(ctx, texture);
> +
> +   struct gl_renderbuffer_attachment *att =
> +      get_attachment(ctx, fb, attachment, NULL);
> +
> +   GLenum textarget = 0;
> +   if (texObj) {
> +      if (texObj->Target == GL_TEXTURE_CUBE_MAP) {
> +         assert(layer >= 0 && layer < 6);
> +         textarget = GL_TEXTURE_CUBE_MAP_POSITIVE_X + layer;
> +         layer = 0;
> +      }
> +   }
> +
> +   _mesa_framebuffer_texture(ctx, fb, attachment, att, texObj, textarget,
> +                             level, layer, GL_FALSE);
> +}

I've got stuck on reading this series a couple of times because this
felt like too much code duplication.  I understand that you're
replicating what the current error-validating code does, but it seems
like for no_error we can do a lot better, quite easily.

What I'd like for these last 4 patches is to have two patches, each of
which implements a Named and non-Named no_error function by doing the
appropriate lookup of the fb and then calling a helper function that
does the rest of their current bodies.

Assuming patch 4.5 is to be squashed into 4, then other than the ';'
comment patches 1-9 are:

Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170509/1e645ba0/attachment.sig>