On Fri, Feb 27, 2015 at 4:07 PM, Laura Ekstrand <la...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev