[Mesa-dev] [PATCH 3/4 v2] mesa: Separate PBO validation checks from buffer mapping, to allow reuse
Laura Ekstrand
laura at jlekstrand.net
Wed Mar 11 16:15:57 PDT 2015
On Tue, Mar 10, 2015 at 11:34 AM, Eduardo Lima Mitev <elima at igalia.com>
wrote:
> Internal PBO functions such as _mesa_map_validate_pbo_source() and
> _mesa_validate_pbo_compressed_teximage() perform validation and buffer
> mapping
> within the same call.
>
> This patch takes out the validation into separate functions to allow reuse
> of functionality by other code (i.e, gl(Compressed)Tex(Sub)Image).
> ---
> src/mesa/main/pbo.c | 119
> ++++++++++++++++++++++++++++++++++++++--------------
> src/mesa/main/pbo.h | 14 +++++++
> 2 files changed, 101 insertions(+), 32 deletions(-)
>
> diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c
> index 259f763..46121a2 100644
> --- a/src/mesa/main/pbo.c
> +++ b/src/mesa/main/pbo.c
> @@ -164,23 +164,18 @@ _mesa_map_pbo_source(struct gl_context *ctx,
> return buf;
> }
>
> -
> /**
> - * Combine PBO-read validation and mapping.
> + * Perform PBO validation for read operations with uncompressed textures.
> * If any GL errors are detected, they'll be recorded and NULL returned.
>
Maybe say something like "If there are errors, return false, else return
true"? This is important because in other parts of the driver (like
main/teximage.c), error checking functions return GL_TRUE if an error is
found and GL_FALSE if one isn't found. (Confusing, I know :< )
> * \sa _mesa_validate_pbo_access
> - * \sa _mesa_map_pbo_source
> - * A call to this function should have a matching call to
> - * _mesa_unmap_pbo_source().
> */
> -const GLvoid *
> -_mesa_map_validate_pbo_source(struct gl_context *ctx,
> - GLuint dimensions,
> - const struct gl_pixelstore_attrib *unpack,
> - GLsizei width, GLsizei height, GLsizei
> depth,
> - GLenum format, GLenum type,
> - GLsizei clientMemSize,
> - const GLvoid *ptr, const char *where)
> +bool
> +_mesa_validate_pbo_source(struct gl_context *ctx, GLuint dimensions,
> + const struct gl_pixelstore_attrib *unpack,
> + GLsizei width, GLsizei height, GLsizei depth,
> + GLenum format, GLenum type,
> + GLsizei clientMemSize,
> + const GLvoid *ptr, const char *where)
> {
> assert(dimensions == 1 || dimensions == 2 || dimensions == 3);
>
> @@ -188,24 +183,85 @@ _mesa_map_validate_pbo_source(struct gl_context *ctx,
> format, type, clientMemSize, ptr)) {
> if (_mesa_is_bufferobj(unpack->BufferObj)) {
> _mesa_error(ctx, GL_INVALID_OPERATION,
> - "%s(out of bounds PBO access)", where);
> - } else {
> - _mesa_error(ctx, GL_INVALID_OPERATION,
>
You changed the order of the conditions from the original version. In the
original, if (_mesa_is_bufferobj), then say "out of bounds PBO access." If
(!_mesa_is_bufferobj), say "out of bounds access: bufSize is too small."
You have them switched around here.
> "%s(out of bounds access: bufSize (%d) is too
> small)",
> where, clientMemSize);
> + } else {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(out of bounds access)",
> + where);
> }
> - return NULL;
> + return false;
> }
>
> if (!_mesa_is_bufferobj(unpack->BufferObj)) {
> /* non-PBO access: no further validation to be done */
> - return ptr;
> + return true;
> }
>
> if (_mesa_check_disallowed_mapping(unpack->BufferObj)) {
> /* buffer is already mapped - that's an error */
> - _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)", where);
> - return NULL;
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)",
> + where);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/**
> + * Perform PBO validation for read operations with compressed textures.
> + * If any GL errors are detected, they'll be recorded and NULL returned.
>
Same here as above. Say something about the return value (true or false).
> + */
> +bool
> +_mesa_validate_pbo_source_compressed(struct gl_context *ctx, GLuint
> dimensions,
> + const struct gl_pixelstore_attrib
> *unpack,
> + GLsizei imageSize, const GLvoid
> *pixels,
> + const char *where)
> +{
> + if (!_mesa_is_bufferobj(unpack->BufferObj)) {
> + /* not using a PBO */
> + return true;
> + }
> +
> + if ((const GLubyte *) pixels + imageSize >
> + ((const GLubyte *) 0) + unpack->BufferObj->Size) {
> + /* out of bounds read! */
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid PBO access)",
> + where);
> + return false;
> + }
> +
> + if (_mesa_check_disallowed_mapping(unpack->BufferObj)) {
> + /* buffer is already mapped - that's an error */
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(PBO is mapped)",
> + where);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/**
> + * Perform PBO-read mapping.
> + * If any GL errors are detected, they'll be recorded and NULL returned.
> + * \sa _mesa_validate_pbo_source
> + * \sa _mesa_map_pbo_source
> + * A call to this function should have a matching call to
> + * _mesa_unmap_pbo_source().
> + */
> +const GLvoid *
> +_mesa_map_validate_pbo_source(struct gl_context *ctx,
> + GLuint dimensions,
> + const struct gl_pixelstore_attrib *unpack,
> + GLsizei width, GLsizei height, GLsizei
> depth,
> + GLenum format, GLenum type,
> + GLsizei clientMemSize,
> + const GLvoid *ptr, const char *where)
> +{
> + if (!_mesa_validate_pbo_source(ctx, dimensions, unpack,
> + width, height, depth, format, type,
> + clientMemSize, ptr, where)) {
> + return NULL;
> }
>
> ptr = _mesa_map_pbo_source(ctx, unpack, ptr);
> @@ -381,28 +437,27 @@ _mesa_validate_pbo_compressed_teximage(struct
> gl_context *ctx,
> {
> GLubyte *buf;
>
> + if (!_mesa_validate_pbo_source_compressed(ctx, dimensions, packing,
> + imageSize, pixels,
> funcName)) {
> + /* error is already set during validation */
> + return NULL;
> + }
> +
> if (!_mesa_is_bufferobj(packing->BufferObj)) {
> /* not using a PBO - return pointer unchanged */
> return pixels;
> }
> - if ((const GLubyte *) pixels + imageSize >
> - ((const GLubyte *) 0) + packing->BufferObj->Size) {
> - /* out of bounds read! */
> - _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(invalid PBO access)",
> - funcName, dimensions);
> - return NULL;
> - }
>
> buf = (GLubyte*) ctx->Driver.MapBufferRange(ctx, 0,
> packing->BufferObj->Size,
> GL_MAP_READ_BIT,
> packing->BufferObj,
> MAP_INTERNAL);
> - if (!buf) {
> - _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(PBO is mapped)",
> funcName,
> - dimensions);
> - return NULL;
> - }
> +
> + /* Validation above already checked that PBO is not mapped, so buffer
> + * should not be null.
> + */
> + assert(buf);
>
> return ADD_POINTERS(buf, pixels);
> }
> diff --git a/src/mesa/main/pbo.h b/src/mesa/main/pbo.h
> index 9851ef1..b3f24e6 100644
> --- a/src/mesa/main/pbo.h
> +++ b/src/mesa/main/pbo.h
> @@ -92,4 +92,18 @@ _mesa_unmap_teximage_pbo(struct gl_context *ctx,
> const struct gl_pixelstore_attrib *unpack);
>
>
> +extern bool
> +_mesa_validate_pbo_source(struct gl_context *ctx, GLuint dimensions,
> + const struct gl_pixelstore_attrib *unpack,
> + GLsizei width, GLsizei height, GLsizei depth,
> + GLenum format, GLenum type,
> + GLsizei clientMemSize,
> + const GLvoid *ptr, const char *where);
> +
> +extern bool
> +_mesa_validate_pbo_source_compressed(struct gl_context *ctx, GLuint
> dimensions,
> + const struct gl_pixelstore_attrib
> *unpack,
> + GLsizei imageSize, const GLvoid *ptr,
> + const char *where);
> +
> #endif
> --
> 2.1.3
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150311/eb11ab3e/attachment-0001.html>
More information about the mesa-dev
mailing list