<div dir="ltr">These comments have been addressed: <a href="http://cgit.freedesktop.org/~ldeks/mesa/commit/?h=adsa-textures&id=7c419db487dc3b2f55a30dca137322baae72f16b">http://cgit.freedesktop.org/~ldeks/mesa/commit/?h=adsa-textures&id=7c419db487dc3b2f55a30dca137322baae72f16b</a>. </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Dec 31, 2014 at 11:37 AM, Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, Dec 31, 2014 at 11:21 AM, Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>> wrote: > On Mon, Dec 15, 2014 at 5:22 PM, Laura Ekstrand <<a href="mailto:laura@jlekstrand.net">laura@jlekstrand.net</a>> 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; >> + } </div></div>Curly braces not required under case. Might use: return dsa ? GL_TRUE : GL_FALSE; <div class="HOEnZb"><div class="h5"> >> 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 >> <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> </div></div></blockquote></div> </div>