Looks good to me. Reviewed-by: Laura Ekstrand <la...@jlekstrand.net>
On Thu, Mar 5, 2015 at 12:20 AM, Eduardo Lima Mitev <el...@igalia.com> wrote: > This patch adds two types of checks to the gl(Compressed)Tex(Sub)Imgage > family > of functions when a pixel buffer object is bound to GL_PIXEL_UNPACK_BUFFER: > > - That the buffer is not mapped. > - The total data size is within the boundaries of the buffer size. > > It does so by calling auxiliary validations functions from PBO API: > _mesa_validate_pbo_source() for non-compressed texture calls, and > _mesa_validate_pbo_source_compressed() for compressed texture calls. > > The first check is defined in Section 6.3.2 'Effects of Mapping Buffers > on Other GL Commands' of the GLES 3.1 spec, page 57: > > "Any GL command which attempts to read from, write to, or change the > state of a buffer object may generate an INVALID_OPERATION error if > all > or part of the buffer object is mapped. However, only commands which > explicitly describe this error are required to do so. If an error is > not > generated, using such commands to perform invalid reads, writes, or > state changes will have undefined results and may result in GL > interruption or termination." > > Similar wording exists in GL 4.5 spec, page 76. > > In the case of gl(Compressed)Tex(Sub)Image(2,3)D, the specification > doesn't force > implemtations to throw an error. However since Mesa don't currently > implement > checks to determine when it is safe to read/write from/to a mapped PBO, we > should always return the error if all or parts of it are mapped. > > The 2nd check is defined in Section 8.5 'Texture Image Specification' of > the > OpenGL 4.5 spec, page 203: > > "An INVALID_OPERATION error is generated if a pixel unpack buffer > object > is bound and storing texture data would access memory beyond the end > of > the pixel unpack buffer." > > Fixes 4 dEQP tests: > * > dEQP-GLES3.functional.negative_api.texture.compressedteximage2d_invalid_buffer_target > * > dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage2d_invalid_buffer_target > * > dEQP-GLES3.functional.negative_api.texture.compressedteximage3d_invalid_buffer_target > * > dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage3d_invalid_buffer_target > --- > src/mesa/main/teximage.c | 51 > +++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 42 insertions(+), 9 deletions(-) > > diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c > index abcafde..f239e39 100644 > --- a/src/mesa/main/teximage.c > +++ b/src/mesa/main/teximage.c > @@ -53,6 +53,7 @@ > #include "mtypes.h" > #include "glformats.h" > #include "texstore.h" > +#include "pbo.h" > > > /** > @@ -2110,7 +2111,8 @@ texture_error_check( struct gl_context *ctx, > GLint level, GLint internalFormat, > GLenum format, GLenum type, > GLint width, GLint height, > - GLint depth, GLint border ) > + GLint depth, GLint border, > + const GLvoid *pixels ) > { > GLenum err; > > @@ -2195,6 +2197,13 @@ texture_error_check( struct gl_context *ctx, > return GL_TRUE; > } > > + /* validate the bound PBO, if any */ > + if (!_mesa_validate_pbo_source(ctx, dimensions, &ctx->Unpack, > + width, height, depth, format, type, > + INT_MAX, pixels, "glTexImage")) { > + return GL_TRUE; > + } > + > /* make sure internal format and format basically agree */ > if (!texture_formats_agree(internalFormat, format)) { > _mesa_error(ctx, GL_INVALID_OPERATION, > @@ -2291,7 +2300,7 @@ compressed_texture_error_check(struct gl_context > *ctx, GLint dimensions, > GLenum target, GLint level, > GLenum internalFormat, GLsizei width, > GLsizei height, GLsizei depth, GLint > border, > - GLsizei imageSize) > + GLsizei imageSize, const GLvoid *data) > { > const GLint maxLevels = _mesa_max_texture_levels(ctx, target); > GLint expectedSize; > @@ -2319,6 +2328,12 @@ compressed_texture_error_check(struct gl_context > *ctx, GLint dimensions, > return GL_TRUE; > } > > + /* validate the bound PBO, if any */ > + if (!_mesa_validate_pbo_source_compressed(ctx, dimensions, > &ctx->Unpack, > + imageSize, data, > "glCompressedTexImage")) { > + return GL_TRUE; > + } > + > switch (internalFormat) { > case GL_PALETTE4_RGB8_OES: > case GL_PALETTE4_RGBA8_OES: > @@ -2451,7 +2466,8 @@ texsubimage_error_check(struct gl_context *ctx, > GLuint dimensions, > GLenum target, GLint level, > GLint xoffset, GLint yoffset, GLint zoffset, > GLint width, GLint height, GLint depth, > - GLenum format, GLenum type, bool dsa) > + GLenum format, GLenum type, const GLvoid *pixels, > + bool dsa) > { > struct gl_texture_image *texImage; > GLenum err; > @@ -2503,6 +2519,13 @@ texsubimage_error_check(struct gl_context *ctx, > GLuint dimensions, > return GL_TRUE; > } > > + /* validate the bound PBO, if any */ > + if (!_mesa_validate_pbo_source(ctx, dimensions, &ctx->Unpack, > + width, height, depth, format, type, > + INT_MAX, pixels, "glTexSubImage")) { > + return GL_TRUE; > + } > + > texImage = _mesa_select_tex_image(texObj, target, level); > if (!texImage) { > /* non-existant texture level */ > @@ -3215,12 +3238,13 @@ teximage(struct gl_context *ctx, GLboolean > compressed, GLuint dims, > if (compressed_texture_error_check(ctx, dims, target, level, > internalFormat, > width, height, depth, > - border, imageSize)) > + border, imageSize, pixels)) > return; > } > else { > if (texture_error_check(ctx, dims, target, level, internalFormat, > - format, type, width, height, depth, border)) > + format, type, width, height, depth, border, > + pixels)) > return; > } > > @@ -3570,7 +3594,8 @@ texsubimage(struct gl_context *ctx, GLuint dims, > GLenum target, GLint level, > > if (texsubimage_error_check(ctx, dims, texObj, target, level, > xoffset, yoffset, zoffset, > - width, height, depth, format, type, > false)) { > + width, height, depth, format, type, > + pixels, false)) { > return; /* error was detected */ > } > > @@ -3624,7 +3649,8 @@ texturesubimage(struct gl_context *ctx, GLuint dims, > > if (texsubimage_error_check(ctx, dims, texObj, texObj->Target, level, > xoffset, yoffset, zoffset, > - width, height, depth, format, type, true)) > { > + width, height, depth, format, type, > + pixels, true)) { > return; /* error was detected */ > } > > @@ -4633,7 +4659,8 @@ compressed_subtexture_error_check(struct gl_context > *ctx, GLint dims, > GLenum target, GLint level, > GLint xoffset, GLint yoffset, GLint > zoffset, > GLsizei width, GLsizei height, GLsizei > depth, > - GLenum format, GLsizei imageSize, bool > dsa) > + GLenum format, GLsizei imageSize, > + const GLvoid *data, bool dsa) > { > struct gl_texture_image *texImage; > GLint expectedSize; > @@ -4653,6 +4680,12 @@ compressed_subtexture_error_check(struct gl_context > *ctx, GLint dims, > return GL_TRUE; > } > > + /* validate the bound PBO, if any */ > + if (!_mesa_validate_pbo_source_compressed(ctx, dims, &ctx->Unpack, > + imageSize, data, > "glCompressedTexImage")) { > + return GL_TRUE; > + } > + > /* Check for invalid pixel storage modes */ > if (!_mesa_compressed_pixel_storage_error_check(ctx, dims, > &ctx->Unpack, > @@ -4758,7 +4791,7 @@ _mesa_compressed_texture_sub_image(struct gl_context > *ctx, GLuint dims, > if (compressed_subtexture_error_check(ctx, dims, texObj, target, > level, xoffset, yoffset, zoffset, > width, height, depth, > - format, imageSize, dsa)) { > + format, imageSize, data, dsa)) { > return; > } > > -- > 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