On Tue, Mar 10, 2015 at 11:34 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 | 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 > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev