[Mesa-dev] [PATCH 5/6] main: Refactor Tex[ture]Buffer[Range].

Anuj Phogat anuj.phogat at gmail.com
Wed Mar 4 11:37:25 PST 2015


On Fri, Feb 27, 2015 at 4:07 PM, Laura Ekstrand <laura at jlekstrand.net> wrote:
> Uses _mesa_lookup_bufferobj_err to clean up buffer object retrieval.  Moves
> error checking statements into separate functions to allow code sharing
> between traditional and ARB_direct_state_access entry points.
It's little difficult to follow the changes in this patch. I think the patch
can be split in to two:
Patch 1: Use _mesa_lookup_bufferobj_err to clean up buffer object retrieval.
Patch 2:  Use helper functions for error checking in
_mesa_Tex{ture}Buffer{Range}

> ---
>  src/mesa/main/teximage.c | 201 ++++++++++++++++++++++++++++++-----------------
>  src/mesa/main/teximage.h |   6 +-
>  2 files changed, 132 insertions(+), 75 deletions(-)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index d454dd9..9853bc0 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -5301,24 +5301,34 @@ _mesa_validate_texbuffer_format(const struct gl_context *ctx,
>
>  void
>  _mesa_texture_buffer_range(struct gl_context *ctx,
> -                           struct gl_texture_object *texObj, GLenum target,
> +                           struct gl_texture_object *texObj,
>                             GLenum internalFormat,
>                             struct gl_buffer_object *bufObj,
> -                           GLintptr offset, GLsizeiptr size, bool range,
> -                           bool dsa)
> +                           GLintptr offset, GLsizeiptr size,
> +                           const char *caller)
>  {
>     mesa_format format;
>
> -   FLUSH_VERTICES(ctx, 0);
> +   /* NOTE: ARB_texture_buffer_object has interactions with
> +    * the compatibility profile that are not implemented.
> +    */
> +   if (!(ctx->API == API_OPENGL_CORE &&
> +         ctx->Extensions.ARB_texture_buffer_object)) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "%s(ARB_texture_buffer_object is not"
> +                  " implemented for the compatibility profile)", caller);
> +      return;
> +   }
>
>     format = _mesa_validate_texbuffer_format(ctx, internalFormat);
>     if (format == MESA_FORMAT_NONE) {
>        _mesa_error(ctx, GL_INVALID_ENUM,
> -                  "glTex%sBuffer%s(internalFormat 0x%x)", dsa ? "ture" : "",
> -                  range ? "Range" : "", internalFormat);
> +                  "%s(internalFormat 0x%x)", caller, internalFormat);
>        return;
>     }
>
> +   FLUSH_VERTICES(ctx, 0);
> +
>     _mesa_lock_texture(ctx, texObj);
>     {
>        _mesa_reference_buffer_object(ctx, &texObj->BufferObject, bufObj);
> @@ -5337,6 +5347,75 @@ _mesa_texture_buffer_range(struct gl_context *ctx,
>  }
>
>
> +/**
> + * Make sure the texture buffer target is GL_TEXTURE_BUFFER.
> + * Return true if it is, and return false if it is not
> + * (and throw INVALID ENUM as dictated in the OpenGL 4.5
> + * core spec, 02.02.2015, PDF page 245).
> + */
> +static bool
> +check_texture_buffer_target(struct gl_context *ctx, GLenum target,
> +                            const char *caller)
> +{
> +   if (target != GL_TEXTURE_BUFFER_ARB) {
> +      _mesa_error(ctx, GL_INVALID_ENUM,
> +                  "%s(texture target is not GL_TEXTURE_BUFFER)", caller);
> +      return false;
> +   }
> +   else
else on the same line as curly brace.
> +      return true;
> +}
> +
> +/**
> + * Check for errors related to the texture buffer range.
> + * Return false if errors are found, true if none are found.
> + */
> +static bool
> +check_texture_buffer_range(struct gl_context *ctx,
> +                           struct gl_buffer_object *bufObj,
> +                           GLintptr offset, GLsizeiptr size,
> +                           const char *caller)
> +{
> +   /* OpenGL 4.5 core spec (02.02.2015) says in Section 8.9 Buffer
> +    * Textures (PDF page 245):
> +    *    "An INVALID_VALUE error is generated if offset is negative, if
> +    *    size is less than or equal to zero, or if offset + size is greater
> +    *    than the value of BUFFER_SIZE for the buffer bound to target."
> +    */
> +   if (offset < 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "%s(offset=%d < 0)", caller,
> +                  (int) offset);
> +      return false;
> +   }
> +
> +   if (size <= 0) {
> +      _mesa_error(ctx, GL_INVALID_VALUE, "%s(size=%d <= 0)", caller,
> +                  (int) size);
> +      return false;
> +   }
> +
> +   if (offset + size > bufObj->Size) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "%s(offset=%d + size=%d > buffer_size=%d)", caller,
> +                  (int) offset, (int) size, (int) bufObj->Size);
> +      return false;
> +   }
> +
> +   /* OpenGL 4.5 core spec (02.02.2015) says in Section 8.9 Buffer
> +    * Textures (PDF page 245):
> +    *    "An INVALID_VALUE error is generated if offset is not an integer
> +    *    multiple of the value of TEXTURE_BUFFER_OFFSET_ALIGNMENT."
> +    */
> +   if (offset % ctx->Const.TextureBufferOffsetAlignment) {
> +      _mesa_error(ctx, GL_INVALID_VALUE,
> +                  "%s(invalid offset alignment)", caller);
> +      return false;
> +   }
> +
> +   return true;
> +}
> +
> +
>  /** GL_ARB_texture_buffer_object */
>  void GLAPIENTRY
>  _mesa_TexBuffer(GLenum target, GLenum internalFormat, GLuint buffer)
> @@ -5346,33 +5425,26 @@ _mesa_TexBuffer(GLenum target, GLenum internalFormat, GLuint buffer)
>
>     GET_CURRENT_CONTEXT(ctx);
>
> -   /* Need to catch this before it gets to _mesa_get_current_tex_object */
> -   if (target != GL_TEXTURE_BUFFER_ARB) {
> -      _mesa_error(ctx, GL_INVALID_ENUM, "glTexBuffer(target)");
> -      return;
> -   }
> -
> -   /* NOTE: ARB_texture_buffer_object has interactions with
> -    * the compatibility profile that are not implemented.
> +   /* Need to catch a bad target before it gets to
> +    * _mesa_get_current_tex_object.
>      */
> -   if (!(ctx->API == API_OPENGL_CORE &&
> -         ctx->Extensions.ARB_texture_buffer_object)) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glTexBuffer");
> +   if (!check_texture_buffer_target(ctx, target, "glTexBuffer"))
>        return;
> -   }
>
> -   bufObj = _mesa_lookup_bufferobj(ctx, buffer);
> -   if (!bufObj && buffer) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glTexBuffer(buffer %u)", buffer);
> -      return;
> +   if (buffer) {
> +      bufObj = _mesa_lookup_bufferobj_err(ctx, buffer, "glTexBuffer");
> +      if (!bufObj)
> +         return;
>     }
> +   else
> +      bufObj = NULL;
>
>     texObj = _mesa_get_current_tex_object(ctx, target);
>     if (!texObj)
>        return;
>
> -   _mesa_texture_buffer_range(ctx, texObj, target, internalFormat, bufObj, 0,
> -                              buffer ? -1 : 0, false, false);
> +   _mesa_texture_buffer_range(ctx, texObj, internalFormat, bufObj, 0,
> +                              buffer ? -1 : 0, "glTexBuffer");
>  }
>
>
> @@ -5386,46 +5458,41 @@ _mesa_TexBufferRange(GLenum target, GLenum internalFormat, GLuint buffer,
>
>     GET_CURRENT_CONTEXT(ctx);
>
> -   /* Need to catch this before it gets to _mesa_get_current_tex_object */
> -   if (target != GL_TEXTURE_BUFFER_ARB) {
> -      _mesa_error(ctx, GL_INVALID_ENUM, "glTexBufferRange(target)");
> -      return;
> -   }
> -
> -   if (!(ctx->API == API_OPENGL_CORE &&
> -         ctx->Extensions.ARB_texture_buffer_range)) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glTexBufferRange");
> +   /* Need to catch a bad target before it gets to
> +    * _mesa_get_current_tex_object.
> +    */
> +   if (!check_texture_buffer_target(ctx, target, "glTexBufferRange"))
>        return;
> -   }
>
> -   bufObj = _mesa_lookup_bufferobj(ctx, buffer);
> -   if (bufObj) {
> -      if (offset < 0 ||
> -          size <= 0 ||
> -          (offset + size) > bufObj->Size) {
> -         _mesa_error(ctx, GL_INVALID_VALUE, "glTexBufferRange");
> +   if (buffer) {
> +      bufObj = _mesa_lookup_bufferobj_err(ctx, buffer, "glTexBufferRange");
> +      if (!bufObj)
>           return;
> -      }
> -      if (offset % ctx->Const.TextureBufferOffsetAlignment) {
> -         _mesa_error(ctx, GL_INVALID_VALUE,
> -                     "glTexBufferRange(invalid offset alignment)");
> +
> +      if (!check_texture_buffer_range(ctx, bufObj, offset, size,
> +          "glTexBufferRange"))
>           return;
> -      }
> -   } else if (buffer) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glTexBufferRange(buffer %u)",
> -                  buffer);
> -      return;
> +
>     } else {
> +
> +      /* OpenGL 4.5 core spec (02.02.2015) says in Section 8.9 Buffer
> +       * Textures (PDF page 254):
> +       *    "If buffer is zero, then any buffer object attached to the buffer
> +       *    texture is detached, the values offset and size are ignored and
> +       *    the state for offset and size for the buffer texture are reset to
> +       *    zero."
> +       */
>        offset = 0;
>        size = 0;
> +      bufObj = NULL;
>     }
>
>     texObj = _mesa_get_current_tex_object(ctx, target);
>     if (!texObj)
>        return;
>
> -   _mesa_texture_buffer_range(ctx, texObj, target, internalFormat, bufObj,
> -                              offset, size, true, false);
> +   _mesa_texture_buffer_range(ctx, texObj, internalFormat, bufObj,
> +                              offset, size, "glTexBufferRange");
>  }
>
>  void GLAPIENTRY
> @@ -5436,37 +5503,27 @@ _mesa_TextureBuffer(GLuint texture, GLenum internalFormat, GLuint buffer)
>
>     GET_CURRENT_CONTEXT(ctx);
>
> -   /* NOTE: ARB_texture_buffer_object has interactions with
> -    * the compatibility profile that are not implemented.
> -    */
> -   if (!(ctx->API == API_OPENGL_CORE &&
> -         ctx->Extensions.ARB_texture_buffer_object)) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureBuffer");
> -      return;
> -   }
> -
> -   bufObj = _mesa_lookup_bufferobj(ctx, buffer);
> -   if (!bufObj && buffer) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glTextureBuffer(buffer %u)",
> -                  buffer);
> -      return;
> +   if (buffer) {
> +      bufObj = _mesa_lookup_bufferobj_err(ctx, buffer, "glTextureBuffer");
> +      if (!bufObj)
> +         return;
>     }
> +   else
> +      bufObj = NULL;
>
>     /* Get the texture object by Name. */
> -   texObj = _mesa_lookup_texture_err(ctx, texture,
> -                                     "glTextureBuffer(texture)");
> +   texObj = _mesa_lookup_texture_err(ctx, texture, "glTextureBuffer");
>     if (!texObj)
>        return;
>
> -   if (texObj->Target != GL_TEXTURE_BUFFER_ARB) {
> -      _mesa_error(ctx, GL_INVALID_ENUM, "glTextureBuffer(target)");
> +   if (!check_texture_buffer_target(ctx, texObj->Target, "glTextureBuffer"))
>        return;
> -   }
>
> -   _mesa_texture_buffer_range(ctx, texObj, texObj->Target, internalFormat,
> -                              bufObj, 0, buffer ? -1 : 0, false, true);
> +   _mesa_texture_buffer_range(ctx, texObj, internalFormat,
> +                              bufObj, 0, buffer ? -1 : 0, "glTextureBuffer");
>  }
>
> +
>  static GLboolean
>  is_renderable_texture_format(struct gl_context *ctx, GLenum internalformat)
>  {
> diff --git a/src/mesa/main/teximage.h b/src/mesa/main/teximage.h
> index 9468650..db6b648 100644
> --- a/src/mesa/main/teximage.h
> +++ b/src/mesa/main/teximage.h
> @@ -209,11 +209,11 @@ _mesa_texture_image_multisample(struct gl_context *ctx, GLuint dims,
>
>  extern void
>  _mesa_texture_buffer_range(struct gl_context *ctx,
> -                           struct gl_texture_object *texObj, GLenum target,
> +                           struct gl_texture_object *texObj,
>                             GLenum internalFormat,
>                             struct gl_buffer_object *bufObj,
> -                           GLintptr offset, GLsizeiptr size, bool range,
> -                           bool dsa);
> +                           GLintptr offset, GLsizeiptr size,
> +                           const char *caller);
>  /*@}*/
>
>
> --
> 2.1.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list