These comments have been addressed: http://cgit.freedesktop.org/~ldeks/mesa/commit/?h=adsa-textures&id=7c419db487dc3b2f55a30dca137322baae72f16b .
On Wed, Dec 31, 2014 at 11:37 AM, Anuj Phogat <anuj.pho...@gmail.com> wrote: > On Wed, Dec 31, 2014 at 11:21 AM, Anuj Phogat <anuj.pho...@gmail.com> > wrote: > > On Mon, Dec 15, 2014 at 5:22 PM, Laura Ekstrand <la...@jlekstrand.net> > wrote: > >> --- > >> src/mapi/glapi/gen/ARB_direct_state_access.xml | 9 + > >> src/mesa/main/texgetimage.c | 315 > +++++++++++++++++++------ > >> src/mesa/main/texgetimage.h | 9 + > >> 3 files changed, 267 insertions(+), 66 deletions(-) > >> > >> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml > b/src/mapi/glapi/gen/ARB_direct_state_access.xml > >> index eef6536..605c614 100644 > >> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml > >> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml > >> @@ -148,6 +148,15 @@ > >> <param name="texture" type="GLuint" /> > >> </function> > >> > >> + <function name="GetTextureImage" offset="assign"> > >> + <param name="texture" type="GLuint" /> > >> + <param name="level" type="GLint" /> > >> + <param name="format" type="GLenum" /> > >> + <param name="type" type="GLenum" /> > >> + <param name="bufSize" type="GLsizei" /> > >> + <param name="pixels" type="GLvoid *" /> > >> + </function> > >> + > >> <function name="GetTextureLevelParameterfv" offset="assign"> > >> <param name="texture" type="GLuint" /> > >> <param name="level" type="GLint" /> > >> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c > >> index 270fc85..32b5ace 100644 > >> --- a/src/mesa/main/texgetimage.c > >> +++ b/src/mesa/main/texgetimage.c > >> @@ -758,24 +758,17 @@ _mesa_GetCompressedTexImage_sw(struct gl_context > *ctx, > >> > >> > >> /** > >> - * Validate the texture target enum supplied to glTexImage or > >> - * glCompressedTexImage. > >> + * Validate the texture target enum supplied to glGetTex(ture)Image or > >> + * glGetCompressedTex(ture)Image. > >> */ > >> static GLboolean > >> -legal_getteximage_target(struct gl_context *ctx, GLenum target) > >> +legal_getteximage_target(struct gl_context *ctx, GLenum target, bool > dsa) > >> { > >> switch (target) { > >> case GL_TEXTURE_1D: > >> case GL_TEXTURE_2D: > >> case GL_TEXTURE_3D: > >> return GL_TRUE; > >> - case GL_TEXTURE_CUBE_MAP_POSITIVE_X_ARB: > >> - case GL_TEXTURE_CUBE_MAP_NEGATIVE_X_ARB: > >> - case GL_TEXTURE_CUBE_MAP_POSITIVE_Y_ARB: > >> - case GL_TEXTURE_CUBE_MAP_NEGATIVE_Y_ARB: > >> - case GL_TEXTURE_CUBE_MAP_POSITIVE_Z_ARB: > >> - case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z_ARB: > >> - return ctx->Extensions.ARB_texture_cube_map; > >> case GL_TEXTURE_RECTANGLE_NV: > >> return ctx->Extensions.NV_texture_rectangle; > >> case GL_TEXTURE_1D_ARRAY_EXT: > >> @@ -783,6 +776,34 @@ legal_getteximage_target(struct gl_context *ctx, > GLenum target) > >> return ctx->Extensions.EXT_texture_array; > >> case GL_TEXTURE_CUBE_MAP_ARRAY: > >> return ctx->Extensions.ARB_texture_cube_map_array; > >> + > >> + /* Section 8.11 (Texture Queries) of the OpenGL 4.5 core profile > spec > >> + * (30.10.2014) says: > >> + * "An INVALID_ENUM error is generated if the effective target > is not > >> + * one of TEXTURE_1D, TEXTURE_2D, TEXTURE_3D, TEXTURE_1D_ARRAY, > >> + * TEXTURE_2D_ARRAY, TEXTURE_CUBE_MAP_ARRAY, TEXTURE_RECTANGLE, > one of > >> + * the targets from table 8.19 (for GetTexImage and GetnTexImage > *only*), > >> + * or TEXTURE_CUBE_MAP (for GetTextureImage *only*)." (Emphasis > added.) > >> + */ > >> + case GL_TEXTURE_CUBE_MAP_POSITIVE_X_ARB: > >> + case GL_TEXTURE_CUBE_MAP_NEGATIVE_X_ARB: > >> + case GL_TEXTURE_CUBE_MAP_POSITIVE_Y_ARB: > >> + case GL_TEXTURE_CUBE_MAP_NEGATIVE_Y_ARB: > >> + case GL_TEXTURE_CUBE_MAP_POSITIVE_Z_ARB: > >> + case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z_ARB: > >> + { > >> + if (dsa) > >> + return GL_FALSE; > >> + else > >> + return ctx->Extensions.ARB_texture_cube_map; > >> + } > >> + case GL_TEXTURE_CUBE_MAP: > >> + { > >> + if (dsa) > >> + return GL_TRUE; > >> + else > >> + return GL_FALSE; > >> + } > Curly braces not required under case. Might use: > return dsa ? GL_TRUE : GL_FALSE; > > >> default: > >> return GL_FALSE; > >> } > >> @@ -790,84 +811,82 @@ legal_getteximage_target(struct gl_context *ctx, > GLenum target) > >> > >> > >> /** > >> - * Do error checking for a glGetTexImage() call. > >> + * Do error checking for a glGetTex(ture)Image() call. > >> * \return GL_TRUE if any error, GL_FALSE if no errors. > >> */ > >> static GLboolean > >> -getteximage_error_check(struct gl_context *ctx, GLenum target, GLint > level, > >> +getteximage_error_check(struct gl_context *ctx, > >> + struct gl_texture_object *texObj, > >> + struct gl_texture_image *texImage, > >> + GLenum target, GLint level, > >> GLenum format, GLenum type, GLsizei > clientMemSize, > >> - GLvoid *pixels ) > >> + GLvoid *pixels, bool dsa ) > >> { > >> - struct gl_texture_object *texObj; > >> - struct gl_texture_image *texImage; > >> + assert(texObj); > >> + assert(texImage); > >> + > >> const GLint maxLevels = _mesa_max_texture_levels(ctx, target); > >> const GLuint dimensions = (target == GL_TEXTURE_3D) ? 3 : 2; > >> - GLenum baseFormat, err; > >> - > >> - if (!legal_getteximage_target(ctx, target)) { > >> - _mesa_error(ctx, GL_INVALID_ENUM, "glGetTexImage(target=0x%x)", > target); > >> - return GL_TRUE; > >> - } > >> + GLenum baseFormat; > >> > >> assert(maxLevels != 0); > >> if (level < 0 || level >= maxLevels) { > >> - _mesa_error( ctx, GL_INVALID_VALUE, "glGetTexImage(level)" ); > >> + _mesa_error( ctx, GL_INVALID_VALUE, "glGetTex%sImage(level)", > >> + dsa ? "ture" : ""); > >> return GL_TRUE; > >> } > >> > >> - err = _mesa_error_check_format_and_type(ctx, format, type); > >> - if (err != GL_NO_ERROR) { > >> - _mesa_error(ctx, err, "glGetTexImage(format/type)"); > >> - return GL_TRUE; > >> - } > >> - > >> - texObj = _mesa_get_current_tex_object(ctx, target); > >> - > >> - if (!texObj) { > >> - _mesa_error(ctx, GL_INVALID_ENUM, "glGetTexImage(target)"); > >> - return GL_TRUE; > >> - } > >> - > >> - texImage = _mesa_select_tex_image(ctx, texObj, target, level); > >> - if (!texImage) { > >> - /* non-existant texture image */ > >> - return GL_TRUE; > >> - } > >> + /* > >> + * Format and type checking has been moved up to GetnTexImage and > >> + * GetTextureImage so that it happens before getting the texImage > object. > >> + */ > >> > >> baseFormat = _mesa_get_format_base_format(texImage->TexFormat); > >> - > >> + > >> /* Make sure the requested image format is compatible with the > >> * texture's format. > >> */ > >> if (_mesa_is_color_format(format) > >> && !_mesa_is_color_format(baseFormat)) { > >> - _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format > mismatch)"); > >> + _mesa_error(ctx, GL_INVALID_OPERATION, > >> + "glGetTex%sImage(format mismatch)", > >> + dsa ? "ture" : ""); > >> return GL_TRUE; > >> } > >> else if (_mesa_is_depth_format(format) > >> && !_mesa_is_depth_format(baseFormat) > >> && !_mesa_is_depthstencil_format(baseFormat)) { > >> - _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format > mismatch)"); > >> + _mesa_error(ctx, GL_INVALID_OPERATION, > >> + "glGetTex%sImage(format mismatch)", > >> + dsa ? "ture" : ""); > >> return GL_TRUE; > >> } > >> else if (_mesa_is_stencil_format(format) > >> && !ctx->Extensions.ARB_texture_stencil8) { > >> - _mesa_error(ctx, GL_INVALID_ENUM, > "glGetTexImage(format=GL_STENCIL_INDEX)"); > >> + _mesa_error(ctx, GL_INVALID_ENUM, > >> + "glGetTex%sImage(format=GL_STENCIL_INDEX)", > >> + dsa ? "ture" : ""); > >> return GL_TRUE; > >> } > >> else if (_mesa_is_ycbcr_format(format) > >> && !_mesa_is_ycbcr_format(baseFormat)) { > >> - _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format > mismatch)"); > >> + _mesa_error(ctx, GL_INVALID_OPERATION, > >> + "glGetTex%sImage(format mismatch)", > >> + dsa ? "ture" : ""); > >> return GL_TRUE; > >> } > >> else if (_mesa_is_depthstencil_format(format) > >> && !_mesa_is_depthstencil_format(baseFormat)) { > >> - _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format > mismatch)"); > >> + _mesa_error(ctx, GL_INVALID_OPERATION, > >> + "glGetTex%sImage(format mismatch)", > >> + dsa ? "ture" : ""); > >> return GL_TRUE; > >> } > >> else if (_mesa_is_enum_format_integer(format) != > >> _mesa_is_format_integer(texImage->TexFormat)) { > >> - _mesa_error(ctx, GL_INVALID_OPERATION, "glGetTexImage(format > mismatch)"); > >> + _mesa_error(ctx, GL_INVALID_OPERATION, > >> + "glGetTex%sImage(format mismatch)", > >> + dsa ? "ture" : ""); > >> return GL_TRUE; > >> } > >> > >> @@ -876,11 +895,14 @@ getteximage_error_check(struct gl_context *ctx, > GLenum target, GLint level, > >> format, type, clientMemSize, > pixels)) { > >> if (_mesa_is_bufferobj(ctx->Pack.BufferObj)) { > >> _mesa_error(ctx, GL_INVALID_OPERATION, > >> - "glGetTexImage(out of bounds PBO access)"); > >> + "glGetTex%sImage(out of bounds PBO access)", > >> + dsa ? "ture" : ""); > >> } else { > >> _mesa_error(ctx, GL_INVALID_OPERATION, > >> - "glGetnTexImageARB(out of bounds access:" > >> - " bufSize (%d) is too small)", clientMemSize); > >> + "%s(out of bounds access:" > >> + " bufSize (%d) is too small)", > >> + dsa ? "glGetTextureImage" : "glGetnTexImageARB", > >> + clientMemSize); > >> } > >> return GL_TRUE; > >> } > >> @@ -889,7 +911,7 @@ getteximage_error_check(struct gl_context *ctx, > GLenum target, GLint level, > >> /* PBO should not be mapped */ > >> if (_mesa_check_disallowed_mapping(ctx->Pack.BufferObj)) { > >> _mesa_error(ctx, GL_INVALID_OPERATION, > >> - "glGetTexImage(PBO is mapped)"); > >> + "glGetTex%sImage(PBO is mapped)", dsa ? "ture" : > ""); > >> return GL_TRUE; > >> } > >> } > >> @@ -898,9 +920,12 @@ getteximage_error_check(struct gl_context *ctx, > GLenum target, GLint level, > >> } > >> > >> > >> - > >> /** > >> - * Get texture image. Called by glGetTexImage. > >> + * This is the implementation for glGetnTexImageARB, glGetTextureImage, > >> + * and glGetTexImage. > >> + * > >> + * Requires caller to pass in texImage object because > _mesa_GetTextureImage > >> + * must handle the GL_TEXTURE_CUBE_MAP target. > >> * > >> * \param target texture target. > >> * \param level image level. > >> @@ -908,19 +933,29 @@ getteximage_error_check(struct gl_context *ctx, > GLenum target, GLint level, > >> * \param type pixel data type for returned image. > >> * \param bufSize size of the pixels data buffer. > >> * \param pixels returned pixel data. > >> + * \param dsa True when the caller an ARB_direct_state_access function, > >> + * false otherwise > >> */ > >> -void GLAPIENTRY > >> -_mesa_GetnTexImageARB( GLenum target, GLint level, GLenum format, > >> - GLenum type, GLsizei bufSize, GLvoid *pixels ) > >> +void > >> +_mesa_get_texture_image( struct gl_context *ctx, > >> + struct gl_texture_object *texObj, > >> + struct gl_texture_image *texImage, GLenum > target, > >> + GLint level, GLenum format, GLenum type, > >> + GLsizei bufSize, GLvoid *pixels, bool dsa ) > >> { > >> - struct gl_texture_object *texObj; > >> - struct gl_texture_image *texImage; > >> - GET_CURRENT_CONTEXT(ctx); > >> + assert(texObj); > >> + assert(texImage); > >> > >> FLUSH_VERTICES(ctx, 0); > >> > >> - if (getteximage_error_check(ctx, target, level, format, type, > >> - bufSize, pixels)) { > >> + /* > >> + * Legal target checking has been moved up to GetnTexImage and > >> + * GetTextureImage so that it can be caught before receiving a NULL > >> + * texImage object and exiting. > >> + */ > >> + > >> + if (getteximage_error_check(ctx, texObj, texImage, target, level, > format, > >> + type, bufSize, pixels, dsa)) { > >> return; > >> } > >> > >> @@ -929,15 +964,13 @@ _mesa_GetnTexImageARB( GLenum target, GLint > level, GLenum format, > >> return; > >> } > >> > >> - texObj = _mesa_get_current_tex_object(ctx, target); > >> - texImage = _mesa_select_tex_image(ctx, texObj, target, level); > >> - > >> if (_mesa_is_zero_size_texture(texImage)) > >> return; > >> > >> if (MESA_VERBOSE & (VERBOSE_API | VERBOSE_TEXTURE)) { > >> - _mesa_debug(ctx, "glGetTexImage(tex %u) format = %s, w=%d, h=%d," > >> + _mesa_debug(ctx, "glGetTex%sImage(tex %u) format = %s, w=%d, > h=%d," > >> " dstFmt=0x%x, dstType=0x%x\n", > >> + dsa ? "ture": "", > >> texObj->Name, > >> _mesa_get_format_name(texImage->TexFormat), > >> texImage->Width, texImage->Height, > >> @@ -951,6 +984,58 @@ _mesa_GetnTexImageARB( GLenum target, GLint level, > GLenum format, > >> _mesa_unlock_texture(ctx, texObj); > >> } > >> > >> +/** > >> + * Get texture image. Called by glGetTexImage. > >> + * > >> + * \param target texture target. > >> + * \param level image level. > >> + * \param format pixel data format for returned image. > >> + * \param type pixel data type for returned image. > >> + * \param bufSize size of the pixels data buffer. > >> + * \param pixels returned pixel data. > >> + */ > >> +void GLAPIENTRY > >> +_mesa_GetnTexImageARB( GLenum target, GLint level, GLenum format, > >> + GLenum type, GLsizei bufSize, GLvoid *pixels ) > >> +{ > >> + struct gl_texture_object *texObj; > >> + struct gl_texture_image *texImage; > >> + GLenum err; > >> + GET_CURRENT_CONTEXT(ctx); > >> + > >> + /* > >> + * This has been moved here because a format/type mismatch can > cause a NULL > >> + * texImage object, which in turn causes the mismatch error to be > >> + * ignored. > >> + */ > >> + err = _mesa_error_check_format_and_type(ctx, format, type); > >> + if (err != GL_NO_ERROR) { > >> + _mesa_error(ctx, err, "glGetnTexImage(format/type)"); > >> + return; > >> + } > >> + > >> + /* > >> + * Legal target checking has been moved here to prevent exiting > with a NULL > >> + * texImage object. > >> + */ > >> + if (!legal_getteximage_target(ctx, target, false)) { > >> + _mesa_error(ctx, GL_INVALID_ENUM, "glGetnTexImage(target=0x%x)", > >> + target); > >> + return; > >> + } > >> + > >> + texObj = _mesa_get_current_tex_object(ctx, target); > >> + if (!texObj) > >> + return; > >> + > >> + texImage = _mesa_select_tex_image(ctx, texObj, target, level); > >> + if (!texImage) > >> + return; > >> + > >> + _mesa_get_texture_image(ctx, texObj, texImage, target, level, > format, type, > >> + bufSize, pixels, false); > >> +} > >> + > >> > >> void GLAPIENTRY > >> _mesa_GetTexImage( GLenum target, GLint level, GLenum format, > >> @@ -959,6 +1044,104 @@ _mesa_GetTexImage( GLenum target, GLint level, > GLenum format, > >> _mesa_GetnTexImageARB(target, level, format, type, INT_MAX, pixels); > >> } > >> > >> +/** > >> + * Get texture image. > >> + * > >> + * \param texture texture name. > >> + * \param level image level. > >> + * \param format pixel data format for returned image. > >> + * \param type pixel data type for returned image. > >> + * \param bufSize size of the pixels data buffer. > >> + * \param pixels returned pixel data. > >> + */ > >> +void GLAPIENTRY > >> +_mesa_GetTextureImage( GLuint texture, GLint level, GLenum format, > >> + GLenum type, GLsizei bufSize, GLvoid *pixels ) > >> +{ > >> + struct gl_texture_object *texObj; > >> + struct gl_texture_image *texImage; > >> + int i; > >> + GLint image_stride; > >> + GLenum err; > >> + GET_CURRENT_CONTEXT(ctx); > >> + > >> + /* > >> + * This has been moved here because a format/type mismatch can > cause a NULL > >> + * texImage object, which in turn causes the mismatch error to be > >> + * ignored. > >> + */ > >> + err = _mesa_error_check_format_and_type(ctx, format, type); > >> + if (err != GL_NO_ERROR) { > >> + _mesa_error(ctx, err, "glGetTextureImage(format/type)"); > >> + return; > >> + } > >> + > >> + texObj = _mesa_lookup_texture_err(ctx, texture, > "glGetTextureImage"); > >> + if (!texObj) > >> + return; > >> + > >> + /* > >> + * Legal target checking has been moved here to prevent exiting > with a NULL > >> + * texImage object. > >> + */ > >> + if (!legal_getteximage_target(ctx, texObj->Target, true)) { > >> + _mesa_error(ctx, GL_INVALID_ENUM, > "glGetTextureImage(target=0x%x)", > >> + texObj->Target); > >> + return; > >> + } > >> + > >> + /* Must special case GL_TEXTURE_CUBE_MAP. */ > > Must handle special case > >> + if (texObj->Target == GL_TEXTURE_CUBE_MAP) { > >> + > >> + /* Error checking */ > >> + if (texObj->NumLayers < 6) { > >> + /* Not enough image planes for a cube map. The spec does not > say > >> + * what should happen in this case because the user has always > >> + * specified each cube face separately (using > >> + * GL_TEXTURE_CUBE_MAP_POSITIVE_X+i) in previous GL versions. > >> + * This is addressed in Khronos Bug 13223. > >> + */ > >> + _mesa_error(ctx, GL_INVALID_OPERATION, > >> + "glGetTextureImage(insufficient cube map > storage)"); > >> + return; > >> + } > >> + for (i = 0; i < 6; ++i) { /* For each face. */ > >> + if (!texObj->Image[i][level]) { > >> + /* Not enough image planes for a cube map. The spec does > not say > >> + * what should happen in this case because the user has > always > >> + * specified each cube face separately (using > >> + * GL_TEXTURE_CUBE_MAP_POSITIVE_X+i) in previous GL > versions. > >> + * This is addressed in Khronos Bug 13223. > >> + */ > >> + _mesa_error(ctx, GL_INVALID_OPERATION, > >> + "glGetTextureImage(insufficient cube map > storage)"); > >> + return; > >> + } > >> + } > >> + > >> + > > extra new line. > >> + /* Copy each face. */ > >> + for (i = 0; i < 6; ++i) { /* For each face. */ > > Duplicate comment. > >> + texImage = texObj->Image[i][level]; > >> + _mesa_get_texture_image(ctx, texObj, texImage, > texObj->Target, level, > >> + format, type, bufSize, pixels, true); > >> + > >> + image_stride = _mesa_image_image_stride(&ctx->Pack, > texImage->Width, > >> + texImage->Height, > format, > >> + type); > >> + pixels += image_stride; > >> + bufSize -= image_stride; > >> + } > >> + } > >> + else { > >> + texImage = _mesa_select_tex_image(ctx, texObj, texObj->Target, > level); > >> + if (!texImage) > >> + return; > >> + > >> + _mesa_get_texture_image(ctx, texObj, texImage, texObj->Target, > level, > >> + format, type, bufSize, pixels, true); > >> + } > >> +} > >> > >> /** > >> * Do error checking for a glGetCompressedTexImage() call. > >> @@ -973,7 +1156,7 @@ getcompressedteximage_error_check(struct > gl_context *ctx, GLenum target, > >> const GLint maxLevels = _mesa_max_texture_levels(ctx, target); > >> GLuint compressedSize, dimensions; > >> > >> - if (!legal_getteximage_target(ctx, target)) { > >> + if (!legal_getteximage_target(ctx, target, false)) { > >> _mesa_error(ctx, GL_INVALID_ENUM, > "glGetCompressedTexImage(target=0x%x)", > >> target); > >> return GL_TRUE; > >> diff --git a/src/mesa/main/texgetimage.h b/src/mesa/main/texgetimage.h > >> index 54739fd..085f580 100644 > >> --- a/src/mesa/main/texgetimage.h > >> +++ b/src/mesa/main/texgetimage.h > >> @@ -47,6 +47,12 @@ _mesa_GetCompressedTexImage_sw(struct gl_context > *ctx, > >> struct gl_texture_image *texImage, > >> GLvoid *data); > >> > >> +extern void > >> +_mesa_get_texture_image( struct gl_context *ctx, > >> + struct gl_texture_object *texObj, > >> + struct gl_texture_image *texImage, GLenum > target, > >> + GLint level, GLenum format, GLenum type, > >> + GLsizei bufSize, GLvoid *pixels, bool dsa ); > >> > >> > >> extern void GLAPIENTRY > >> @@ -55,6 +61,9 @@ _mesa_GetTexImage( GLenum target, GLint level, > >> extern void GLAPIENTRY > >> _mesa_GetnTexImageARB( GLenum target, GLint level, GLenum format, > >> GLenum type, GLsizei bufSize, GLvoid *pixels ); > >> +extern void GLAPIENTRY > >> +_mesa_GetTextureImage( GLuint texture, GLint level, GLenum format, > >> + GLenum type, GLsizei bufSize, GLvoid *pixels ); > >> > >> extern void GLAPIENTRY > >> _mesa_GetCompressedTexImage(GLenum target, GLint lod, GLvoid *img); > >> -- > >> 2.1.0 > >> > >> _______________________________________________ > >> 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