On Thu, Mar 5, 2015 at 12:20 AM, Eduardo Lima Mitev <el...@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 | 117 > ++++++++++++++++++++++++++++++++++++++-------------- > src/mesa/main/pbo.h | 14 +++++++ > 2 files changed, 100 insertions(+), 31 deletions(-) > > diff --git a/src/mesa/main/pbo.c b/src/mesa/main/pbo.c > index 259f763..5ae248c 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. > * \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,26 +183,87 @@ _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); > Changing the error to "%s%uD, where, dimensions" is not ideal because the other function that calls _mesa_map_validate_pbo_source is _mesa_PolygonStipple, and printing "glPolygonStipple2D" doesn't make sense because the name of the function is "glPolygonStipple." > + "%s%uD(out of bounds PBO access)", > + where, dimensions); > } else { > _mesa_error(ctx, GL_INVALID_OPERATION, > Why did you remove "bufSize (%d) is too small"? If we are going to remove that, maybe we should remove the if, then, else block because the error messages are pretty much the same. I recommend keeping the original error messages for both and moving the "%sD" up to the calling functions (for example, texture_error_check). > - "%s(out of bounds access: bufSize (%d) is too > small)", > - where, clientMemSize); > + "%s%uD(out of bounds access)", > + where, dimensions); > } > - 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); > + _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(PBO is mapped)", > + where, dimensions); > + 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. > + */ > +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%uD(invalid PBO access)", > + where, dimensions); > This should be return false. > return NULL; > } > > + if (_mesa_check_disallowed_mapping(unpack->BufferObj)) { > + /* buffer is already mapped - that's an error */ > + _mesa_error(ctx, GL_INVALID_OPERATION, "%s%uD(PBO is mapped)", > + where, dimensions); > + 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); > return 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 > > _______________________________________________ > 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