<div dir="ltr"><div>Looks good to me. </div>Reviewed-by: Laura Ekstrand <<a href="mailto:laura@jlekstrand.net">laura@jlekstrand.net</a>> </div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Mar 5, 2015 at 12:20 AM, Eduardo Lima Mitev <<a href="mailto:elima@igalia.com" target="_blank">elima@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>